Merge branch 'id-stub-keeping-around' into 'master'

Stub keeping commits around in tests

See merge request gitlab-org/gitlab!16653

app/models/repository.rb

@@ -260,28 +260,8 @@ class Repository
     raw_repository.languages(root_ref)
   end
 
-  # Makes sure a commit is kept around when Git garbage collection runs.
-  # Git GC will delete commits from the repository that are no longer in any
-  # branches or tags, but we want to keep some of these commits around, for
-  # example if they have comments or CI builds.
-  #
-  # For Geo's sake, pass in multiple shas rather than calling it multiple times,
-  # to avoid unnecessary syncing.
   def keep_around(*shas)
-    shas.each do |sha|
-      next unless sha.present? && commit_by(oid: sha)
-
-      next if kept_around?(sha)
-
-      # This will still fail if the file is corrupted (e.g. 0 bytes)
-      raw_repository.write_ref(keep_around_ref_name(sha), sha)
-    rescue Gitlab::Git::CommandError => ex
-      Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}" # rubocop:disable Gitlab/RailsLogger
-    end
-  end
-
-  def kept_around?(sha)
-    ref_exists?(keep_around_ref_name(sha))
+    Gitlab::Git::KeepAround.execute(self, shas)
   end
 
   def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:, path: nil)
@@ -1157,10 +1137,6 @@ class Repository
     end
   end
 
-  def keep_around_ref_name(sha)
-    "refs/#{REF_KEEP_AROUND}/#{sha}"
-  end
-
   def repository_event(event, tags = {})
     Gitlab::Metrics.add_event(event, tags)
   end

lib/gitlab/git/keep_around.rb

+# frozen_string_literal: true
+
+# Makes sure a commit is kept around when Git garbage collection runs.
+# Git GC will delete commits from the repository that are no longer in any
+# branches or tags, but we want to keep some of these commits around, for
+# example if they have comments or CI builds.
+#
+# For Geo's sake, pass in multiple shas rather than calling it multiple times,
+# to avoid unnecessary syncing.
+module Gitlab
+  module Git
+    class KeepAround
+      def self.execute(repository, shas)
+        new(repository).execute(shas)
+      end
+
+      def initialize(repository)
+        @repository = repository
+      end
+
+      def execute(shas)
+        shas.each do |sha|
+          next unless sha.present? && commit_by(oid: sha)
+
+          next if kept_around?(sha)
+
+          # This will still fail if the file is corrupted (e.g. 0 bytes)
+          raw_repository.write_ref(keep_around_ref_name(sha), sha)
+        rescue Gitlab::Git::CommandError => ex
+          Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}" # rubocop:disable Gitlab/RailsLogger
+        end
+      end
+
+      def kept_around?(sha)
+        ref_exists?(keep_around_ref_name(sha))
+      end
+
+      delegate :commit_by, :raw_repository, :ref_exists?, :disk_path, to: :@repository
+      private :commit_by, :raw_repository, :ref_exists?, :disk_path
+
+      private
+
+      def keep_around_ref_name(sha)
+        "refs/#{::Repository::REF_KEEP_AROUND}/#{sha}"
+      end
+    end
+  end
+end

spec/lib/gitlab/git/keep_around_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Git::KeepAround do
+  include RepoHelpers
+
+  let(:repository) { create(:project, :repository).repository }
+  let(:service) { described_class.new(repository) }
+
+  it "does not fail if we attempt to reference bad commit" do
+    expect(service.kept_around?('abc1234')).to be_falsey
+  end
+
+  it "stores a reference to the specified commit sha so it isn't garbage collected" do
+    service.execute([sample_commit.id])
+
+    expect(service.kept_around?(sample_commit.id)).to be_truthy
+  end
+
+  it "attempting to call keep around on truncated ref does not fail" do
+    service.execute([sample_commit.id])
+    ref = service.send(:keep_around_ref_name, sample_commit.id)
+
+    path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      File.join(repository.path, ref)
+    end
+    # Corrupt the reference
+    File.truncate(path, 0)
+
+    expect(service.kept_around?(sample_commit.id)).to be_falsey
+
+    service.execute([sample_commit.id])
+
+    expect(service.kept_around?(sample_commit.id)).to be_falsey
+
+    File.delete(path)
+  end
+
+  context 'for multiple SHAs' do
+    it 'skips non-existent SHAs' do
+      service.execute(['aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', sample_commit.id])
+
+      expect(service.kept_around?(sample_commit.id)).to be_truthy
+    end
+
+    it 'skips already-kept-around SHAs' do
+      service.execute([sample_commit.id])
+
+      expect(repository.raw_repository).to receive(:write_ref).exactly(1).and_call_original
+
+      service.execute([sample_commit.id, another_sample_commit.id])
+
+      expect(service.kept_around?(another_sample_commit.id)).to be_truthy
+    end
+  end
+end

