Remove Rugged::Repository#empty?

This commit is contained in:
Zeger-Jan van de Weg 2017-12-07 15:33:30 +00:00 committed by Sean McGivern
parent b756b0af2b
commit 03ac8d5d0b
15 changed files with 127 additions and 171 deletions

View file

@ -227,7 +227,6 @@ class Project < ActiveRecord::Base
delegate :members, to: :team, prefix: true
delegate :add_user, :add_users, to: :team
delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
delegate :empty_repo?, to: :repository
# Validations
validates :creator, presence: true, on: :create
@ -499,6 +498,10 @@ class Project < ActiveRecord::Base
auto_devops&.enabled.nil? && !current_application_settings.auto_devops_enabled?
end
def empty_repo?
repository.empty?
end
def repository_storage_path
Gitlab.config.repositories.storages[repository_storage].try(:[], 'path')
end

View file

@ -37,7 +37,7 @@ class Repository
issue_template_names merge_request_template_names).freeze
# Methods that use cache_method but only memoize the value
MEMOIZED_CACHED_METHODS = %i(license empty_repo?).freeze
MEMOIZED_CACHED_METHODS = %i(license).freeze
# Certain method caches should be refreshed when certain types of files are
# changed. This Hash maps file types (as returned by Gitlab::FileDetector) to
@ -497,7 +497,11 @@ class Repository
end
cache_method :exists?
delegate :empty?, to: :raw_repository
def empty?
return true unless exists?
!has_visible_content?
end
cache_method :empty?
# The size of this repository in megabytes.
@ -944,13 +948,8 @@ class Repository
end
end
def empty_repo?
!exists? || !has_visible_content?
end
cache_method :empty_repo?, memoize_only: true
def search_files_by_content(query, ref)
return [] if empty_repo? || query.blank?
return [] if empty? || query.blank?
offset = 2
args = %W(grep -i -I -n --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
@ -959,7 +958,7 @@ class Repository
end
def search_files_by_name(query, ref)
return [] if empty_repo? || query.blank?
return [] if empty? || query.blank?
args = %W(ls-tree --full-tree -r #{ref || root_ref} --name-status | #{Regexp.escape(query)})

View file

@ -193,12 +193,9 @@ module Backup
end
def empty_repo?(project_or_wiki)
project_or_wiki.repository.expire_exists_cache # protect backups from stale cache
project_or_wiki.repository.empty_repo?
rescue => e
progress.puts "Ignoring repository error and continuing backing up project: #{display_repo_path(project_or_wiki)} - #{e.message}".color(:orange)
false
# Protect against stale caches
project_or_wiki.repository.expire_emptiness_caches
project_or_wiki.repository.empty?
end
def repository_storage_paths_args

View file

@ -83,7 +83,7 @@ module Gitlab
Gitlab::Git.check_namespace!(start_repository)
start_repository = RemoteRepository.new(start_repository) unless start_repository.is_a?(RemoteRepository)
start_branch_name = nil if start_repository.empty_repo?
start_branch_name = nil if start_repository.empty?
if start_branch_name && !start_repository.branch_exists?(start_branch_name)
raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.relative_path}"

View file

@ -24,10 +24,12 @@ module Gitlab
@path = repository.path
end
def empty_repo?
def empty?
# We will override this implementation in gitaly-ruby because we cannot
# use '@repository' there.
@repository.empty_repo?
#
# Caches and memoization used on the Rails side
!@repository.exists? || @repository.empty?
end
def commit_id(revision)

View file

@ -75,9 +75,6 @@ module Gitlab
@attributes = Gitlab::Git::Attributes.new(path)
end
delegate :empty?,
to: :rugged
def ==(other)
path == other.path
end
@ -206,6 +203,13 @@ module Gitlab
end
end
# Git repository can contains some hidden refs like:
# /refs/notes/*
# /refs/git-as-svn/*
# /refs/pulls/*
# This refs by default not visible in project page and not cloned to client side.
alias_method :has_visible_content?, :has_local_branches?
def has_local_branches_rugged?
rugged.branches.each(:local).any? do |ref|
begin
@ -1004,7 +1008,7 @@ module Gitlab
Gitlab::Git.check_namespace!(start_repository)
start_repository = RemoteRepository.new(start_repository) unless start_repository.is_a?(RemoteRepository)
return yield nil if start_repository.empty_repo?
return yield nil if start_repository.empty?
if start_repository.same_repository?(self)
yield commit(start_branch_name)
@ -1120,24 +1124,8 @@ module Gitlab
Gitlab::Git::Commit.find(self, ref)
end
# Refactoring aid; allows us to copy code from app/models/repository.rb
def empty_repo?
!exists? || !has_visible_content?
end
#
# Git repository can contains some hidden refs like:
# /refs/notes/*
# /refs/git-as-svn/*
# /refs/pulls/*
# This refs by default not visible in project page and not cloned to client side.
#
# This method return true if repository contains some content visible in project page.
#
def has_visible_content?
return @has_visible_content if defined?(@has_visible_content)
@has_visible_content = has_local_branches?
def empty?
!has_visible_content?
end
# Like all public `Gitlab::Git::Repository` methods, this method is part

View file

@ -27,7 +27,7 @@ module Gitlab
end
SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze
MAXIMUM_GITALY_CALLS = 30
MAXIMUM_GITALY_CALLS = 35
CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
MUTEX = Mutex.new

View file

@ -0,0 +1,69 @@
require 'spec_helper'
describe Backup::Repository do
let(:progress) { StringIO.new }
let!(:project) { create(:project) }
before do
allow(progress).to receive(:puts)
allow(progress).to receive(:print)
allow_any_instance_of(String).to receive(:color) do |string, _color|
string
end
allow_any_instance_of(described_class).to receive(:progress).and_return(progress)
end
describe '#dump' do
describe 'repo failure' do
before do
allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0])
end
it 'does not raise error' do
expect { described_class.new.dump }.not_to raise_error
end
end
end
describe '#restore' do
describe 'command failure' do
before do
allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1])
end
it 'shows the appropriate error' do
described_class.new.restore
expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error")
end
end
end
describe '#empty_repo?' do
context 'for a wiki' do
let(:wiki) { create(:project_wiki) }
it 'invalidates the emptiness cache' do
expect(wiki.repository).to receive(:expire_emptiness_caches).once
wiki.empty?
end
context 'wiki repo has content' do
let!(:wiki_page) { create(:wiki_page, wiki: wiki) }
it 'returns true, regardless of bad cache value' do
expect(described_class.new.send(:empty_repo?, wiki)).to be(false)
end
end
context 'wiki repo does not have content' do
it 'returns true, regardless of bad cache value' do
expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy
end
end
end
end
end

