Merge branch '49591-use-cached-readme-blobs' into 'master'

Use cached readme blobs where appropriate

Closes #49591

See merge request gitlab-org/gitlab-ce!22325

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

app/models/tree.rb

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

changelogs/unreleased/49591-use-cached-readme-blobs.yml

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

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

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