Resolves N+1 RPC call for Project#readme_url
Caches repository.path into Repository#readme_path
This commit is contained in:
parent
00cbe6c9fd
commit
329a890f8b
4 changed files with 54 additions and 6 deletions
|
@ -988,9 +988,9 @@ class Project < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def readme_url
|
||||
readme = repository.readme
|
||||
if readme
|
||||
Gitlab::Routing.url_helpers.project_blob_url(self, File.join(default_branch, readme.path))
|
||||
readme_path = repository.readme_path
|
||||
if readme_path
|
||||
Gitlab::Routing.url_helpers.project_blob_url(self, File.join(default_branch, readme_path))
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -35,7 +35,7 @@ class Repository
|
|||
#
|
||||
# For example, for entry `:commit_count` there's a method called `commit_count` which
|
||||
# stores its data in the `commit_count` cache key.
|
||||
CACHED_METHODS = %i(size commit_count rendered_readme contribution_guide
|
||||
CACHED_METHODS = %i(size commit_count rendered_readme readme_path contribution_guide
|
||||
changelog license_blob license_key gitignore
|
||||
gitlab_ci_yml branch_names tag_names branch_count
|
||||
tag_count avatar exists? root_ref has_visible_content?
|
||||
|
@ -48,7 +48,7 @@ class Repository
|
|||
# changed. This Hash maps file types (as returned by Gitlab::FileDetector) to
|
||||
# the corresponding methods to call for refreshing caches.
|
||||
METHOD_CACHES_FOR_FILE_TYPES = {
|
||||
readme: :rendered_readme,
|
||||
readme: %i(rendered_readme readme_path),
|
||||
changelog: :changelog,
|
||||
license: %i(license_blob license_key license),
|
||||
contributing: :contribution_guide,
|
||||
|
@ -585,6 +585,11 @@ class Repository
|
|||
head_tree&.readme
|
||||
end
|
||||
|
||||
def readme_path
|
||||
readme&.path
|
||||
end
|
||||
cache_method :readme_path
|
||||
|
||||
def rendered_readme
|
||||
return unless readme
|
||||
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Improves performance of Project#readme_url by caching the README path
|
||||
merge_request: 23357
|
||||
author:
|
||||
type: performance
|
|
@ -1488,6 +1488,7 @@ describe Repository do
|
|||
:size,
|
||||
:commit_count,
|
||||
:rendered_readme,
|
||||
:readme_path,
|
||||
:contribution_guide,
|
||||
:changelog,
|
||||
:license_blob,
|
||||
|
@ -1874,6 +1875,42 @@ describe Repository do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#readme_path', :use_clean_rails_memory_store_caching do
|
||||
context 'with a non-existing repository' do
|
||||
let(:project) { create(:project) }
|
||||
|
||||
it 'returns nil' do
|
||||
expect(repository.readme_path).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'with an existing repository' do
|
||||
context 'when no README exists' do
|
||||
let(:project) { create(:project, :empty_repo) }
|
||||
|
||||
it 'returns nil' do
|
||||
expect(repository.readme_path).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a README exists' do
|
||||
let(:project) { create(:project, :repository) }
|
||||
|
||||
it 'returns the README' do
|
||||
expect(repository.readme_path).to eq("README.md")
|
||||
end
|
||||
|
||||
it 'caches the response' do
|
||||
expect(repository).to receive(:readme).and_call_original.once
|
||||
|
||||
2.times do
|
||||
expect(repository.readme_path).to eq("README.md")
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#expire_statistics_caches' do
|
||||
it 'expires the caches' do
|
||||
expect(repository).to receive(:expire_method_caches)
|
||||
|
@ -2042,9 +2079,10 @@ describe Repository do
|
|||
describe '#refresh_method_caches' do
|
||||
it 'refreshes the caches of the given types' do
|
||||
expect(repository).to receive(:expire_method_caches)
|
||||
.with(%i(rendered_readme license_blob license_key license))
|
||||
.with(%i(rendered_readme readme_path license_blob license_key license))
|
||||
|
||||
expect(repository).to receive(:rendered_readme)
|
||||
expect(repository).to receive(:readme_path)
|
||||
expect(repository).to receive(:license_blob)
|
||||
expect(repository).to receive(:license_key)
|
||||
expect(repository).to receive(:license)
|
||||
|
|
Loading…
Reference in a new issue