Use Hash to store paths and entries metadata in artifacts browser

lib/gitlab/ci/build/artifacts/metadata.rb

@@ -37,14 +37,14 @@ module Gitlab
           end
 
           def to_entry
-            entries, metadata = find_entries!
-            Entry.new(@path, entries, metadata)
+            entries = find_entries!
+            Entry.new(@path, entries)
           end
 
           private
 
           def match_entries(gz)
-            paths, metadata = [], []
+            entries = {}
             match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$}
 
             until gz.eof? do
@@ -56,14 +56,13 @@ module Gitlab
                 next unless path =~ match_pattern
                 next if path =~ INVALID_PATH_PATTERN
 
-                paths.push(path)
-                metadata.push(JSON.parse(meta, symbolize_names: true))
+                entries.store(path, JSON.parse(meta, symbolize_names: true))
               rescue JSON::ParserError, Encoding::CompatibilityError
                 next
               end
             end
 
-            [paths, metadata]
+            entries
           end
 
           def read_version

lib/gitlab/ci/build/artifacts/metadata/entry.rb

@@ -14,10 +14,9 @@ module Gitlab
         attr_reader :path, :entries
         attr_accessor :name
 
-        def initialize(path, entries, metadata = [])
-          @path = path.force_encoding('UTF-8')
+        def initialize(path, entries)
+          @path = path.dup.force_encoding('UTF-8')
           @entries = entries
-          @metadata = metadata
 
           if path.include?("\0")
             raise ArgumentError, 'Path contains zero byte character!'
@@ -42,7 +41,7 @@ module Gitlab
 
         def parent
           return nil unless has_parent?
-          new_entry(@path.chomp(basename))
+          self.class.new(@path.chomp(basename), @entries)
         end
 
         def basename
@@ -77,8 +76,7 @@ module Gitlab
         end
 
         def metadata
-          @index ||= @entries.index(@path)
-          @metadata[@index] || {}
+          @entries[@path] || {}
         end
 
         def nodes
@@ -111,13 +109,9 @@ module Gitlab
 
         private
 
-        def new_entry(path)
-          self.class.new(path, @entries, @metadata)
-        end
-
         def select_entries
-          selected = @entries.select { |entry| yield entry }
-          selected.map { |path| new_entry(path) }
+          selected = @entries.select { |entry, _metadata| yield entry }
+          selected.map { |path, _metadata| self.class.new(path, @entries) }
         end
       end
     end

spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb

@@ -2,26 +2,26 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
   let(:entries) do
-    ['path/',
-     'path/dir_1/',
-     'path/dir_1/file_1',
-     'path/dir_1/file_b',
-     'path/dir_1/subdir/',
-     'path/dir_1/subdir/subfile',
-     'path/second_dir',
-     'path/second_dir/dir_3/file_2',
-     'path/second_dir/dir_3/file_3',
-     'another_directory/',
-     'another_file',
-     '/file/with/absolute_path']
+    { 'path/' => {},
+      'path/dir_1/' => {},
+      'path/dir_1/file_1' => {},
+      'path/dir_1/file_b' => {},
+      'path/dir_1/subdir/' => {},
+      'path/dir_1/subdir/subfile' => {},
+      'path/second_dir' => {},
+      'path/second_dir/dir_3/file_2' => {},
+      'path/second_dir/dir_3/file_3'=> {},
+      'another_directory/'=> {},
+      'another_file' => {},
+      '/file/with/absolute_path' => {} }
   end
 
   def path(example)
-    string_path(example.metadata[:path])
+    entry(example.metadata[:path])
   end
 
-  def string_path(string_path)
-    described_class.new(string_path, entries)
+  def entry(path)
+    described_class.new(path, entries)
   end
 
   describe '/file/with/absolute_path', path: '/file/with/absolute_path' do
@@ -53,7 +53,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
 
     describe '#parent' do
       subject { |example| path(example).parent }
-      it { is_expected.to eq string_path('path/') }
+      it { is_expected.to eq entry('path/') }
     end
 
     describe '#children' do
@@ -61,9 +61,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
 
       it { is_expected.to all(be_an_instance_of described_class) }
       it do
-        is_expected.to contain_exactly string_path('path/dir_1/file_1'),
-                                       string_path('path/dir_1/file_b'),
-                                       string_path('path/dir_1/subdir/')
+        is_expected.to contain_exactly entry('path/dir_1/file_1'),
+                                       entry('path/dir_1/file_b'),
+                                       entry('path/dir_1/subdir/')
       end
     end
 
@@ -73,8 +73,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
       it { is_expected.to all(be_file) }
       it { is_expected.to all(be_an_instance_of described_class) }
       it do
-        is_expected.to contain_exactly string_path('path/dir_1/file_1'),
-                                       string_path('path/dir_1/file_b')
+        is_expected.to contain_exactly entry('path/dir_1/file_1'),
+                                       entry('path/dir_1/file_b')
       end
     end
 
@@ -84,7 +84,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
 
         it { is_expected.to all(be_directory) }
         it { is_expected.to all(be_an_instance_of described_class) }
-        it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') }
+        it { is_expected.to contain_exactly entry('path/dir_1/subdir/') }
       end
 
       context 'with option parent: true' do
@@ -93,8 +93,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
         it { is_expected.to all(be_directory) }
         it { is_expected.to all(be_an_instance_of described_class) }
         it do
-          is_expected.to contain_exactly string_path('path/dir_1/subdir/'),
-                                         string_path('path/')
+          is_expected.to contain_exactly entry('path/dir_1/subdir/'),
+                                         entry('path/')
         end
       end
 
@@ -154,15 +154,13 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
 
   describe '#metadata' do
     let(:entries) do
-      ['path/', 'path/file1', 'path/file2']
-    end
-
-    let(:metadata) do
-      [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }]
+      { 'path/' => { name: '/path/' },
+        'path/file1' => { name: '/path/file1' },
+        'path/file2' => { name: '/path/file2' } }
     end
 
     subject do
-      described_class.new('path/file1', entries, metadata).metadata[:name]
+      described_class.new('path/file1', entries).metadata[:name]
     end
 
     it { is_expected.to eq '/path/file1' }

spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb

@@ -14,18 +14,18 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
       subject { metadata('').find_entries! }
 
       it 'matches correct paths' do
-        expect(subject.first).to contain_exactly 'ci_artifacts.txt',
+        expect(subject.keys).to contain_exactly 'ci_artifacts.txt',
                                                 'other_artifacts_0.1.2/',
                                                 'rails_sample.jpg',
                                                 'tests_encoding/'
       end
 
       it 'matches metadata for every path' do
-        expect(subject.last.count).to eq 4
+        expect(subject.keys.count).to eq 4
       end
 
       it 'return Hashes for each metadata' do
-        expect(subject.last).to all(be_kind_of(Hash))
+        expect(subject.values).to all(be_kind_of(Hash))
       end
     end
 
@@ -33,7 +33,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
       subject { metadata('other_artifacts_0.1.2/').find_entries! }
 
       it 'matches correct paths' do
-        expect(subject.first).
+        expect(subject.keys).
           to contain_exactly 'other_artifacts_0.1.2/',
                              'other_artifacts_0.1.2/doc_sample.txt',
                              'other_artifacts_0.1.2/another-subdirectory/'
@@ -44,7 +44,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
       subject { metadata('other_artifacts_0.1.2/another-subdirectory/').find_entries! }
 
       it 'matches correct paths' do
-        expect(subject.first).
+        expect(subject.keys).
           to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/',
                              'other_artifacts_0.1.2/another-subdirectory/empty_directory/',
                              'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif'