Client implementation for InfoAttributes

Clients can now request the attributes from `$GIT_DIR/info/attributes`
through Gitaly. The Gitaly migration is described in gitlab-org/gitaly#1082.

The parser algorithm was implemented in a way it could handle both file
contents or a File handle, and both were already tested.

Other than that, using the boy scout rule, I've removed a class,
InfoAttributes, as it was delegating everything to the parser and
therefor wasn't really needed in my opinion.

7 files changed, 46 insertions, 114 deletions

diff --git a/lib/gitlab/git/attributes_parser.rb b/lib/gitlab/git/attributes_parser.rb
index d8aeabb6cba..08f4d7d4f5c 100644
--- a/lib/gitlab/git/attributes_parser.rb
+++ b/lib/gitlab/git/attributes_parser.rb
@@ -3,12 +3,8 @@ module Gitlab
     # Class for parsing Git attribute files and extracting the attributes for
     # file patterns.
     class AttributesParser
-      def initialize(attributes_data)
+      def initialize(attributes_data = "")
         @data = attributes_data || ""
-
-        if @data.is_a?(File)
-          @patterns = parse_file
-        end
       end
 
       # Returns all the Git attributes for the given path.
@@ -28,7 +24,7 @@ module Gitlab
 
       # Returns a Hash containing the file patterns and their attributes.
       def patterns
-        @patterns ||= parse_file
+        @patterns ||= parse_data
       end
 
       # Parses an attribute string.
@@ -91,8 +87,8 @@ module Gitlab
 
       private
 
-      # Parses the Git attributes file.
-      def parse_file
+      # Parses the Git attributes file contents.
+      def parse_data
         pairs = []
         comment = '#'
 
diff --git a/lib/gitlab/git/info_attributes.rb b/lib/gitlab/git/info_attributes.rb
deleted file mode 100644
index e79a440950b..00000000000
--- a/lib/gitlab/git/info_attributes.rb
+++ /dev/null
@@ -1,49 +0,0 @@
-# Gitaly note: JV: not sure what to make of this class. Why does it use
-# the full disk path of the repository to look up attributes This is
-# problematic in Gitaly, because Gitaly hides the full disk path to the
-# repository from gitlab-ce.
-
-module Gitlab
-  module Git
-    # Parses gitattributes at `$GIT_DIR/info/attributes`
-    #
-    # Unlike Rugged this parser only needs a single IO call (a call to `open`),
-    # vastly reducing the time spent in extracting attributes.
-    #
-    # This class _only_ supports parsing the attributes file located at
-    # `$GIT_DIR/info/attributes` as GitLab doesn't use any other files
-    # (`.gitattributes` is copied to this particular path).
-    #
-    # Basic usage:
-    #
-    #     attributes = Gitlab::Git::InfoAttributes.new(some_repo.path)
-    #
-    #     attributes.attributes('README.md') # => { "eol" => "lf }
-    class InfoAttributes
-      delegate :attributes, :patterns, to: :parser
-
-      # path - The path to the Git repository.
-      def initialize(path)
-        @repo_path = File.expand_path(path)
-      end
-
-      def parser
-        @parser ||= begin
-          if File.exist?(attributes_path)
-            File.open(attributes_path, 'r') do |file_handle|
-              AttributesParser.new(file_handle)
-            end
-          else
-            AttributesParser.new("")
-          end
-        end
-      end
-
-      private
-
-      def attributes_path
-        @attributes_path ||= File.join(@repo_path, 'info/attributes')
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index f1b575bd872..aa429090816 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -105,7 +105,6 @@ module Gitlab
         )
         @path = File.join(storage_path, @relative_path)
         @name = @relative_path.split("/").last
-        @attributes = Gitlab::Git::InfoAttributes.new(path)
       end
 
       def ==(other)
@@ -993,11 +992,32 @@ module Gitlab
         raise InvalidRef
       end
 
+      def info_attributes
+        return @info_attributes if @info_attributes
+
+        content =
+          gitaly_migrate(:get_info_attributes) do |is_enabled|
+            if is_enabled
+              gitaly_repository_client.info_attributes
+            else
+              attributes_path = File.join(File.expand_path(@path), 'info', 'attributes')
+
+              if File.exist?(attributes_path)
+                File.read(attributes_path)
+              else
+                ""
+              end
+            end
+          end
+
+        @info_attributes = AttributesParser.new(content)
+      end
+
       # Returns the Git attributes for the given file path.
       #
       # See `Gitlab::Git::Attributes` for more information.
       def attributes(path)
