Make Repository#has_visible_content more efficient
This commit is contained in:
parent
f43d94a630
commit
403712f06e
|
@ -91,12 +91,6 @@ class Repository
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
# we need to have this method here because it is not cached in ::Git and
|
|
||||||
# the method is called multiple times for every request
|
|
||||||
def has_visible_content?
|
|
||||||
branch_count > 0
|
|
||||||
end
|
|
||||||
|
|
||||||
def inspect
|
def inspect
|
||||||
"#<#{self.class.name}:#{@disk_path}>"
|
"#<#{self.class.name}:#{@disk_path}>"
|
||||||
end
|
end
|
||||||
|
@ -523,9 +517,10 @@ class Repository
|
||||||
delegate :tag_names, to: :raw_repository
|
delegate :tag_names, to: :raw_repository
|
||||||
cache_method :tag_names, fallback: []
|
cache_method :tag_names, fallback: []
|
||||||
|
|
||||||
delegate :branch_count, :tag_count, to: :raw_repository
|
delegate :branch_count, :tag_count, :has_visible_content?, to: :raw_repository
|
||||||
cache_method :branch_count, fallback: 0
|
cache_method :branch_count, fallback: 0
|
||||||
cache_method :tag_count, fallback: 0
|
cache_method :tag_count, fallback: 0
|
||||||
|
cache_method :has_visible_content?, fallback: false
|
||||||
|
|
||||||
def avatar
|
def avatar
|
||||||
# n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/38327
|
# n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/38327
|
||||||
|
|
|
@ -192,6 +192,28 @@ module Gitlab
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def has_local_branches?
|
||||||
|
gitaly_migrate(:has_local_branches) do |is_enabled|
|
||||||
|
if is_enabled
|
||||||
|
gitaly_ref_client.has_local_branches?
|
||||||
|
else
|
||||||
|
has_local_branches_rugged?
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def has_local_branches_rugged?
|
||||||
|
rugged.branches.each(:local).any? do |ref|
|
||||||
|
begin
|
||||||
|
ref.name && ref.target # ensures the branch is valid
|
||||||
|
|
||||||
|
true
|
||||||
|
rescue Rugged::ReferenceError
|
||||||
|
false
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
# Returns the number of valid tags
|
# Returns the number of valid tags
|
||||||
def tag_count
|
def tag_count
|
||||||
gitaly_migrate(:tag_names) do |is_enabled|
|
gitaly_migrate(:tag_names) do |is_enabled|
|
||||||
|
@ -1037,7 +1059,9 @@ module Gitlab
|
||||||
# This method return true if repository contains some content visible in project page.
|
# This method return true if repository contains some content visible in project page.
|
||||||
#
|
#
|
||||||
def has_visible_content?
|
def has_visible_content?
|
||||||
branch_count > 0
|
return @has_visible_content if defined?(@has_visible_content)
|
||||||
|
|
||||||
|
@has_visible_content = has_local_branches?
|
||||||
end
|
end
|
||||||
|
|
||||||
def gitaly_repository
|
def gitaly_repository
|
||||||
|
|
|
@ -57,6 +57,14 @@ module Gitlab
|
||||||
branch_names.count
|
branch_names.count
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# TODO implement a more efficient RPC for this https://gitlab.com/gitlab-org/gitaly/issues/616
|
||||||
|
def has_local_branches?
|
||||||
|
request = Gitaly::FindAllBranchNamesRequest.new(repository: @gitaly_repo)
|
||||||
|
response = GitalyClient.call(@storage, :ref_service, :find_all_branch_names, request).first
|
||||||
|
|
||||||
|
response&.names.present?
|
||||||
|
end
|
||||||
|
|
||||||
def local_branches(sort_by: nil)
|
def local_branches(sort_by: nil)
|
||||||
request = Gitaly::FindLocalBranchesRequest.new(repository: @gitaly_repo)
|
request = Gitaly::FindLocalBranchesRequest.new(repository: @gitaly_repo)
|
||||||
request.sort_by = sort_by_param(sort_by) if sort_by
|
request.sort_by = sort_by_param(sort_by) if sort_by
|
||||||
|
|
|
@ -389,6 +389,40 @@ describe Gitlab::Git::Repository, seed_helper: true do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#has_local_branches?' do
|
||||||
|
shared_examples 'check for local branches' do
|
||||||
|
it { expect(repository.has_local_branches?).to eq(true) }
|
||||||
|
|
||||||
|
context 'mutable' do
|
||||||
|
let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
|
||||||
|
|
||||||
|
after do
|
||||||
|
ensure_seeds
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns false when there are no branches' do
|
||||||
|
# Sanity check
|
||||||
|
expect(repository.has_local_branches?).to eq(true)
|
||||||
|
|
||||||
|
FileUtils.rm_rf(File.join(repository.path, 'packed-refs'))
|
||||||
|
heads_dir = File.join(repository.path, 'refs/heads')
|
||||||
|
FileUtils.rm_rf(heads_dir)
|
||||||
|
FileUtils.mkdir_p(heads_dir)
|
||||||
|
|
||||||
|
expect(repository.has_local_branches?).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with gitaly' do
|
||||||
|
it_behaves_like 'check for local branches'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'without gitaly', skip_gitaly_mock: true do
|
||||||
|
it_behaves_like 'check for local branches'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "#delete_branch" do
|
describe "#delete_branch" do
|
||||||
shared_examples "deleting a branch" do
|
shared_examples "deleting a branch" do
|
||||||
let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
|
let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') }
|
||||||
|
|
|
@ -1131,21 +1131,31 @@ describe Repository do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#has_visible_content?' do
|
describe '#has_visible_content?' do
|
||||||
subject { repository.has_visible_content? }
|
before do
|
||||||
|
# If raw_repository.has_visible_content? gets called more than once then
|
||||||
describe 'when there are no branches' do
|
# caching is broken. We don't want that.
|
||||||
before do
|
expect(repository.raw_repository).to receive(:has_visible_content?)
|
||||||
allow(repository.raw_repository).to receive(:branch_count).and_return(0)
|
.once
|
||||||
end
|
.and_return(result)
|
||||||
|
|
||||||
it { is_expected.to eq(false) }
|
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'when there are branches' do
|
context 'when true' do
|
||||||
it 'returns true' do
|
let(:result) { true }
|
||||||
expect(repository.raw_repository).to receive(:branch_count).and_return(3)
|
|
||||||
|
|
||||||
expect(subject).to eq(true)
|
it 'returns true and caches it' do
|
||||||
|
expect(repository.has_visible_content?).to eq(true)
|
||||||
|
# Second call hits the cache
|
||||||
|
expect(repository.has_visible_content?).to eq(true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when false' do
|
||||||
|
let(:result) { false }
|
||||||
|
|
||||||
|
it 'returns false and caches it' do
|
||||||
|
expect(repository.has_visible_content?).to eq(false)
|
||||||
|
# Second call hits the cache
|
||||||
|
expect(repository.has_visible_content?).to eq(false)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -32,7 +32,7 @@ describe GitGarbageCollectWorker do
|
||||||
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
|
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
|
||||||
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
|
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
|
||||||
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
|
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
|
||||||
expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original
|
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
|
||||||
|
|
||||||
subject.perform(project.id, :gc, lease_key, lease_uuid)
|
subject.perform(project.id, :gc, lease_key, lease_uuid)
|
||||||
end
|
end
|
||||||
|
@ -47,7 +47,6 @@ describe GitGarbageCollectWorker do
|
||||||
expect(subject).not_to receive(:command)
|
expect(subject).not_to receive(:command)
|
||||||
expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
|
expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
|
||||||
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
|
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
|
||||||
expect_any_instance_of(Repository).not_to receive(:branch_count).and_call_original
|
|
||||||
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
|
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
|
||||||
|
|
||||||
subject.perform(project.id, :gc, lease_key, lease_uuid)
|
subject.perform(project.id, :gc, lease_key, lease_uuid)
|
||||||
|
@ -78,7 +77,7 @@ describe GitGarbageCollectWorker do
|
||||||
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
|
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
|
||||||
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
|
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
|
||||||
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
|
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
|
||||||
expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original
|
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
|
||||||
|
|
||||||
subject.perform(project.id)
|
subject.perform(project.id)
|
||||||
end
|
end
|
||||||
|
@ -93,7 +92,6 @@ describe GitGarbageCollectWorker do
|
||||||
expect(subject).not_to receive(:command)
|
expect(subject).not_to receive(:command)
|
||||||
expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
|
expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
|
||||||
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
|
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
|
||||||
expect_any_instance_of(Repository).not_to receive(:branch_count).and_call_original
|
|
||||||
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
|
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
|
||||||
|
|
||||||
subject.perform(project.id)
|
subject.perform(project.id)
|
||||||
|
|
Loading…
Reference in New Issue