Commit ff02c63c authored by Douwe Maan's avatar Douwe Maan

Merge branch 'remove-has-visible-content-caching' into 'master'

Remove caching of Repository#has_visible_content?

This MR removes the caching of `Repository#has_visible_content?`. The cache for this method is no longer necessary and this should solve the problem described in https://gitlab.com/gitlab-org/gitlab-ce/issues/25278.

See merge request !7947
parents 73a066c2 5eb12bd7
...@@ -85,11 +85,7 @@ class Repository ...@@ -85,11 +85,7 @@ class Repository
# This method return true if repository contains some content visible in project page. # This method return true if repository contains some content visible in project page.
# #
def has_visible_content? def has_visible_content?
return @has_visible_content unless @has_visible_content.nil? branch_count > 0
@has_visible_content = cache.fetch(:has_visible_content?) do
branch_count > 0
end
end end
def commit(ref = 'HEAD') def commit(ref = 'HEAD')
...@@ -374,12 +370,6 @@ class Repository ...@@ -374,12 +370,6 @@ class Repository
return unless empty? return unless empty?
expire_method_caches(%i(empty?)) expire_method_caches(%i(empty?))
expire_has_visible_content_cache
end
def expire_has_visible_content_cache
cache.expire(:has_visible_content?)
@has_visible_content = nil
end end
def lookup_cache def lookup_cache
...@@ -467,7 +457,6 @@ class Repository ...@@ -467,7 +457,6 @@ class Repository
# Runs code after a new branch has been created. # Runs code after a new branch has been created.
def after_create_branch def after_create_branch
expire_branches_cache expire_branches_cache
expire_has_visible_content_cache
repository_event(:push_branch) repository_event(:push_branch)
end end
...@@ -481,7 +470,6 @@ class Repository ...@@ -481,7 +470,6 @@ class Repository
# Runs code after an existing branch has been removed. # Runs code after an existing branch has been removed.
def after_remove_branch def after_remove_branch
expire_has_visible_content_cache
expire_branches_cache expire_branches_cache
end end
......
---
title: Remove visible content caching
merge_request:
author:
...@@ -768,7 +768,6 @@ describe Repository, models: true do ...@@ -768,7 +768,6 @@ describe Repository, models: true do
expect(repository).not_to receive(:expire_root_ref_cache) expect(repository).not_to receive(:expire_root_ref_cache)
expect(repository).not_to receive(:expire_emptiness_caches) expect(repository).not_to receive(:expire_emptiness_caches)
expect(repository).to receive(:expire_branches_cache) expect(repository).to receive(:expire_branches_cache)
expect(repository).to receive(:expire_has_visible_content_cache)
repository.update_branch_with_hooks(user, 'new-feature') { new_rev } repository.update_branch_with_hooks(user, 'new-feature') { new_rev }
end end
...@@ -786,7 +785,6 @@ describe Repository, models: true do ...@@ -786,7 +785,6 @@ describe Repository, models: true do
expect(empty_repository).to receive(:expire_root_ref_cache) expect(empty_repository).to receive(:expire_root_ref_cache)
expect(empty_repository).to receive(:expire_emptiness_caches) expect(empty_repository).to receive(:expire_emptiness_caches)
expect(empty_repository).to receive(:expire_branches_cache) expect(empty_repository).to receive(:expire_branches_cache)
expect(empty_repository).to receive(:expire_has_visible_content_cache)
empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!', empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!',
'Updates file content', 'master', false) 'Updates file content', 'master', false)
...@@ -829,15 +827,6 @@ describe Repository, models: true do ...@@ -829,15 +827,6 @@ describe Repository, models: true do
expect(subject).to eq(true) expect(subject).to eq(true)
end end
it 'caches the output' do
expect(repository).to receive(:branch_count).
once.
and_return(3)
repository.has_visible_content?
repository.has_visible_content?
end
end end
end end
...@@ -918,20 +907,6 @@ describe Repository, models: true do ...@@ -918,20 +907,6 @@ describe Repository, models: true do
end end
end end
describe '#expire_has_visible_content_cache' do
it 'expires the visible content cache' do
repository.has_visible_content?
expect(repository).to receive(:branch_count).
once.
and_return(0)
repository.expire_has_visible_content_cache
expect(repository.has_visible_content?).to eq(false)
end
end
describe '#expire_branch_cache' do describe '#expire_branch_cache' do
# This method is private but we need it for testing purposes. Sadly there's # This method is private but we need it for testing purposes. Sadly there's
# no other proper way of testing caching operations. # no other proper way of testing caching operations.
...@@ -967,7 +942,6 @@ describe Repository, models: true do ...@@ -967,7 +942,6 @@ describe Repository, models: true 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(:empty?)
expect(repository).to receive(:expire_has_visible_content_cache)
repository.expire_emptiness_caches repository.expire_emptiness_caches
end end
...@@ -976,7 +950,6 @@ describe Repository, models: true do ...@@ -976,7 +950,6 @@ describe Repository, models: true 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(:empty?)
expect(repository).not_to receive(:expire_has_visible_content_cache)
repository.expire_emptiness_caches repository.expire_emptiness_caches
end end
...@@ -1204,7 +1177,7 @@ describe Repository, models: true do ...@@ -1204,7 +1177,7 @@ describe Repository, models: true do
describe '#after_create_branch' do describe '#after_create_branch' do
it 'flushes the visible content cache' do it 'flushes the visible content cache' do
expect(repository).to receive(:expire_has_visible_content_cache) expect(repository).to receive(:expire_branches_cache)
repository.after_create_branch repository.after_create_branch
end end
...@@ -1212,7 +1185,7 @@ describe Repository, models: true do ...@@ -1212,7 +1185,7 @@ describe Repository, models: true do
describe '#after_remove_branch' do describe '#after_remove_branch' do
it 'flushes the visible content cache' do it 'flushes the visible content cache' do
expect(repository).to receive(:expire_has_visible_content_cache) expect(repository).to receive(:expire_branches_cache)
repository.after_remove_branch repository.after_remove_branch
end end
......
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