From 10a60f7d944f0bef0bdee92c3b0ad9c6137215d1 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 12 Oct 2018 14:42:35 +0100 Subject: Use cached readme blobs where appropriate --- app/models/repository.rb | 19 ++++++++++++---- app/models/tree.rb | 51 +++++++++++++++++++++++------------------- spec/models/project_spec.rb | 14 ++++++------ spec/models/repository_spec.rb | 28 +++++++++++++++++++---- 4 files changed, 74 insertions(+), 38 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 6ce480c32c4..37a1dd64052 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -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 diff --git a/app/models/tree.rb b/app/models/tree.rb index 3641c33254c..cd385872171 100644 --- a/app/models/tree.rb +++ b/app/models/tree.rb @@ -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 - return @readme if defined?(@readme) - - 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) + 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 - # 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 + def readme + strong_memoize(:readme) do + repository.blob_at(sha, readme_path) if readme_path end - - readme_path = path == '/' ? readme_tree.name : File.join(path, readme_tree.name) - - @readme = repository.blob_at(sha, readme_path) end def trees diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a807c336165..88f70a88210 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -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 diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index fb16a321e4b..799a60ac62f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -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 -- cgit v1.2.1