Commit 58df8b53 authored by Dylan Griffith's avatar Dylan Griffith Committed by Stan Hu

Fix stale caching of Repository#has_visible_content?

parent 43dcd3e4
...@@ -568,7 +568,7 @@ class Repository ...@@ -568,7 +568,7 @@ class Repository
delegate :branch_count, :tag_count, :has_visible_content?, to: :raw_repository delegate :branch_count, :tag_count, :has_visible_content?, to: :raw_repository
cache_method :branch_count, fallback: 0 cache_method :branch_count, fallback: 0
cache_method :tag_count, fallback: 0 cache_method :tag_count, fallback: 0
cache_method :has_visible_content?, fallback: false cache_method_asymmetrically :has_visible_content?
def avatar def avatar
# n+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/38327 # n+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/38327
......
---
title: Use cache_method_asymmetrically with Repository#has_visible_content?
merge_request: 17975
author:
type: fixed
...@@ -1193,33 +1193,21 @@ describe Repository do ...@@ -1193,33 +1193,21 @@ describe Repository do
end end
describe '#has_visible_content?' do describe '#has_visible_content?' do
before do it 'delegates to raw_repository when true' do
# If raw_repository.has_visible_content? gets called more than once then
# caching is broken. We don't want that.
expect(repository.raw_repository).to receive(:has_visible_content?) expect(repository.raw_repository).to receive(:has_visible_content?)
.once .and_return(true)
.and_return(result)
end
context 'when true' do
let(:result) { true }
it 'returns true and caches it' do expect(repository.has_visible_content?).to eq(true)
expect(repository.has_visible_content?).to eq(true)
# Second call hits the cache
expect(repository.has_visible_content?).to eq(true)
end
end end
context 'when false' do it 'delegates to raw_repository when false' do
let(:result) { false } expect(repository.raw_repository).to receive(:has_visible_content?)
.and_return(false)
it 'returns false and caches it' do expect(repository.has_visible_content?).to eq(false)
expect(repository.has_visible_content?).to eq(false)
# Second call hits the cache
expect(repository.has_visible_content?).to eq(false)
end
end end
it_behaves_like 'asymmetric cached method', :has_visible_content?
end end
describe '#branch_exists?' do describe '#branch_exists?' do
......
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