diff options
author | Stan Hu <stanhu@gmail.com> | 2018-03-07 04:01:12 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-03-07 17:25:21 +0300 |
commit | 611929eb96c0ee9f07d98163d0610325d8a2a322 (patch) | |
tree | 6dda6341c6a44c1b94d0db4d5fd915035c590c2a /spec | |
parent | ff00cfe45c744393de5bfcafdcfbd12108cc664f (diff) |
Memoize Repository#empty? instead of double caching the value
We saw that in a customer instance, `empty?` was cached to be `true` even
though `has_visible_content?` and `exists?` were `true`. This double caching
can run into edge cases because there's no guarantee that the inner values
will properly expire the outer one, especially if there is Redis replication lag.
Consider this scenario:
1. `exists?` and `has_visible_content?` are false
2. `empty?` is expired
3. A subsequent call to `empty?` returns `true` because `exists?` is false even though `empty` is true
4. `exists?` and `has_visible_content?` are then expired
5. `exists?` and `has_visible_content?` are set to true
6. `empty?` is still stuck in the wrong value as `true`
Closes #43882
Diffstat (limited to 'spec')
-rw-r--r-- | spec/models/repository_spec.rb | 2 |
1 files changed, 0 insertions, 2 deletions
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 38653e18306..81fbd15b2b1 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1447,7 +1447,6 @@ describe Repository do it 'expires the caches for an empty repository' do allow(repository).to receive(:empty?).and_return(true) - expect(cache).to receive(:expire).with(:empty?) expect(cache).to receive(:expire).with(:has_visible_content?) repository.expire_emptiness_caches @@ -1456,7 +1455,6 @@ describe Repository do it 'does not expire the cache for a non-empty repository' do allow(repository).to receive(:empty?).and_return(false) - expect(cache).not_to receive(:expire).with(:empty?) expect(cache).not_to receive(:expire).with(:has_visible_content?) repository.expire_emptiness_caches |