Merge branch 'pks-new-blobs-list-blobs' into 'master'

Implement `#new_blobs` via `#list_blobs`

See merge request gitlab-org/gitlab!68935

Gemfile

@@ -474,7 +474,7 @@ end
 gem 'spamcheck', '~> 0.1.0'
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 14.2.0.pre.rc2'
+gem 'gitaly', '~> 14.3.0.pre.rc1'
 
 # KAS GRPC protocol definitions
 gem 'kas-grpc', '~> 0.0.2'

Gemfile.lock

@@ -453,7 +453,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (14.2.0.pre.rc2)
+    gitaly (14.3.0.pre.rc1)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab (4.16.1)
@@ -1471,7 +1471,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 14.2.0.pre.rc2)
+  gitaly (~> 14.3.0.pre.rc1)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-dangerfiles (~> 2.3.0)

config/feature_flags/development/new_blobs_via_list_blobs.yml

+---
+name: new_blobs_via_list_blobs
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68935
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339637
+milestone: '14.3'
+type: development
+group: group::gitaly
+default_enabled: false

lib/gitlab/git/repository.rb

@@ -364,8 +364,12 @@ module Gitlab
         return [] if newrev.blank? || newrev == ::Gitlab::Git::BLANK_SHA
 
         strong_memoize("new_blobs_#{newrev}") do
-          wrapped_gitaly_errors do
-            gitaly_ref_client.list_new_blobs(newrev, REV_LIST_COMMIT_LIMIT, dynamic_timeout: dynamic_timeout)
+          if Feature.enabled?(:new_blobs_via_list_blobs)
+            blobs(['--not', '--all', '--not', newrev], with_paths: true, dynamic_timeout: dynamic_timeout)
+          else
+            wrapped_gitaly_errors do
+              gitaly_ref_client.list_new_blobs(newrev, REV_LIST_COMMIT_LIMIT, dynamic_timeout: dynamic_timeout)
+            end
           end
         end
       end
@@ -374,13 +378,14 @@ module Gitlab
       # pseudo-revisions `--not` and `--all`. Uses the minimum of
       # GitalyClient.medium_timeout and dynamic timeout if the dynamic
       # timeout is set, otherwise it'll always use the medium timeout.
-      def blobs(revisions, dynamic_timeout: nil)
+      def blobs(revisions, with_paths: false, dynamic_timeout: nil)
         revisions = revisions.reject { |rev| rev.blank? || rev == ::Gitlab::Git::BLANK_SHA }
 
         return [] if revisions.blank?
 
         wrapped_gitaly_errors do
-          gitaly_blob_client.list_blobs(revisions, limit: REV_LIST_COMMIT_LIMIT, dynamic_timeout: dynamic_timeout)
+          gitaly_blob_client.list_blobs(revisions, limit: REV_LIST_COMMIT_LIMIT,
+                                        with_paths: with_paths, dynamic_timeout: dynamic_timeout)
         end
       end
 

lib/gitlab/gitaly_client/blob_service.rb

@@ -19,12 +19,13 @@ module Gitlab
         consume_blob_response(response)
       end
 
-      def list_blobs(revisions, limit: 0, bytes_limit: 0, dynamic_timeout: nil)
+      def list_blobs(revisions, limit: 0, bytes_limit: 0, with_paths: false, dynamic_timeout: nil)
         request = Gitaly::ListBlobsRequest.new(
           repository: @gitaly_repo,
           revisions: Array.wrap(revisions),
           limit: limit,
-          bytes_limit: bytes_limit
+          bytes_limit: bytes_limit,
+          with_paths: with_paths
         )
 
         timeout =

spec/lib/gitlab/git/repository_spec.rb

@@ -936,6 +936,75 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
     end
   end
 
+  describe '#new_blobs' do
+    let(:repository) { mutable_repository }
+    let(:repository_rugged) { mutable_repository_rugged }
+    let(:new_blob) do
+      repository_rugged.write('This is a new blob', :blob)
+    end
+
+    let(:new_commit) do
+      author = { name: 'Test User', email: 'mail@example.com', time: Time.now }
+
+      index = repository_rugged.index
+      index.add(path: 'nested/new-blob.txt', oid: new_blob, mode: 0100644)
+
+      Rugged::Commit.create(repository_rugged,
+                            author: author,
+                            committer: author,
+                            message: "Message",
+                            parents: [],
+                            tree: index.write_tree(repository_rugged))
+    end
+
+    subject { repository.new_blobs(new_commit).to_a }
+
+    context 'with :new_blobs_via_list_blobs enabled' do
+      before do
+        stub_feature_flags(new_blobs_via_list_blobs: true)
+
+        expect_next_instance_of(Gitlab::GitalyClient::BlobService) do |service|
+          expect(service)
+            .to receive(:list_blobs)
+            .with(['--not', '--all', '--not', new_commit],
+                  limit: Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT,
+                  with_paths: true,
+                  dynamic_timeout: nil)
+            .and_call_original
+        end
+      end
+
+      it 'enumerates new blobs' do
+        expect(subject).to match_array(
+          [have_attributes(class: Gitlab::Git::Blob, id: new_blob, path: 'nested/new-blob.txt', size: 18)]
+        )
+      end
+    end
+
+    context 'with :new_blobs_via_list_blobs disabled' do
+      before do
+        stub_feature_flags(new_blobs_via_list_blobs: false)
+
+        expect_next_instance_of(Gitlab::GitalyClient::RefService) do |service|
+          expect(service)
+            .to receive(:list_new_blobs)
+            .with(new_commit,
+                  Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT,
+                  dynamic_timeout: nil)
+            .and_call_original
+        end
+      end
+
+      it 'enumerates new blobs' do
+        expect(subject).to match_array([Gitaly::NewBlobObject.new(
+          size: 18,
+          oid: new_blob,
+          path: "nested/new-blob.txt"
+        )])
+      end
+    end
+  end
+
   describe '#new_commits' do
     let(:repository) { mutable_repository }
     let(:new_commit) do

spec/lib/gitlab/gitaly_client/blob_service_spec.rb

@@ -92,13 +92,14 @@ RSpec.describe Gitlab::GitalyClient::BlobService do
   describe '#list_blobs' do
     let(:limit) { 0 }
     let(:bytes_limit) { 0 }
-    let(:expected_params) { { revisions: revisions, limit: limit, bytes_limit: bytes_limit } }
+    let(:with_paths) { false }
+    let(:expected_params) { { revisions: revisions, limit: limit, bytes_limit: bytes_limit, with_paths: with_paths } }
 
     before do
       ::Gitlab::GitalyClient.clear_stubs!
     end
 
-    subject { client.list_blobs(revisions, limit: limit, bytes_limit: bytes_limit) }
+    subject { client.list_blobs(revisions, limit: limit, bytes_limit: bytes_limit, with_paths: with_paths) }
 
     context 'with a single revision' do
       let(:revisions) { ['master'] }
@@ -147,6 +148,24 @@ RSpec.describe Gitlab::GitalyClient::BlobService do
       end
     end
 
+    context 'with paths' do
+      let(:revisions) { ['master'] }
+      let(:limit) { 10 }
+      let(:bytes_lmit) { 1024 }
+      let(:with_paths) { true }
+
+      it 'sends a list_blobs message' do
+        expect_next_instance_of(Gitaly::BlobService::Stub) do |service|
+          expect(service)
+            .to receive(:list_blobs)
+            .with(gitaly_request_with_params(expected_params), kind_of(Hash))
+            .and_return([])
+        end
+
+        subject
+      end
+    end
+
     context 'with split contents' do
       let(:revisions) { ['master'] }