Invalidate branches cache on PostReceive

Whenever `PostReceive` is enqueued, `UpdateMergeRequestsWorker`
is enqueued and `MergeRequests::RefreshService` is called, it'll
check if the source branch of each MR asssociated to the push exists
or not via `MergeRequest#source_branch_exists?`. The said method will
call `Repository#branch_exists?` which is cached in `Rails.cache`.

When the cache contains outdated data and the source branch actually
exists, the `MergeRequests#RefreshService` job will close associated
MRs which is not correct.

The fix is to expire the branches cache of the project so we have
updated data during the post receive hook which will help in the
accuracy of the check if we need to close associated MRs or not.

app/models/repository.rb

@@ -460,8 +460,8 @@ class Repository
   end
   # Runs code after a new branch has been created.
-  def after_create_branch
-    expire_branches_cache
+  def after_create_branch(expire_cache: true)
+    expire_branches_cache if expire_cache
     repository_event(:push_branch)
   end
@@ -474,8 +474,8 @@ class Repository
   end
   # Runs code after an existing branch has been removed.
-  def after_remove_branch
-    expire_branches_cache
+  def after_remove_branch(expire_cache: true)
+    expire_branches_cache if expire_cache
   end
   def method_missing(msg, *args, &block)

app/services/git/branch_hooks_service.rb

@@ -63,7 +63,7 @@ module Git
     end
     def branch_create_hooks
-      project.repository.after_create_branch
+      project.repository.after_create_branch(expire_cache: false)
       project.after_create_default_branch if default_branch?
     end
@@ -78,7 +78,7 @@ module Git
     end
     def branch_remove_hooks
-      project.repository.after_remove_branch
+      project.repository.after_remove_branch(expire_cache: false)
     end
     # Schedules processing of commit messages

app/workers/post_receive.rb

@@ -42,6 +42,9 @@ class PostReceive
     user = identify_user(post_received)
     return false unless user
+    # Expire the branches cache so we have updated data for this push
+    post_received.project.repository.expire_branches_cache if post_received.branches_exist?
+
     post_received.enum_for(:changes_refs).with_index do |(oldrev, newrev, ref), index|
       service_klass =
         if Gitlab::Git.tag_ref?(ref)

changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml

+---
+title: Invalidate branches cache on PostReceive
+merge_request: 31653
+author:
+type: fixed

lib/gitlab/git_post_receive.rb

@@ -27,6 +27,14 @@ module Gitlab
       end
     end
+    def branches_exist?
+      changes_refs do |_oldrev, _newrev, ref|
+        return true if Gitlab::Git.branch_ref?(ref) # rubocop:disable Cop/AvoidReturnFromBlocks
+      end
+
+      false
+    end
+
     private
     def deserialize_changes(changes)

spec/lib/gitlab/git_post_receive_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Gitlab::GitPostReceive do
+  let(:project) { create(:project) }
+
+  subject { described_class.new(project, "project-#{project.id}", changes.dup, {}) }
+
+  describe '#branches_exist?' do
+    context 'with no branches' do
+      let(:changes) do
+        <<~EOF
+          654321 210987 refs/nobranches/tag1
+          654322 210986 refs/tags/test1
+          654323 210985 refs/merge-requests/mr1
+        EOF
+      end
+
+      it 'returns false' do
+        expect(subject.branches_exist?).to be_falsey
+      end
+    end
+
+    context 'with branches' do
+      let(:changes) do
+        <<~EOF
+          654322 210986 refs/heads/test1
+          654321 210987 refs/tags/tag1
+          654323 210985 refs/merge-requests/mr1
+        EOF
+      end
+
+      it 'returns true' do
+        expect(subject.branches_exist?).to be_truthy
+      end
+    end
+
+    context 'with malformed changes' do
+      let(:changes) do
+        <<~EOF
+          ref/heads/1 a
+          somebranch refs/heads/2
+        EOF
+      end
+
+      it 'returns false' do
+        expect(subject.branches_exist?).to be_falsey
+      end
+    end
+  end
+end

spec/models/repository_spec.rb

@@ -1781,6 +1781,12 @@ describe Repository do
       repository.after_create_branch
     end
+
+    it 'does not expire the branch caches when specified' do
+      expect(repository).not_to receive(:expire_branches_cache)
+
+      repository.after_create_branch(expire_cache: false)
+    end
   end
   describe '#after_remove_branch' do
@@ -1789,6 +1795,12 @@ describe Repository do
       repository.after_remove_branch
     end
+
+    it 'does not expire the branch caches when specified' do
+      expect(repository).not_to receive(:expire_branches_cache)
+
+      repository.after_remove_branch(expire_cache: false)
+    end
   end
   describe '#after_create' do

spec/workers/post_receive_spec.rb

@@ -57,12 +57,19 @@ describe PostReceive do
     context 'with changes' do
       before do
         allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner)
+        allow(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT])
       end
       context "branches" do
-        let(:changes) { "123456 789012 refs/heads/tést" }
-        it "calls Git::BranchPushService" do
+        let(:changes) { '123456 789012 refs/heads/tést' }
+
+        it 'expires the branches cache' do
+          expect(project.repository).to receive(:expire_branches_cache)
+
+          described_class.new.perform(gl_repository, key_id, base64_changes)
+        end
+        it 'calls Git::BranchPushService' do
           expect_next_instance_of(Git::BranchPushService) do |service|
             expect(service).to receive(:execute).and_return(true)
           end
@@ -73,16 +80,22 @@ describe PostReceive do
         end
       end
-      context "tags" do
-        let(:changes) { "123456 789012 refs/tags/tag" }
-        it "calls Git::TagPushService" do
-          expect(Git::BranchPushService).not_to receive(:execute)
+      context 'tags' do
+        let(:changes) { '123456 789012 refs/tags/tag' }
+        it 'does not expire branches cache' do
+          expect(project.repository).not_to receive(:expire_branches_cache)
+          described_class.new.perform(gl_repository, key_id, base64_changes)
+        end
+
+        it 'calls Git::TagPushService' do
           expect_next_instance_of(Git::TagPushService) do |service|
             expect(service).to receive(:execute).and_return(true)
           end
+          expect(Git::BranchPushService).not_to receive(:new)
+
           described_class.new.perform(gl_repository, key_id, base64_changes)
         end
       end