diff --git a/app/models/repository.rb b/app/models/repository.rb index 6c6023a8709..b9f57169ea5 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -568,7 +568,7 @@ class Repository delegate :branch_count, :tag_count, :has_visible_content?, to: :raw_repository cache_method :branch_count, fallback: 0 cache_method :tag_count, fallback: 0 - cache_method :has_visible_content?, fallback: false + cache_method_asymmetrically :has_visible_content? def avatar # n+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/38327 diff --git a/changelogs/unreleased/cache-issues-with-has_visible_content.yml b/changelogs/unreleased/cache-issues-with-has_visible_content.yml new file mode 100644 index 00000000000..0007b3086e8 --- /dev/null +++ b/changelogs/unreleased/cache-issues-with-has_visible_content.yml @@ -0,0 +1,5 @@ +--- +title: Use cache_method_asymmetrically with Repository#has_visible_content? +merge_request: 17975 +author: +type: fixed diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7226beacebe..cf9100eb6cf 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1193,33 +1193,21 @@ describe Repository do end describe '#has_visible_content?' do - before do - # If raw_repository.has_visible_content? gets called more than once then - # caching is broken. We don't want that. + it 'delegates to raw_repository when true' do expect(repository.raw_repository).to receive(:has_visible_content?) - .once - .and_return(result) + .and_return(true) + + expect(repository.has_visible_content?).to eq(true) end - context 'when true' do - let(:result) { true } + it 'delegates to raw_repository when false' do + expect(repository.raw_repository).to receive(:has_visible_content?) + .and_return(false) - it 'returns true and caches it' do - expect(repository.has_visible_content?).to eq(true) - # Second call hits the cache - expect(repository.has_visible_content?).to eq(true) - end + expect(repository.has_visible_content?).to eq(false) end - context 'when false' do - let(:result) { false } - - it 'returns false and caches it' do - expect(repository.has_visible_content?).to eq(false) - # Second call hits the cache - expect(repository.has_visible_content?).to eq(false) - end - end + it_behaves_like 'asymmetric cached method', :has_visible_content? end describe '#branch_exists?' do