Commit cd08fd23 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'non-existing-repo-optimization' into 'master'

Avoid calling underlying methods on non-existing repos

See merge request gitlab-org/gitlab-ce!14962
parents c29d2c6a 6d04f378
......@@ -1021,6 +1021,10 @@ class Repository
if instance_variable_defined?(ivar)
instance_variable_get(ivar)
else
# If the repository doesn't exist and a fallback was specified we return
# that value inmediately. This saves us Rugged/gRPC invocations.
return fallback unless fallback.nil? || exists?
begin
value =
if memoize_only
......@@ -1030,8 +1034,9 @@ class Repository
end
instance_variable_set(ivar, value)
rescue Rugged::ReferenceError, Gitlab::Git::Repository::NoRepository
# if e.g. HEAD or the entire repository doesn't exist we want to
# gracefully handle this and not cache anything.
# Even if the above `#exists?` check passes these errors might still
# occur (for example because of a non-existing HEAD). We want to
# gracefully handle this and not cache anything
fallback
end
end
......
......@@ -2110,19 +2110,41 @@ describe Repository do
end
describe '#cache_method_output', :use_clean_rails_memory_store_caching do
let(:fallback) { 10 }
context 'with a non-existing repository' do
let(:value) do
repository.cache_method_output(:cats, fallback: 10) do
raise Rugged::ReferenceError
let(:project) { create(:project) } # No repository
subject do
repository.cache_method_output(:cats, fallback: fallback) do
repository.cats_call_stub
end
end
it 'returns a fallback value' do
expect(value).to eq(10)
it 'returns the fallback value' do
expect(subject).to eq(fallback)
end
it 'avoids calling the original method' do
expect(repository).not_to receive(:cats_call_stub)
subject
end
end
context 'with a method throwing a non-existing-repository error' do
subject do
repository.cache_method_output(:cats, fallback: fallback) do
raise Gitlab::Git::Repository::NoRepository
end
end
it 'returns the fallback value' do
expect(subject).to eq(fallback)
end
it 'does not cache the data' do
value
subject
expect(repository.instance_variable_defined?(:@cats)).to eq(false)
expect(repository.send(:cache).exist?(:cats)).to eq(false)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment