Add latest changes from gitlab-org/gitlab@15-11-stable-ee

6 files changed, 44 insertions, 6 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1e7ff6e8f0e..ebb57f6213f 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -1488,6 +1488,7 @@ class MergeRequest < ApplicationRecord
 
   def fetch_ref!
     target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path)
+    expire_ancestor_cache
   end
 
   # Returns the current merge-ref HEAD commit.
@@ -2046,6 +2047,10 @@ class MergeRequest < ApplicationRecord
     self.draft = draft?
   end
 
+  def expire_ancestor_cache
+    project.repository.expire_ancestor_cache(target_branch_sha, diff_head_sha)
+  end
+
   def missing_report_error(report_type)
     { status: :error, status_reason: "This merge request does not have #{report_type} reports" }
   end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index b4a0eaf0324..62c402f1ae7 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1008,7 +1008,7 @@ class Repository
   def ancestor?(ancestor_id, descendant_id)
     return false if ancestor_id.nil? || descendant_id.nil?
 
-    cache_key = "ancestor:#{ancestor_id}:#{descendant_id}"
+    cache_key = ancestor_cache_key(ancestor_id, descendant_id)
     request_store_cache.fetch(cache_key) do
       cache.fetch(cache_key) do
         raw_repository.ancestor?(ancestor_id, descendant_id)
@@ -1016,6 +1016,12 @@ class Repository
     end
   end
 
+  def expire_ancestor_cache(ancestor_id, descendant_id)
+    cache_key = ancestor_cache_key(ancestor_id, descendant_id)
+    request_store_cache.expire(cache_key)
+    cache.expire(cache_key)
+  end
+
   def clone_as_mirror(url, http_authorization_header: "", resolved_address: "")
     import_repository(url, http_authorization_header: http_authorization_header, mirror: true, resolved_address: resolved_address)
   end
@@ -1236,6 +1242,10 @@ class Repository
 
   private
 
+  def ancestor_cache_key(ancestor_id, descendant_id)
+    "ancestor:#{ancestor_id}:#{descendant_id}"
+  end
+
   # TODO Genericize finder, later split this on finders by Ref or Oid
   # https://gitlab.com/gitlab-org/gitlab/issues/19877
   def find_commit(oid_or_ref)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index f3aa174a964..9460aa5d3c7 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -4261,10 +4261,14 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev
 
     subject { create(:merge_request, source_project: project) }
 
-    it 'fetches the ref correctly' do
+    it 'fetches the ref and expires the ancestor cache' do
       expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error
 
+      expect(project.repository).to receive(:expire_ancestor_cache).with(subject.target_branch_sha, subject.diff_head_sha).and_call_original
+      expect(subject).to receive(:expire_ancestor_cache).and_call_original
+
       subject.fetch_ref!
+
       expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy
     end
   end
@@ -4275,7 +4279,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev
     # We use build instead of create to test that an IID is allocated
     subject { build(:merge_request, source_project: project) }
 
-    it 'fetches the ref correctly' do
+    it 'fetches the ref and expires the ancestor cache' do
+      expect(subject).to receive(:expire_ancestor_cache).and_call_original
       expect(subject.iid).to be_nil
 
       expect { subject.eager_fetch_ref! }.to change { subject.iid.to_i }.by(1)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 72011693e20..ea237768333 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -3132,6 +3132,18 @@ RSpec.describe Repository, feature_category: :source_code_management do
         2.times { repository.ancestor?(commit.id, ancestor.id) }
       end
 
+      it 'calls out to Gitaly again after expiration' do
+        expect(repository.raw_repository).to receive(:ancestor?).once
+
+        repository.ancestor?(commit.id, ancestor.id)
+
+        repository.expire_ancestor_cache(commit.id, ancestor.id)
+
+        expect(repository.raw_repository).to receive(:ancestor?).once
+
+        2.times { repository.ancestor?(commit.id, ancestor.id) }
+      end
+
       it 'returns the value from the request store' do
         repository.__send__(:request_store_cache).write(cache_key, "it's apparent")
 
diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb
index a4b1d8742d0..dab06637c1a 100644
--- a/spec/services/draft_notes/publish_service_spec.rb
+++ b/spec/services/draft_notes/publish_service_spec.rb
@@ -172,7 +172,12 @@ RSpec.describe DraftNotes::PublishService, feature_category: :code_review_workfl
         end
       end
 
-      it 'does not requests a lot from Gitaly', :request_store do
+      it 'does not request a lot from Gitaly', :request_store, :clean_gitlab_redis_cache do
+        merge_request
+        position
+
+        Gitlab::GitalyClient.reset_counts
+
         # NOTE: This should be reduced as we work on reducing Gitaly calls.
         # Gitaly requests shouldn't go above this threshold as much as possible
         # as it may add more to the Gitaly N+1 issue we are experiencing.
diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb
index 4dd05f75a3e..f079e3983d7 100644
--- a/spec/services/merge_requests/reload_diffs_service_spec.rb
+++ b/spec/services/merge_requests/reload_diffs_service_spec.rb
@@ -35,8 +35,9 @@ feature_category: :code_review_workflow do
 
     context 'cache clearing' do
       it 'clears the cache for older diffs on the merge request' do
-        expect_any_instance_of(Redis).to receive(:del).once.and_call_original
-        expect(Rails.cache).to receive(:delete).once.and_call_original
+        expect_next_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff) do |instance|
+          expect(instance).to receive(:clear_cache).and_call_original
+        end
 
         subject.execute
       end