Commit 15b4e981 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'sh-remove-double-caching-repo-empty' into 'master'

Remove double caching of Repository#empty?

Closes #43882

See merge request gitlab-org/gitlab-ce!17588
parents 0a6c82ea 611929eb
...@@ -35,7 +35,7 @@ class Repository ...@@ -35,7 +35,7 @@ class Repository
CACHED_METHODS = %i(size commit_count rendered_readme contribution_guide CACHED_METHODS = %i(size commit_count rendered_readme contribution_guide
changelog license_blob license_key gitignore koding_yml changelog license_blob license_key gitignore koding_yml
gitlab_ci_yml branch_names tag_names branch_count gitlab_ci_yml branch_names tag_names branch_count
tag_count avatar exists? empty? root_ref has_visible_content? tag_count avatar exists? root_ref has_visible_content?
issue_template_names merge_request_template_names).freeze issue_template_names merge_request_template_names).freeze
# Methods that use cache_method but only memoize the value # Methods that use cache_method but only memoize the value
...@@ -360,7 +360,7 @@ class Repository ...@@ -360,7 +360,7 @@ class Repository
def expire_emptiness_caches def expire_emptiness_caches
return unless empty? return unless empty?
expire_method_caches(%i(empty? has_visible_content?)) expire_method_caches(%i(has_visible_content?))
end end
def lookup_cache def lookup_cache
...@@ -506,12 +506,14 @@ class Repository ...@@ -506,12 +506,14 @@ class Repository
end end
cache_method :exists? cache_method :exists?
# We don't need to cache the output of this method because both exists? and
# has_visible_content? are already memoized and cached. There's no guarantee
# that the values are expired and loaded atomically.
def empty? def empty?
return true unless exists? return true unless exists?
!has_visible_content? !has_visible_content?
end end
cache_method :empty?
# The size of this repository in megabytes. # The size of this repository in megabytes.
def size def size
......
---
title: Remove double caching of Repository#empty?
merge_request:
author:
type: fixed
...@@ -1447,7 +1447,6 @@ describe Repository do ...@@ -1447,7 +1447,6 @@ describe Repository do
it 'expires the caches for an empty repository' do it 'expires the caches for an empty repository' do
allow(repository).to receive(:empty?).and_return(true) allow(repository).to receive(:empty?).and_return(true)
expect(cache).to receive(:expire).with(:empty?)
expect(cache).to receive(:expire).with(:has_visible_content?) expect(cache).to receive(:expire).with(:has_visible_content?)
repository.expire_emptiness_caches repository.expire_emptiness_caches
...@@ -1456,7 +1455,6 @@ describe Repository do ...@@ -1456,7 +1455,6 @@ describe Repository do
it 'does not expire the cache for a non-empty repository' do it 'does not expire the cache for a non-empty repository' do
allow(repository).to receive(:empty?).and_return(false) 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?) expect(cache).not_to receive(:expire).with(:has_visible_content?)
repository.expire_emptiness_caches repository.expire_emptiness_caches
......
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