diff options
author | Alejandro RodrÃguez <alejorro70@gmail.com> | 2017-10-19 15:57:20 -0300 |
---|---|---|
committer | Alejandro RodrÃguez <alejorro70@gmail.com> | 2017-10-20 12:58:49 -0300 |
commit | 6d04f3789cc16f7211fb3d465956bbd84c9430b4 (patch) | |
tree | 930ed33ccbf391cc8a984e61288749141144f1a3 | |
parent | 00c15cc27c33dd387069fce5777beb29d01f55ac (diff) | |
download | gitlab-ce-non-existing-repo-optimization.tar.gz |
Avoid calling underlying methods on non-existing reposnon-existing-repo-optimization
This saves us Rugged/gRPC invocations
-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 4324ea46aac..8a1b81b5337 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1031,6 +1031,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 @@ -1040,8 +1044,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) |