diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-10-26 14:29:46 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-10-26 14:29:46 +0300 |
commit | cd08fd2363dedf435c3de5e53b603ffaa31393b0 (patch) | |
tree | b0fece4ee79716d1e51344e65dc76d75ae54024c | |
parent | c29d2c6a41518e3c00c15b6472477532ee072469 (diff) | |
parent | 6d04f3789cc16f7211fb3d465956bbd84c9430b4 (diff) |
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.rb | 9 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 34 |
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) |