diff options
author | Bob Van Landuyt <bob@gitlab.com> | 2019-08-13 18:41:25 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@gitlab.com> | 2019-08-13 18:41:25 +0300 |
commit | 3702ab7317533896c7455357dd6643181666f22b (patch) | |
tree | cd1a6ff66d7ac489922da4a6a38df6057f5cf172 | |
parent | 530f7f6f0f3172d5712beb0a00c861ffa6935bd7 (diff) | |
parent | afe867921cab046a34bc463840c6e9f5d51f1f70 (diff) |
Merge branch '65803-invalidate-branches-cache-on-refresh' into 'master'
Only expire branch cache once per push
See merge request gitlab-org/gitlab-ce!31653
-rw-r--r-- | app/models/repository.rb | 8 | ||||
-rw-r--r-- | app/services/git/branch_hooks_service.rb | 4 | ||||
-rw-r--r-- | app/workers/post_receive.rb | 3 | ||||
-rw-r--r-- | changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/git_post_receive.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/git_post_receive_spec.rb | 52 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 12 | ||||
-rw-r--r-- | spec/workers/post_receive_spec.rb | 32 |
8 files changed, 109 insertions, 13 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb index a89f573e3d6..58abfaef801 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 c41f445c3c4..431a5aedf2e 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 dba7837bd12..d8dfbc0faf7 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 00000000000..217db8aa05a --- /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 d98b85fecc4..6c7f23a673c 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 00000000000..1911e954df9 --- /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 12dff440ce2..fa243876632 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 39f1beb4efa..081d95d4d79 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 |