Commit ae563565 authored by Douwe Maan's avatar Douwe Maan

Merge branch '51061-readme-url-n-1-rpc-call-resolved' into 'master'

Resolves N+1 RPC call for Project#readme_url

Closes #51061

See merge request gitlab-org/gitlab-ce!23357
parents 06d1db85 329a890f
...@@ -878,9 +878,9 @@ class Project < ActiveRecord::Base ...@@ -878,9 +878,9 @@ class Project < ActiveRecord::Base
end end
def readme_url def readme_url
readme = repository.readme readme_path = repository.readme_path
if readme if readme_path
Gitlab::Routing.url_helpers.project_blob_url(self, File.join(default_branch, readme.path)) Gitlab::Routing.url_helpers.project_blob_url(self, File.join(default_branch, readme_path))
end end
end end
......
...@@ -35,7 +35,7 @@ class Repository ...@@ -35,7 +35,7 @@ class Repository
# #
# For example, for entry `:commit_count` there's a method called `commit_count` which # For example, for entry `:commit_count` there's a method called `commit_count` which
# stores its data in the `commit_count` cache key. # 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 changelog license_blob license_key gitignore
gitlab_ci_yml branch_names tag_names branch_count gitlab_ci_yml branch_names tag_names branch_count
tag_count avatar exists? root_ref has_visible_content? tag_count avatar exists? root_ref has_visible_content?
...@@ -48,7 +48,7 @@ class Repository ...@@ -48,7 +48,7 @@ class Repository
# changed. This Hash maps file types (as returned by Gitlab::FileDetector) to # changed. This Hash maps file types (as returned by Gitlab::FileDetector) to
# the corresponding methods to call for refreshing caches. # the corresponding methods to call for refreshing caches.
METHOD_CACHES_FOR_FILE_TYPES = { METHOD_CACHES_FOR_FILE_TYPES = {
readme: :rendered_readme, readme: %i(rendered_readme readme_path),
changelog: :changelog, changelog: :changelog,
license: %i(license_blob license_key license), license: %i(license_blob license_key license),
contributing: :contribution_guide, contributing: :contribution_guide,
...@@ -591,6 +591,11 @@ class Repository ...@@ -591,6 +591,11 @@ class Repository
head_tree&.readme head_tree&.readme
end end
def readme_path
readme&.path
end
cache_method :readme_path
def rendered_readme def rendered_readme
return unless 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 @@ describe Repository do ...@@ -1488,6 +1488,7 @@ describe Repository do
:size, :size,
:commit_count, :commit_count,
:rendered_readme, :rendered_readme,
:readme_path,
:contribution_guide, :contribution_guide,
:changelog, :changelog,
:license_blob, :license_blob,
...@@ -1874,6 +1875,42 @@ describe Repository do ...@@ -1874,6 +1875,42 @@ describe Repository do
end end
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 describe '#expire_statistics_caches' do
it 'expires the caches' do it 'expires the caches' do
expect(repository).to receive(:expire_method_caches) expect(repository).to receive(:expire_method_caches)
...@@ -2042,9 +2079,10 @@ describe Repository do ...@@ -2042,9 +2079,10 @@ describe Repository do
describe '#refresh_method_caches' do describe '#refresh_method_caches' do
it 'refreshes the caches of the given types' do it 'refreshes the caches of the given types' do
expect(repository).to receive(:expire_method_caches) 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(:rendered_readme)
expect(repository).to receive(:readme_path)
expect(repository).to receive(:license_blob) expect(repository).to receive(:license_blob)
expect(repository).to receive(:license_key) expect(repository).to receive(:license_key)
expect(repository).to receive(:license) 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