-        @attributes.attributes(path)
+        info_attributes.attributes(path)
       end
 
       def gitattribute(path, name)
diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb
index 6441065f5fe..39057beefba 100644
--- a/lib/gitlab/gitaly_client/repository_service.rb
+++ b/lib/gitlab/gitaly_client/repository_service.rb
@@ -50,6 +50,15 @@ module Gitlab
         GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request)
       end
 
+      def info_attributes
+        request = Gitaly::GetInfoAttributesRequest.new(repository: @gitaly_repo)
+
+        response = GitalyClient.call(@storage, :repository_service, :get_info_attributes, request)
+        response.each_with_object("") do |message, attributes|
+          attributes << message.attributes
+        end
+      end
+
       def fetch_remote(remote, ssh_auth:, forced:, no_tags:, timeout:, prune: true)
         request = Gitaly::FetchRemoteRequest.new(
           repository: @gitaly_repo, remote: remote, force: forced,
diff --git a/spec/lib/gitlab/git/attributes_parser_spec.rb b/spec/lib/gitlab/git/attributes_parser_spec.rb
index 323334e99a5..2d103123998 100644
--- a/spec/lib/gitlab/git/attributes_parser_spec.rb
+++ b/spec/lib/gitlab/git/attributes_parser_spec.rb
@@ -66,18 +66,6 @@ describe Gitlab::Git::AttributesParser, seed_helper: true do
       end
     end
 
-    context 'when attributes data is a file handle' do
-      subject do
-        File.open(attributes_path, 'r') do |file_handle|
-          described_class.new(file_handle)
-        end
-      end
-
-      it 'returns the attributes as a Hash' do
-        expect(subject.attributes('test.txt')).to eq({ 'text' => true })
-      end
-    end
-
     context 'when attributes data is nil' do
       let(:data) { nil }
 
diff --git a/spec/lib/gitlab/git/info_attributes_spec.rb b/spec/lib/gitlab/git/info_attributes_spec.rb
deleted file mode 100644
index ea84909c3e0..00000000000
--- a/spec/lib/gitlab/git/info_attributes_spec.rb
+++ /dev/null
@@ -1,43 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Git::InfoAttributes, seed_helper: true do
-  let(:path) do
-    File.join(SEED_STORAGE_PATH, 'with-git-attributes.git')
-  end
-
-  subject { described_class.new(path) }
-
-  describe '#attributes' do
-    context 'using a path with attributes' do
-      it 'returns the attributes as a Hash' do
-        expect(subject.attributes('test.txt')).to eq({ 'text' => true })
-      end
-
-      it 'returns an empty Hash for a defined path without attributes' do
-        expect(subject.attributes('bla/bla.txt')).to eq({})
-      end
-    end
-  end
-
-  describe '#parser' do
-    it 'parses a file with entries' do
-      expect(subject.patterns).to be_an_instance_of(Hash)
-      expect(subject.patterns["/*.txt"]).to eq({ 'text' => true })
-    end
-
-    it 'does not parse anything when the attributes file does not exist' do
-      expect(File).to receive(:exist?)
-        .with(File.join(path, 'info/attributes'))
-        .and_return(false)
-
-      expect(subject.patterns).to eq({})
-    end
-
-    it 'does not parse attributes files with unsupported encoding' do
-      path = File.join(SEED_STORAGE_PATH, 'with-invalid-git-attributes.git')
-      subject = described_class.new(path)
-
-      expect(subject.patterns).to eq({})
-    end
-  end
-end
diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb
index 21592688bf0..074323d47d2 100644
--- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb
@@ -84,6 +84,17 @@ describe Gitlab::GitalyClient::RepositoryService do
     end
   end
 
+  describe '#info_attributes' do
+    it 'reads the info attributes' do
+      expect_any_instance_of(Gitaly::RepositoryService::Stub)
+        .to receive(:get_info_attributes)
+        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+        .and_return([])
+
+      client.info_attributes
+    end
+  end
+
   describe '#has_local_branches?' do
     it 'sends a has_local_branches message' do
       expect_any_instance_of(Gitaly::RepositoryService::Stub)