summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-10-26 11:29:46 +0000
committerDouwe Maan <douwe@gitlab.com>2017-10-26 11:29:46 +0000
commitcd08fd2363dedf435c3de5e53b603ffaa31393b0 (patch)
treeb0fece4ee79716d1e51344e65dc76d75ae54024c
parentc29d2c6a41518e3c00c15b6472477532ee072469 (diff)
parent6d04f3789cc16f7211fb3d465956bbd84c9430b4 (diff)
downloadgitlab-ce-cd08fd2363dedf435c3de5e53b603ffaa31393b0.tar.gz
Merge branch 'non-existing-repo-optimization' into 'master'
Avoid calling underlying methods on non-existing repos See merge request gitlab-org/gitlab-ce!14962
-rw-r--r--app/models/repository.rb9
-rw-r--r--spec/models/repository_spec.rb34
2 files changed, 35 insertions, 8 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 327dbd2ea18..7497b69f674 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -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
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 39d188f18af..874368ada67 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -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)