Update API and fetching task

4 files changed, 51 insertions, 41 deletions

diff --git a/lib/gitlab/gitignore.rb b/lib/gitlab/gitignore.rb
index a2de2831e38..f46b43b61a4 100644
--- a/lib/gitlab/gitignore.rb
+++ b/lib/gitlab/gitignore.rb
@@ -2,15 +2,16 @@ module Gitlab
   class Gitignore
     FILTER_REGEX = /\.gitignore\z/.freeze
 
-    attr_accessor :name, :directory
+    def initialize(path)
+      @path = path
+    end
 
-    def initialize(name, directory)
-      @name       = name
-      @directory  = directory
+    def name
+      File.basename(@path, '.gitignore')
     end
 
     def content
-      File.read(path)
+      File.read(@path)
     end
 
     class << self
@@ -22,46 +23,33 @@ module Gitlab
         file_name = "#{key}.gitignore"
 
         directory = select_directory(file_name)
-        directory ? new(key, directory) : nil
+        directory ? new(File.join(directory, file_name)) : nil
       end
 
       def global
-        files_for_folder(global_dir).map { |f| new(f, global_dir) }
+        files_for_folder(global_dir).map { |file| new(File.join(global_dir, file)) }
       end
 
       def languages_frameworks
-        files_for_folder(gitignore_dir).map { |f| new(f, gitignore_dir) }
+        files_for_folder(gitignore_dir).map { |file| new(File.join(gitignore_dir, file)) }
       end
-    end
 
-    private
-
-    def path
-      File.expand_path("#{name}.gitignore", directory)
-    end
+      private
 
-    class << self
       def select_directory(file_name)
-        [self.gitignore_dir, self.global_dir].find { |dir| File.exist?(File.expand_path(file_name, dir)) }
+        [gitignore_dir, global_dir].find { |dir| File.exist?(File.join(dir, file_name)) }
       end
 
       def global_dir
-        File.expand_path('Global', gitignore_dir)
+        File.join(gitignore_dir, 'Global')
       end
 
       def gitignore_dir
-        File.expand_path('vendor/gitignore', Rails.root)
+        Rails.root.join('vendor/gitignore')
       end
 
       def files_for_folder(dir)
-        gitignores = []
-        Dir.entries(dir).each do |e|
-          next unless e.end_with?('.gitignore')
-
-          gitignores << e.gsub(FILTER_REGEX, '')
-        end
-
-        gitignores
+        Dir.glob("#{dir.to_s}/*.gitignore").map { |file| file.gsub(FILTER_REGEX, '') }
       end
     end
   end
diff --git a/lib/tasks/gitlab/update_gitignore.rake b/lib/tasks/gitlab/update_gitignore.rake
index 61cbfd6737d..84aa312002b 100644
--- a/lib/tasks/gitlab/update_gitignore.rake
+++ b/lib/tasks/gitlab/update_gitignore.rake
@@ -1,26 +1,46 @@
 namespace :gitlab do
   desc "GitLab | Update gitignore"
   task :update_gitignore do
-    dir = File.expand_path('vendor', Rails.root)
-    FileUtils.cd(dir)
+    unless clone_gitignores
+      puts "Cloning the gitignores failed".red
+      return
+    end
 
-    dir = File.expand_path('gitignore', dir)
-    clone_gitignores(dir)
-    remove_unneeded_files(dir)
+    remove_unneeded_files(gitignore_directory)
+    remove_unneeded_files(global_directory)
 
     puts "Done".green
   end
 
-  def clone_gitignores(dir)
-    FileUtils.rm_rf(dir) if Dir.exist?(dir)
+  def clone_gitignores
+    FileUtils.rm_rf(gitignore_directory) if Dir.exist?(gitignore_directory)
+    FileUtils.cd vendor_directory
+
     system('git clone --depth=1 --branch=master https://github.com/github/gitignore.git')
   end
 
-  def remove_unneeded_files(dir)
-    [File.expand_path('Global', dir), dir].each do |path|
-      Dir.entries(path).reject { |e| e =~ /(\.{1,2}|Global|\.gitignore)\z/ }.each do |file|
-        FileUtils.rm_rf File.expand_path(file, path)
-      end
+  # Retain only certain files:
+  # - The LICENSE, because we have to
+  # - The sub dir global
+  # - The gitignores themself
+  # - Dir.entires returns also the entries '.' and '..'
+  def remove_unneeded_files(path)
+    Dir.foreach(path) do |file|
+      FileUtils.rm_rf(File.join(path, file)) unless file =~ /(\.{1,2}|LICENSE|Global|\.gitignore)\z/
     end
   end
+
+  private
+
+  def vendor_directory
+    Rails.root.join('vendor')
+  end
+
+  def gitignore_directory
+    File.join(vendor_directory, 'gitignore')
+  end
+
+  def global_directory
+    File.join(gitignore_directory, 'Global')
+  end
 end
diff --git a/spec/lib/gitlab/gitignore_spec.rb b/spec/lib/gitlab/gitignore_spec.rb
index 5dab821a8ec..72baa516cc4 100644
--- a/spec/lib/gitlab/gitignore_spec.rb
+++ b/spec/lib/gitlab/gitignore_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Gitignore do
   subject { Gitlab::Gitignore }
+
   describe '.all' do
     it 'strips the gitignore suffix' do
       expect(subject.all.first.name).not_to end_with('.gitignore')
@@ -24,12 +25,13 @@ describe Gitlab::Gitignore do
       ruby = subject.find('Ruby')
 
       expect(ruby).to be_a Gitlab::Gitignore
+      expect(ruby.name).to eq('Ruby')
     end
   end
 
   describe '#content' do
     it 'loads the full file' do
-      gitignore = subject.new('Ruby', File.expand_path('vendor/gitignore', Rails.root))
+      gitignore = subject.new(Rails.root.join('vendor/gitignore/Ruby.gitignore'))
 
       expect(gitignore.name).to eq 'Ruby'
       expect(gitignore.content).to start_with('*.gem')
diff --git a/spec/requests/api/gitignores_spec.rb b/spec/requests/api/gitignores_spec.rb
index d0e576e637c..aab2d8c81b9 100644
--- a/spec/requests/api/gitignores_spec.rb
+++ b/spec/requests/api/gitignores_spec.rb
@@ -13,8 +13,8 @@ describe API::Gitignores, api: true  do
   describe 'Entity GitignoresList' do
     before { get api('/gitignores') }
 
-    it { expect(json_response.first['name']).to be_truthy }
-    it { expect(json_response.first['content']).to be_falsey }
+    it { expect(json_response.first['name']).not_to be_nil }
+    it { expect(json_response.first['content']).to be_nil }
   end
 
   describe 'GET /gitignores' do