View file

@ -1,117 +0,0 @@
require 'spec_helper'
describe Backup::Repository do
let(:progress) { StringIO.new }
let!(:project) { create(:project) }
before do
allow(progress).to receive(:puts)
allow(progress).to receive(:print)
allow_any_instance_of(String).to receive(:color) do |string, _color|
string
end
allow_any_instance_of(described_class).to receive(:progress).and_return(progress)
end
describe '#dump' do
describe 'repo failure' do
before do
allow_any_instance_of(Repository).to receive(:empty_repo?).and_raise(Rugged::OdbError)
allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0])
end
it 'does not raise error' do
expect { described_class.new.dump }.not_to raise_error
end
it 'shows the appropriate error' do
described_class.new.dump
expect(progress).to have_received(:puts).with("Ignoring repository error and continuing backing up project: #{project.full_path} - Rugged::OdbError")
end
end
describe 'command failure' do
before do
allow_any_instance_of(Repository).to receive(:empty_repo?).and_return(false)
allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1])
end
it 'shows the appropriate error' do
described_class.new.dump
expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error")
end
end
end
describe '#restore' do
describe 'command failure' do
before do
allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1])
end
it 'shows the appropriate error' do
described_class.new.restore
expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error")
end
end
end
describe '#empty_repo?' do
context 'for a wiki' do
let(:wiki) { create(:project_wiki) }
context 'wiki repo has content' do
let!(:wiki_page) { create(:wiki_page, wiki: wiki) }
before do
wiki.repository.exists? # initial cache
end
context '`repository.exists?` is incorrectly cached as false' do
before do
repo = wiki.repository
repo.send(:cache).expire(:exists?)
repo.send(:cache).fetch(:exists?) { false }
repo.send(:instance_variable_set, :@exists, false)
end
it 'returns false, regardless of bad cache value' do
expect(described_class.new.send(:empty_repo?, wiki)).to be_falsey
end
end
context '`repository.exists?` is correctly cached as true' do
it 'returns false' do
expect(described_class.new.send(:empty_repo?, wiki)).to be_falsey
end
end
end
context 'wiki repo does not have content' do
context '`repository.exists?` is incorrectly cached as true' do
before do
repo = wiki.repository
repo.send(:cache).expire(:exists?)
repo.send(:cache).fetch(:exists?) { true }
repo.send(:instance_variable_set, :@exists, true)
end
it 'returns true, regardless of bad cache value' do
expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy
end
end
context '`repository.exists?` is correctly cached as false' do
it 'returns true' do
expect(described_class.new.send(:empty_repo?, wiki)).to be_truthy
end
end
end
end
end
end

