Read about our upcoming Code of Conduct on this issue

Commit 6ae90c83 authored by Tiago Botelho's avatar Tiago Botelho
Browse files

Resolves N+1 RPC call for Project#readme_url

Caches repository.path into Repository#readme_path
parent 467c6c6b773c
......@@ -988,9 +988,9 @@ def web_url
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 @@ def readme
head_tree&.readme
end
def readme_path
readme&.path
end
cache_method :readme_path
def rendered_readme
return unless readme
......
---
title: Improves performance of Project#readme_url by caching the README path
merge_request: 23357
author:
type: performance
......@@ -1488,6 +1488,7 @@ def merge(repository, user, merge_request, message)
:size,
:commit_count,
:rendered_readme,
:readme_path,
:contribution_guide,
:changelog,
:license_blob,
......@@ -1874,6 +1875,42 @@ def merge(repository, user, merge_request, message)
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 @@ def merge(repository, user, merge_request, message)
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)
......
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