Migrate Repository#last_commit_for_path to Gitaly

Closes gitaly#433

GITALY_SERVER_VERSION

-0.26.0
+0.27.0

Gemfile

@@ -391,7 +391,7 @@ gem 'vmstat', '~> 2.3.0'
 gem 'sys-filesystem', '~> 1.1.6'
 # Gitaly GRPC client
-gem 'gitaly', '~> 0.23.0'
+gem 'gitaly', '~> 0.24.0'
 gem 'toml-rb', '~> 0.3.15', require: false

Gemfile.lock

@@ -269,7 +269,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly (0.23.0)
+    gitaly (0.24.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -978,7 +978,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly (~> 0.23.0)
+  gitaly (~> 0.24.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.5.1)

app/models/repository.rb

@@ -613,17 +613,26 @@ class Repository
   end
   def last_commit_for_path(sha, path)
-    sha = last_commit_id_for_path(sha, path)
-    commit(sha)
+    raw_repository.gitaly_migrate(:last_commit_for_path) do |is_enabled|
+      if is_enabled
+        last_commit_for_path_by_gitaly(sha, path)
+      else
+        last_commit_for_path_by_rugged(sha, path)
+      end
+    end
   end
-  # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/383
   def last_commit_id_for_path(sha, path)
     key = path.blank? ? "last_commit_id_for_path:#{sha}" : "last_commit_id_for_path:#{sha}:#{Digest::SHA1.hexdigest(path)}"
     cache.fetch(key) do
-      args = %W(#{Gitlab.config.git.bin_path} rev-list --max-count=1 #{sha} -- #{path})
-      Gitlab::Popen.popen(args, path_to_repo).first.strip
+      raw_repository.gitaly_migrate(:last_commit_for_path) do |is_enabled|
+        if is_enabled
+          last_commit_for_path_by_gitaly(sha, path).id
+        else
+          last_commit_id_for_path_by_shelling_out(sha, path)
+        end
+      end
     end
   end
@@ -1138,6 +1147,21 @@ class Repository
     Rugged::Commit.create(rugged, params)
   end
+  def last_commit_for_path_by_gitaly(sha, path)
+    c = raw_repository.gitaly_commit_client.last_commit_for_path(sha, path)
+    commit(c)
+  end
+
+  def last_commit_for_path_by_rugged(sha, path)
+    sha = last_commit_id_for_path_by_shelling_out(sha, path)
+    commit(sha)
+  end
+
+  def last_commit_id_for_path_by_shelling_out(sha, path)
+    args = %W(#{Gitlab.config.git.bin_path} rev-list --max-count=1 #{sha} -- #{path})
+    Gitlab::Popen.popen(args, path_to_repo).first.strip
+  end
+
   def repository_storage_path
     @project.repository_storage_path
   end

lib/gitlab/git/repository.rb

@@ -683,6 +683,14 @@ module Gitlab
         @gitaly_repository_client ||= Gitlab::GitalyClient::RepositoryService.new(self)
       end
+      def gitaly_migrate(method, &block)
+        Gitlab::GitalyClient.migrate(method, &block)
+      rescue GRPC::NotFound => e
+        raise NoRepository.new(e)
+      rescue GRPC::BadStatus => e
+        raise CommandError.new(e)
+      end
+
       private
       # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'.
@@ -998,6 +1006,11 @@ module Gitlab
         end.sort_by(&:name)
       end
+      def last_commit_for_path_by_rugged(sha, path)
+        sha = last_commit_id_for_path(sha, path)
+        commit(sha)
+      end
+
       def tags_from_gitaly
         gitaly_ref_client.tags
       end
@@ -1017,14 +1030,6 @@ module Gitlab
         raw_output.to_i
       end
-
-      def gitaly_migrate(method, &block)
-        Gitlab::GitalyClient.migrate(method, &block)
-      rescue GRPC::NotFound => e
-        raise NoRepository.new(e)
-      rescue GRPC::BadStatus => e
-        raise CommandError.new(e)
-      end
     end
   end
 end

lib/gitlab/gitaly_client/commit_service.rb

@@ -97,6 +97,20 @@ module Gitlab
         GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count
       end
+      def last_commit_for_path(revision, path)
+        request = Gitaly::LastCommitForPathRequest.new(
+          repository: @gitaly_repo,
+          revision: revision.force_encoding(Encoding::ASCII_8BIT),
+          path: path.to_s.force_encoding(Encoding::ASCII_8BIT)
+        )
+
+        gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request).commit
+        return unless gitaly_commit
+
+        commit = GitalyClient::Commit.new(@repository, gitaly_commit)
+        Gitlab::Git::Commit.new(commit)
+      end
+
       def between(from, to)
         request = Gitaly::CommitsBetweenRequest.new(
           repository: @gitaly_repo,

spec/models/repository_spec.rb

@@ -139,24 +139,44 @@ describe Repository do
   end
   describe '#last_commit_for_path' do
-    subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id }
-    it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') }
+    shared_examples 'getting last commit for path' do
+      subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id }
+      it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') }
+    end
+
+    context 'when Gitaly feature last_commit_for_path is enabled' do
+      it_behaves_like 'getting last commit for path'
+    end
+
+    context 'when Gitaly feature last_commit_for_path is disabled', skip_gitaly_mock: true do
+      it_behaves_like 'getting last commit for path'
+    end
   end
   describe '#last_commit_id_for_path' do
-    subject { repository.last_commit_id_for_path(sample_commit.id, '.gitignore') }
-    it "returns last commit id for a given path" do
-      is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8')
+    shared_examples 'getting last commit ID for path' do
+      subject { repository.last_commit_id_for_path(sample_commit.id, '.gitignore') }
+      it "returns last commit id for a given path" do
+        is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8')
+      end
+
+      it "caches last commit id for a given path" do
+        cache = repository.send(:cache)
+        key = "last_commit_id_for_path:#{sample_commit.id}:#{Digest::SHA1.hexdigest('.gitignore')}"
+
+        expect(cache).to receive(:fetch).with(key).and_return('c1acaa5')
+        is_expected.to eq('c1acaa5')
+      end
     end
-    it "caches last commit id for a given path" do
-      cache = repository.send(:cache)
-      key = "last_commit_id_for_path:#{sample_commit.id}:#{Digest::SHA1.hexdigest('.gitignore')}"
-      expect(cache).to receive(:fetch).with(key).and_return('c1acaa5')
-      is_expected.to eq('c1acaa5')
+    context 'when Gitaly feature last_commit_for_path is enabled' do
+      it_behaves_like 'getting last commit ID for path'
+    end
+    context 'when Gitaly feature last_commit_for_path is disabled', skip_gitaly_mock: true do
+      it_behaves_like 'getting last commit ID for path'
     end
   end