spec/models/note_spec.rb

@@ -73,6 +73,10 @@ describe Note do
   end
 
   describe "Commit notes" do
+    before do
+      allow(Gitlab::Git::KeepAround).to receive(:execute).and_call_original
+    end
+
     let!(:note) { create(:note_on_commit, note: "+1 from me") }
     let!(:commit) { note.noteable }
 
@@ -92,7 +96,9 @@ describe Note do
     end
 
     it "keeps the commit around" do
-      expect(note.project.repository.kept_around?(commit.id)).to be_truthy
+      repo = note.project.repository
+
+      expect(repo.ref_exists?("refs/keep-around/#{commit.id}")).to be_truthy
     end
 
     it 'does not generate N+1 queries for participants', :request_store do

spec/models/repository_spec.rb

@@ -2040,55 +2040,6 @@ describe Repository do
     end
   end
 
-  describe "#keep_around" do
-    it "does not fail if we attempt to reference bad commit" do
-      expect(repository.kept_around?('abc1234')).to be_falsey
-    end
-
-    it "stores a reference to the specified commit sha so it isn't garbage collected" do
-      repository.keep_around(sample_commit.id)
-
-      expect(repository.kept_around?(sample_commit.id)).to be_truthy
-    end
-
-    it "attempting to call keep_around on truncated ref does not fail" do
-      repository.keep_around(sample_commit.id)
-      ref = repository.send(:keep_around_ref_name, sample_commit.id)
-
-      path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-        File.join(repository.path, ref)
-      end
-      # Corrupt the reference
-      File.truncate(path, 0)
-
-      expect(repository.kept_around?(sample_commit.id)).to be_falsey
-
-      repository.keep_around(sample_commit.id)
-
-      expect(repository.kept_around?(sample_commit.id)).to be_falsey
-
-      File.delete(path)
-    end
-
-    context 'for multiple SHAs' do
-      it 'skips non-existent SHAs' do
-        repository.keep_around('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', sample_commit.id)
-
-        expect(repository.kept_around?(sample_commit.id)).to be_truthy
-      end
-
-      it 'skips already-kept-around SHAs' do
-        repository.keep_around(sample_commit.id)
-
-        expect(repository.raw_repository).to receive(:write_ref).exactly(1).and_call_original
-
-        repository.keep_around(sample_commit.id, another_sample_commit.id)
-
-        expect(repository.kept_around?(another_sample_commit.id)).to be_truthy
-      end
-    end
-  end
-
   describe '#contribution_guide', :use_clean_rails_memory_store_caching do
     it 'returns and caches the output' do
       expect(repository).to receive(:file_on_head)

spec/spec_helper.rb

@@ -151,11 +151,13 @@ RSpec.configure do |config|
       .with(:force_autodevops_on_by_default, anything)
       .and_return(false)
 
-    # Stub this call due to being an expensive operation
+    # Stub these calls due to being expensive operations
     # It can be reenabled for specific tests via:
     #
     # allow(DetectRepositoryLanguagesWorker).to receive(:perform_async).and_call_original
+    # allow(Gitlab::Git::KeepAround).to receive(:execute).and_call_original
     allow(DetectRepositoryLanguagesWorker).to receive(:perform_async).and_return(true)
+    allow(Gitlab::Git::KeepAround).to receive(:execute)
 
     Gitlab::ThreadMemoryCache.cache_backend.clear
   end