Use cached readme blobs where appropriate

GitLab keeps a cache of the rendered HTML for a repository's README as
stored in the HEAD branch. However, it was not used in all
circumstances. In particular, the new blob viewer framework bypassed
this cache entirely.

This MR ensures a ::ReadmeBlob is returned instead of a ::Blob when
asking a repository for an individual blob, if the commit and path
match the readme for HEAD. This makes the cached HTML available to
consumers, including the blob viewer.

The ReadmeBlob is a simple delegator to the Blob, so should be
compatible in all cases. Adding the rendered_markdown method is the
only additional behaviour it contains.
This commit is contained in:
Nick Thomas 2018-10-12 14:42:35 +01:00
parent e347170cc5
commit 0669127a32
No known key found for this signature in database
GPG key ID: 2A313A47AFADACE9
5 changed files with 80 additions and 39 deletions

View file

@ -487,7 +487,20 @@ class Repository
end
def blob_at(sha, path)
Blob.decorate(raw_repository.blob_at(sha, path), project)
blob = Blob.decorate(raw_repository.blob_at(sha, path), project)
# Don't attempt to return a special result if there is no blob at all
return unless blob
# Don't attempt to return a special result unless we're looking at HEAD
return blob unless head_commit&.sha == sha
case path
when head_tree&.readme_path
ReadmeBlob.new(blob, self)
else
blob
end
rescue Gitlab::Git::Repository::NoRepository
nil
end
@ -569,9 +582,7 @@ class Repository
cache_method :merge_request_template_names, fallback: []
def readme
if readme = tree(:head)&.readme
ReadmeBlob.new(readme, self)
end
head_tree&.readme
end
def rendered_readme

View file

@ -2,6 +2,7 @@
class Tree
include Gitlab::MarkupHelper
include Gitlab::Utils::StrongMemoize
attr_accessor :repository, :sha, :path, :entries
@ -16,32 +17,36 @@ class Tree
@entries = Gitlab::Git::Tree.where(git_repo, @sha, @path, recursive)
end
def readme_path
strong_memoize(:readme_path) do
available_readmes = blobs.select do |blob|
Gitlab::FileDetector.type_of(blob.name) == :readme
end
previewable_readmes = available_readmes.select do |blob|
previewable?(blob.name)
end
plain_readmes = available_readmes.select do |blob|
plain?(blob.name)
end
# Prioritize previewable over plain readmes
entry = previewable_readmes.first || plain_readmes.first
next nil unless entry
if path == '/'
entry.name
else
File.join(path, entry.name)
end
end
end
def readme
return @readme if defined?(@readme)
available_readmes = blobs.select do |blob|
Gitlab::FileDetector.type_of(blob.name) == :readme
strong_memoize(:readme) do
repository.blob_at(sha, readme_path) if readme_path
end
previewable_readmes = available_readmes.select do |blob|
previewable?(blob.name)
end
plain_readmes = available_readmes.select do |blob|
plain?(blob.name)
end
# Prioritize previewable over plain readmes
readme_tree = previewable_readmes.first || plain_readmes.first
# Return if we can't preview any of them
if readme_tree.nil?
return @readme = nil
end
readme_path = path == '/' ? readme_tree.name : File.join(path, readme_tree.name)
@readme = repository.blob_at(sha, readme_path)
end
def trees

View file

@ -0,0 +1,5 @@
---
title: Use cached readme contents when available
merge_request: 22325
author:
type: performance

View file

@ -527,28 +527,28 @@ describe Project do
end
describe "#readme_url" do
let(:project) { create(:project, :repository, path: "somewhere") }
context 'with a non-existing repository' do
it 'returns nil' do
allow(project.repository).to receive(:tree).with(:head).and_return(nil)
let(:project) { create(:project) }
it 'returns nil' do
expect(project.readme_url).to be_nil
end
end
context 'with an existing repository' do
context 'when no README exists' do
it 'returns nil' do
allow_any_instance_of(Tree).to receive(:readme).and_return(nil)
let(:project) { create(:project, :empty_repo) }
it 'returns nil' do
expect(project.readme_url).to be_nil
end
end
context 'when a README exists' do
let(:project) { create(:project, :repository) }
it 'returns the README' do
expect(project.readme_url).to eql("#{Gitlab.config.gitlab.url}/#{project.namespace.full_path}/somewhere/blob/master/README.md")
expect(project.readme_url).to eq("#{project.web_url}/blob/master/README.md")
end
end
end

View file

@ -461,6 +461,24 @@ describe Repository do
it { is_expected.to be_nil }
end
context 'regular blob' do
subject { repository.blob_at(repository.head_commit.sha, '.gitignore') }
it { is_expected.to be_an_instance_of(::Blob) }
end
context 'readme blob on HEAD' do
subject { repository.blob_at(repository.head_commit.sha, 'README.md') }
it { is_expected.to be_an_instance_of(::ReadmeBlob) }
end
context 'readme blob not on HEAD' do
subject { repository.blob_at(repository.find_branch('feature').target, 'README.md') }
it { is_expected.to be_an_instance_of(::Blob) }
end
end
describe '#merged_to_root_ref?' do
@ -1922,23 +1940,25 @@ describe Repository do
describe '#readme', :use_clean_rails_memory_store_caching do
context 'with a non-existing repository' do
it 'returns nil' do
allow(repository).to receive(:tree).with(:head).and_return(nil)
let(:project) { create(:project) }
it 'returns nil' do
expect(repository.readme).to be_nil
end
end
context 'with an existing repository' do
context 'when no README exists' do
it 'returns nil' do
allow_any_instance_of(Tree).to receive(:readme).and_return(nil)
let(:project) { create(:project, :empty_repo) }
it 'returns nil' do
expect(repository.readme).to be_nil
end
end
context 'when a README exists' do
let(:project) { create(:project, :repository) }
it 'returns the README' do
expect(repository.readme).to be_an_instance_of(ReadmeBlob)
end