diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 97a1b0d554c39baf9891edcd42f4bacb7f1aff00..4568de6e9de3d190f53286e3c94c3fa10e0ae39d 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -29,6 +29,10 @@ def index fetch_branches_by_mode fetch_merge_requests_for_branches + @branches.each do |branch| + Gitlab::Git::RefPreloader.collect_ref(@project.id, Gitlab::Git::BRANCH_REF_PREFIX + branch.name) + end + @refs_pipelines = @project.ci_pipelines.latest_successful_for_refs(@branches.map(&:name)) @merged_branch_names = repository.merged_branch_names(@branches.map(&:name), include_identical: true) @branch_pipeline_statuses = Ci::CommitStatusesFinder.new(@project, repository, current_user, @branches).execute diff --git a/app/models/repository.rb b/app/models/repository.rb index a76ef5de4f727399a8c6cde3a201f06845a4b73e..4f8910b0c2240e81fd367f1089556596c3b1997d 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -329,6 +329,9 @@ def branch_exists?(branch_name) return false unless exists? return false if branch_name.blank? + # Optimization: Use a root_ref cache to check the presence of the default branch + return true if branch_name == root_ref + Gitlab::Git::RefPreloader.preload_refs_for_project(project) return lazy_ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + branch_name).itself @@ -577,7 +580,10 @@ def before_remove_branch # Runs code after an existing branch has been removed. def after_remove_branch(expire_cache: true) - expire_branches_cache if expire_cache + if expire_cache + expire_branches_cache + expire_root_ref_cache + end end def lookup(sha) diff --git a/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb b/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb index e8e6550a7234529952cee6e15f9eeca37e9e59ea..d630fceff8e87c21dbd10335d6237eebf90ba616 100644 --- a/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb +++ b/ee/spec/graphql/types/dast_scanner_profile_type_spec.rb @@ -95,8 +95,8 @@ create_file_in_repo(policies_project, 'master', 'master', Security::OrchestrationPolicyConfiguration::POLICY_PATH, policy_yaml) end - it 'only calls Gitaly twice when multiple profiles are present', :request_store do - expect { response }.to change { Gitlab::GitalyClient.get_request_count }.by(2) + it 'only calls Gitaly once when multiple profiles are present', :request_store do + expect { response }.to change { Gitlab::GitalyClient.get_request_count }.by(1) end end end diff --git a/ee/spec/graphql/types/dast_site_profile_type_spec.rb b/ee/spec/graphql/types/dast_site_profile_type_spec.rb index 6a886be2885f7adcaefface56c15022e4973b5c3..e83ee65e2f023a684a778aa29ffc36a2924116ef 100644 --- a/ee/spec/graphql/types/dast_site_profile_type_spec.rb +++ b/ee/spec/graphql/types/dast_site_profile_type_spec.rb @@ -175,8 +175,8 @@ create_file_in_repo(policies_project, 'master', 'master', Security::OrchestrationPolicyConfiguration::POLICY_PATH, policy_yaml) end - it 'only calls Gitaly twice when multiple profiles are present', :request_store do - expect { response }.to change { Gitlab::GitalyClient.get_request_count }.by(2) + it 'only calls Gitaly once when multiple profiles are present', :request_store do + expect { response }.to change { Gitlab::GitalyClient.get_request_count }.by(1) end end end diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index a35365cbdf4be72a2fb547760a14c546ffb5d7c6..7ae635c38fa29c8b77da903f47f93f0dd6dd2b53 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -569,12 +569,14 @@ def destroy_all_merged # was not raised whenever the cache is enabled yet cold. context 'when cache is enabled yet cold', :request_store do it 'return with a status 200' do - get :index, format: :html, params: { - namespace_id: project.namespace, - project_id: project, - sort: 'name_asc', - state: 'all' - } + expect do + get :index, format: :html, params: { + namespace_id: project.namespace, + project_id: project, + sort: 'name_asc', + state: 'all' + } + end.to change { Gitlab::GitalyClient.get_request_count }.by(10) expect(response).to have_gitlab_http_status(:ok) expect(assigns[:sort]).to eq('name_asc') diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 9df549874b111e807224893af273deced8a3d041..281a94d887bf44bdeee40b4d2d6ef61f0d933f3e 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -79,7 +79,7 @@ # ListCommitsByOid, RepositoryExists, HasLocalBranches, ListCommitsByRefNames, ListRefs expect { get_pipelines_index_json } - .to change { Gitlab::GitalyClient.get_request_count }.by(5) + .to change { Gitlab::GitalyClient.get_request_count }.by(6) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index bfc04966d06ed2875640937ac0d26c9fd2010d14..0e01a1dc6fb3e82e4b30a130aef21bfdffa78ded 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2020,7 +2020,7 @@ def expect_to_raise_storage_error end describe '#branch_exists?' do - let(:branch) { repository.root_ref } + let(:branch) { 'feature' } subject { repository.branch_exists?(branch) } @@ -2030,6 +2030,16 @@ def expect_to_raise_storage_error is_expected.to eq(true) end + context 'when default branch is provided' do + let(:branch) { repository.root_ref } + + it 'does not make list_refs query and rely on root_ref cache' do + expect(repository).not_to receive(:list_refs).with(["refs/heads/#{branch}"]) + + is_expected.to eq(true) + end + end + context 'when "ref_existence_check_gitaly" is disabled' do before do stub_feature_flags(ref_existence_check_gitaly: false)