Improve readability of artifacts `Metadata` related code

4 files changed, 34 insertions, 45 deletions

diff --git a/config/routes.rb b/config/routes.rb
index 0ba81a3411a..d7fb7407794 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -603,15 +603,14 @@ Rails.application.routes.draw do
 
           member do
             get :status
-            get :download
             post :cancel
             post :retry
           end
 
           resource :artifacts, only: [] do
             get :download
-            get 'browse(/*path)', action: :browse, as: :browse, format: false
-            get 'file/*path', action: :file, as: :file, format: false
+            get :browse, path: 'browse(/*path)', action: :browse, format: false
+            get :file, path: 'file/*path', action: :file, format: false
           end
         end
 
diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb
index bfdfc9a1d7d..94821c0eae0 100644
--- a/lib/gitlab/ci/build/artifacts/metadata.rb
+++ b/lib/gitlab/ci/build/artifacts/metadata.rb
@@ -6,7 +6,9 @@ module Gitlab
     module Build
       module Artifacts
         class Metadata
-          VERSION_PATTERN = '[\w\s]+(\d+\.\d+\.\d+)'
+          VERSION_PATTERN = /^[\w\s]+(\d+\.\d+\.\d+)/
+          INVALID_PATH_PATTERN = %r{(^\.?\.?/)|(/\.?\.?/)}
+
           attr_reader :file, :path, :full_version
 
           def initialize(file, path)
@@ -15,7 +17,7 @@ module Gitlab
           end
 
           def version
-            @full_version.match(/#{VERSION_PATTERN}/).captures.first
+            @full_version.match(VERSION_PATTERN)[1]
           end
 
           def errors
@@ -27,7 +29,7 @@ module Gitlab
             end
           end
 
-          def match!
+          def find_entries!
             gzip do |gz|
               2.times { read_string(gz) } # version and errors fields
               match_entries(gz)
@@ -35,8 +37,8 @@ module Gitlab
           end
 
           def to_entry
-            entires, metadata = match!
-            Entry.new(@path, entires, metadata)
+            entries, metadata = find_entries!
+            Entry.new(@path, entries, metadata)
           end
 
           private
@@ -44,7 +46,6 @@ module Gitlab
           def match_entries(gz)
             paths, metadata = [], []
             match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$}
-            invalid_pattern = %r{(^\.?\.?/)|(/\.?\.?/)}
 
             until gz.eof? do
               begin
@@ -53,7 +54,7 @@ module Gitlab
                
                 next unless path.valid_encoding? && meta.valid_encoding?
                 next unless path =~ match_pattern
-                next if path =~ invalid_pattern
+                next if path =~ INVALID_PATH_PATTERN
 
                 paths.push(path)
                 metadata.push(JSON.parse(meta, symbolize_names: true))
@@ -73,7 +74,7 @@ module Gitlab
                 raise StandardError, 'Artifacts metadata file empty!'
               end
 
-              unless version_string =~ /^#{VERSION_PATTERN}/
+              unless version_string =~ VERSION_PATTERN
                 raise StandardError, 'Invalid version!'
               end
 
@@ -92,19 +93,8 @@ module Gitlab
             gz.read(string_size)
           end
 
-          def gzip
-            open do |file|
-              gzip = Zlib::GzipReader.new(file)
-              begin
-                yield gzip
-              ensure
-                gzip.close
-              end
-            end
-          end
-
-          def open
-            File.open(@file) { |file| yield file }
+          def gzip(&block)
+            Zlib::GzipReader.open(@file, &block)
           end
         end
       end
diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb
index 2fb6c327729..12bb1bf0346 100644
--- a/lib/gitlab/ci/build/artifacts/metadata/entry.rb
+++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb
@@ -11,12 +11,12 @@ module Gitlab
       # This class is working only with UTF-8 encoded paths.
       #
       class Entry
-        attr_reader :path, :entires
+        attr_reader :path, :entries
         attr_accessor :name
 
-        def initialize(path, entires, metadata = [])
+        def initialize(path, entries, metadata = [])
           @path = path.force_encoding('UTF-8')
-          @entires = entires
+          @entries = entries
           @metadata = metadata
 
           if path.include?("\0")
@@ -42,7 +42,7 @@ module Gitlab
 
         def parent
           return nil unless has_parent?
-          new(@path.chomp(basename))
+          new_entry(@path.chomp(basename))
         end
 
         def basename
@@ -58,7 +58,7 @@ module Gitlab
           return @children if @children
 
           child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$}
-          @children = select_entires { |entry| entry =~ child_pattern }
+          @children = select_entries { |entry| entry =~ child_pattern }
         end
 
         def directories(opts = {})
@@ -77,7 +77,7 @@ module Gitlab
         end
 
         def metadata
-          @index ||= @entires.index(@path)
+          @index ||= @entries.index(@path)
           @metadata[@index] || {}
         end
 
@@ -90,7 +90,7 @@ module Gitlab
         end
 
         def exists?
-          blank_node? || @entires.include?(@path)
+          blank_node? || @entries.include?(@path)
         end
 
         def empty?
@@ -102,7 +102,7 @@ module Gitlab
         end
 
         def ==(other)
-          @path == other.path && @entires == other.entires
+          @path == other.path && @entries == other.entries
         end
 
         def inspect
@@ -111,13 +111,13 @@ module Gitlab
 
         private
 
-        def new(path)
-          self.class.new(path, @entires, @metadata)
+        def new_entry(path)
+          self.class.new(path, @entries, @metadata)
         end
 
-        def select_entires
-          selected = @entires.select { |entry| yield entry }
-          selected.map { |path| new(path) }
+        def select_entries
+          selected = @entries.select { |entry| yield entry }
+          selected.map { |path| new_entry(path) }
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb
index 42fbe40c890..8560493f5b5 100644
--- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb
+++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb
@@ -10,8 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
   end
 
   context 'metadata file exists' do
-    describe '#match! empty string' do
-      subject { metadata('').match! }
+    describe '#find_entries! empty string' do
+      subject { metadata('').find_entries! }
 
       it 'matches correct paths' do
         expect(subject.first).to contain_exactly 'ci_artifacts.txt',
@@ -29,8 +29,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
       end
     end
 
-    describe '#match! other_artifacts_0.1.2/' do
-      subject { metadata('other_artifacts_0.1.2/').match! }
+    describe '#find_entries! other_artifacts_0.1.2/' do
+      subject { metadata('other_artifacts_0.1.2/').find_entries! }
 
       it 'matches correct paths' do
         expect(subject.first).
@@ -40,8 +40,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
       end
     end
 
-    describe '#match! other_artifacts_0.1.2/another-subdirectory/' do
-      subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! }
+    describe '#find_entries! other_artifacts_0.1.2/another-subdirectory/' do
+      subject { metadata('other_artifacts_0.1.2/another-subdirectory/').find_entries! }
 
       it 'matches correct paths' do
         expect(subject.first).
@@ -75,9 +75,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
   context 'metadata file does not exist' do
     let(:metadata_file_path) { '' }
 
-    describe '#match!' do
+    describe '#find_entries!' do
       it 'raises error' do
-        expect { metadata.match! }.to raise_error(Errno::ENOENT)
+        expect { metadata.find_entries! }.to raise_error(Errno::ENOENT)
       end
     end
   end