View file

@ -4,7 +4,7 @@ describe Gitlab::Git::RemoteRepository, seed_helper: true do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
subject { described_class.new(repository) }
describe '#empty_repo?' do
describe '#empty?' do
using RSpec::Parameterized::TableSyntax
where(:repository, :result) do
@ -13,7 +13,7 @@ describe Gitlab::Git::RemoteRepository, seed_helper: true do
end
with_them do
it { expect(subject.empty_repo?).to eq(result) }
it { expect(subject.empty?).to eq(result) }
end
end

View file

@ -257,7 +257,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
describe '#empty?' do
it { expect(repository.empty?).to be_falsey }
it { expect(repository).not_to be_empty }
end
describe '#ref_names' do

View file

@ -313,7 +313,6 @@ describe Project do
it { is_expected.to delegate_method(method).to(:team) }
end
it { is_expected.to delegate_method(:empty_repo?).to(:repository) }
it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) }
it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) }
end
@ -656,6 +655,24 @@ describe Project do
end
end
describe '#empty_repo?' do
context 'when the repo does not exist' do
let(:project) { build_stubbed(:project) }
it 'returns true' do
expect(project.empty_repo?).to be(true)
end
end
context 'when the repo exists' do
let(:project) { create(:project, :repository) }
let(:empty_project) { create(:project, :empty_repo) }
it { expect(empty_project.empty_repo?).to be(true) }
it { expect(project.empty_repo?).to be(false) }
end
end
describe '#external_issue_tracker' do
let(:project) { create(:project) }
let(:ext_project) { create(:redmine_project) }

View file

@ -583,7 +583,7 @@ describe Repository do
end
it 'properly handles query when repo is empty' do
repository = create(:project).repository
repository = create(:project, :empty_repo).repository
results = repository.search_files_by_content('test', 'master')
expect(results).to match_array([])
@ -619,7 +619,7 @@ describe Repository do
end
it 'properly handles query when repo is empty' do
repository = create(:project).repository
repository = create(:project, :empty_repo).repository
results = repository.search_files_by_name('test', 'master')
@ -1204,17 +1204,15 @@ describe Repository do
let(:empty_repository) { create(:project_empty_repo).repository }
it 'returns true for an empty repository' do
expect(empty_repository.empty?).to eq(true)
expect(empty_repository).to be_empty
end
it 'returns false for a non-empty repository' do
expect(repository.empty?).to eq(false)
expect(repository).not_to be_empty
end
it 'caches the output' do
expect(repository.raw_repository).to receive(:empty?)
.once
.and_return(false)
expect(repository.raw_repository).to receive(:has_visible_content?).once
repository.empty?
repository.empty?

View file

@ -212,7 +212,7 @@ describe 'gitlab:app namespace rake task' do
# Avoid asking gitaly about the root ref (which will fail beacuse of the
# mocked storages)
allow_any_instance_of(Repository).to receive(:empty_repo?).and_return(false)
allow_any_instance_of(Repository).to receive(:empty?).and_return(false)
end
after do