Simplify implementation of build artifacts browser (refactoring)

3 files changed, 153 insertions, 174 deletions

diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb
index d90a64fdbb8..996b5d91ff2 100644
--- a/lib/gitlab/ci/build/artifacts/metadata.rb
+++ b/lib/gitlab/ci/build/artifacts/metadata.rb
@@ -6,65 +6,54 @@ module Gitlab
     module Build
       module Artifacts
         class Metadata
-          def initialize(file, path)
-            @file = file
-
-            @path = path.sub(/^\.\//, '')
-            @path << '/' unless path.end_with?('/')
-          end
+          VERSION_PATTERN = '[\w\s]+(\d+\.\d+\.\d+)'
+          attr_reader :file, :path, :full_version
 
-          def exists?
-            File.exists?(@file)
-          end
-
-          def full_version
-            gzip do|gz|
-              read_string(gz) do |size|
-                raise StandardError, 'Artifacts metadata file empty!' unless size
-              end
-            end
+          def initialize(file, path)
+            @file, @path = file, path
+            @full_version = read_version
+            @path << '/' unless path.end_with?('/') || path.empty?
           end
 
           def version
-            full_version.match(/\w+ (\d+\.\d+\.\d+)/).captures.first
+            @full_version.match(/#{VERSION_PATTERN}/).captures.first
           end
 
           def errors
             gzip do|gz|
               read_string(gz) # version
-              JSON.parse(read_string(gz))
+              errors = read_string(gz)
+              raise StandardError, 'Errors field not found!' unless errors
+              JSON.parse(errors)
             end
           end
 
           def match!
-            raise StandardError, 'Metadata file not found !' unless exists?
-
             gzip do |gz|
-              read_string(gz) # version field
-              read_string(gz) # errors field
-              iterate_entries(gz)
+              2.times { read_string(gz) } # version and errors fields
+              match_entries(gz)
             end
           end
 
-          def to_string_path
-            universe, metadata = match!
-            ::Gitlab::StringPath.new(@path, universe, metadata)
+          def to_path
+            Path.new(@path, *match!)
           end
 
           private
 
-          def iterate_entries(gz)
+          def match_entries(gz)
             paths, metadata = [], []
-            
+            child_pattern = %r{^#{Regexp.escape(@path)}[^/\s]*/?$}
+
             until gz.eof? do
               begin
                 path = read_string(gz)
                 meta = read_string(gz)
                
-                next unless path =~ %r{^#{Regexp.escape(@path)}[^/\s]*/?$}
-                
+                next unless path =~ child_pattern
+
                 paths.push(path)
-                metadata.push(JSON.parse(meta, symbolize_names: true))
+                metadata.push(JSON.parse(meta.chomp, symbolize_names: true))
               rescue JSON::ParserError
                 next
               end
@@ -73,16 +62,31 @@ module Gitlab
             [paths, metadata]
           end
 
-          def read_string_size(gz)
+          def read_version
+            gzip do|gz|
+              version_string = read_string(gz)
+
+              unless version_string
+                raise StandardError, 'Artifacts metadata file empty!'
+              end
+
+              unless version_string =~ /^#{VERSION_PATTERN}/
+                raise StandardError, 'Invalid version!'
+              end
+
+              version_string.chomp
+            end
+          end
+
+          def read_uint32(gz)
             binary = gz.read(4)
             binary.unpack('L>')[0] if binary
           end
 
           def read_string(gz)
-            string_size = read_string_size(gz)
-            yield string_size if block_given?
+            string_size = read_uint32(gz)
             return false unless string_size
-            gz.read(string_size).chomp
+            gz.read(string_size)
           end
 
           def gzip
diff --git a/lib/gitlab/ci/build/artifacts/metadata/path.rb b/lib/gitlab/ci/build/artifacts/metadata/path.rb
new file mode 100644
index 00000000000..222903b348e
--- /dev/null
+++ b/lib/gitlab/ci/build/artifacts/metadata/path.rb
@@ -0,0 +1,114 @@
+module Gitlab
+  module Ci::Build::Artifacts
+    class Metadata
+      ##
+      # Class that represents a simplified path to a file or
+      # directory in GitLab CI Build Artifacts binary file / archive
+      #
+      # This is IO-operations safe class, that does similar job to
+      # Ruby's Pathname but without the risk of accessing filesystem.
+      #
+      class Path
+        attr_reader :path, :universe
+        attr_accessor :name
+
+        def initialize(path, universe, metadata = [])
+          @path = path
+          @universe = universe
+          @metadata = metadata
+
+          if path.include?("\0")
+            raise ArgumentError, 'Path contains zero byte character!'
+          end
+        end
+
+        def directory?
+          @path.end_with?('/') || @path.blank?
+        end
+
+        def file?
+          !directory?
+        end
+
+        def has_parent?
+          nodes > 0
+        end
+
+        def parent
+          return nil unless has_parent?
+          new(@path.chomp(basename))
+        end
+
+        def basename
+          directory? ? name + ::File::SEPARATOR : name
+        end
+
+        def name
+          @name || @path.split(::File::SEPARATOR).last
+        end
+
+        def children
+          return [] unless directory?
+          return @children if @children
+
+          child_pattern = %r{^#{Regexp.escape(@path)}[^/\s]+/?$}
+          @children = select { |entry| entry =~ child_pattern }
+        end
+
+        def directories
+          return [] unless directory?
+          children.select(&:directory?)
+        end
+
+        def directories!
+          return directories unless has_parent?
+
+          dotted_parent = parent
+          dotted_parent.name = '..'
+          directories.prepend(dotted_parent)
+        end
+
+        def files
+          return [] unless directory?
+          children.select(&:file?)
+        end
+
+        def metadata
+          @index ||= @universe.index(@path)
+          @metadata[@index] || {}
+        end
+
+        def nodes
+          @path.count('/') + (file? ? 1 : 0)
+        end
+
+        def exists?
+          @path.blank? || @universe.include?(@path)
+        end
+
+        def to_s
+          @path
+        end
+
+        def ==(other)
+          @path == other.path && @universe == other.universe
+        end
+
+        def inspect
+          "#{self.class.name}: #{@path}"
+        end
+
+        private
+
+        def new(path)
+          self.class.new(path, @universe, @metadata)
+        end
+
+        def select
+          selected = @universe.select { |entry| yield entry }
+          selected.map { |path| new(path) }
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb
deleted file mode 100644
index 774d4244a2a..00000000000
--- a/lib/gitlab/string_path.rb
+++ /dev/null
@@ -1,139 +0,0 @@
-module Gitlab
-  ## 
-  # Class that represents a simplified path to a file or directory
-  #
-  # This is IO-operations safe class, that does similar job to 
-  # Ruby's Pathname but without the risk of accessing filesystem.
-  #
-  class StringPath
-    attr_reader :path, :universe
-    attr_accessor :name
-
-    def initialize(path, universe, metadata = [])
-      @path = sanitize(path)
-      @universe = universe.map { |entry| sanitize(entry) }
-      @metadata = metadata
-    end
-
-    def to_s
-      @path
-    end
-
-    def absolute?
-      @path.start_with?('/')
-    end
-
-    def relative?
-      !absolute?
-    end
-
-    def directory?
-      @path.end_with?('/')
-    end
-
-    def file?
-      !directory?
-    end
-
-    def has_parent?
-      nodes > 1
-    end
-
-    def parent
-      return nil unless has_parent?
-      new(@path.sub(basename, ''))
-    end
-
-    def basename
-      directory? ? name + ::File::SEPARATOR : name
-    end
-
-    def name
-      @name || @path.split(::File::SEPARATOR).last
-    end
-
-    def has_descendants?
-      descendants.any?
-    end
-
-    def descendants
-      return [] unless directory?
-      select { |entry| entry =~ /^#{Regexp.escape(@path)}.+/ }
-    end
-
-    def children
-      return [] unless directory?
-      return @children if @children
-
-      @children = select do |entry|
-        entry =~ %r{^#{Regexp.escape(@path)}[^/\s]+/?$}
-      end
-    end
-
-    def directories
-      return [] unless directory?
-      children.select(&:directory?)
-    end
-
-    def directories!
-      return directories unless has_parent? && directory?
-
-      dotted_parent = parent
-      dotted_parent.name = '..'
-      directories.prepend(dotted_parent)
-    end
-
-    def files
-      return [] unless directory?
-      children.select(&:file?)
-    end
-
-    def metadata
-      index = @universe.index(@path)
-      @metadata[index] || {}
-    end
-
-    def nodes
-      @path.count('/') + (file? ? 1 : 0)
-    end
-
-    def exists?
-      @path == './' || @universe.include?(@path)
-    end
-
-    def ==(other)
-      @path == other.path && @universe == other.universe
-    end
-
-    def inspect
-      "#{self.class.name}: #{@path}"
-    end
-
-    private
-
-    def new(path)
-      self.class.new(path, @universe, @metadata)
-    end
-
-    def select
-      selected = @universe.select { |entry| yield entry }
-      selected.map { |path| new(path) }
-    end
-
-    def sanitize(path)
-      self.class.sanitize(path)
-    end
-
-    def self.sanitize(path)
-      # It looks like Pathname#new doesn't touch a file system,
-      # neither Pathname#cleanpath does, so it is, hopefully, filesystem safe
-
-      clean_path = Pathname.new(path).cleanpath.to_s
-      raise ArgumentError, 'Invalid path' if clean_path.start_with?('../')
-
-      prefix = './' unless clean_path =~ %r{^[\.|/]}
-      suffix = '/' if path.end_with?('/') || ['.', '..'].include?(clean_path)
-      prefix.to_s + clean_path + suffix.to_s
-    end
-  end
-end