diff --git a/app/models/repository.rb b/app/models/repository.rb index a89f573e3d6694be589175c0f6ce85e019f0008a..58abfaef80162bb1eff1722e78235a4ac1729d28 100644 --- a/app/models/repository.rb +++ b/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) diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index c41f445c3c4e47cf929002f8832607ab3fe85570..431a5aedf2eb6b174120a0c94b590ab01c8c07cb 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/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 diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index dba7837bd128936f0321eeb99cc156c002ef4753..d8dfbc0faf79e7cdbc505ee197df9811071028db 100644 --- a/app/workers/post_receive.rb +++ b/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.includes_branches? + post_received.enum_for(:changes_refs).with_index do |(oldrev, newrev, ref), index| service_klass = if Gitlab::Git.tag_ref?(ref) diff --git a/changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml b/changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml new file mode 100644 index 0000000000000000000000000000000000000000..217db8aa05af408ce12d8251a9b076466f920a58 --- /dev/null +++ b/changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml @@ -0,0 +1,5 @@ +--- +title: Invalidate branches cache on PostReceive +merge_request: 31653 +author: +type: fixed diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index d98b85fecc4a354d3831df8c6b83e47673f3a7d2..6c7f23a673c315e33bc323c6f69e7708a1c36e87 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -27,6 +27,12 @@ module Gitlab end end + def includes_branches? + enum_for(:changes_refs).any? do |_oldrev, _newrev, ref| + Gitlab::Git.branch_ref?(ref) + end + end + private def deserialize_changes(changes) diff --git a/spec/lib/gitlab/git_post_receive_spec.rb b/spec/lib/gitlab/git_post_receive_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..1911e954df92c533007699353da6215270a8bf76 --- /dev/null +++ b/spec/lib/gitlab/git_post_receive_spec.rb @@ -0,0 +1,52 @@ +# 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 '#includes_branches?' 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.includes_branches?).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.includes_branches?).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.includes_branches?).to be_falsey + end + end + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 12dff440ce270c6063f2fadc9a3c17d3b6055b3e..fa24387663229458dfa44be132c29837e138e9ff 100644 --- a/spec/models/repository_spec.rb +++ b/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 diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 39f1beb4efaccdfcfef4eb479706224d1c4cae21..081d95d4d79be9c44a90680363a95026f849f303 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -57,13 +57,25 @@ 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" } + let(:changes) do + <<~EOF + '123456 789012 refs/heads/tést1' + '123456 789012 refs/heads/tést2' + EOF + end - it "calls Git::BranchPushService" do - expect_next_instance_of(Git::BranchPushService) do |service| + it 'expires the branches cache' do + expect(project.repository).to receive(:expire_branches_cache).once + + described_class.new.perform(gl_repository, key_id, base64_changes) + end + + it 'calls Git::BranchPushService' do + expect_any_instance_of(Git::BranchPushService) do |service| expect(service).to receive(:execute).and_return(true) end @@ -73,16 +85,22 @@ describe PostReceive do end end - context "tags" do - let(:changes) { "123456 789012 refs/tags/tag" } + 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) - it "calls Git::TagPushService" do - expect(Git::BranchPushService).not_to receive(:execute) + 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