Merge branch 'id-cache-logs-tree' into 'master'

Cache TreeSummary response for logs_tree

See merge request gitlab-org/gitlab!29828

app/controllers/projects/refs_controller.rb

@@ -40,29 +40,25 @@ class Projects::RefsController < Projects::ApplicationController
   end
 
   def logs_tree
-    summary = ::Gitlab::TreeSummary.new(
-      @commit,
-      @project,
-      path: @path,
-      offset: params[:offset],
-      limit: 25
-    )
-
-    @logs, commits = summary.summarize
-    @more_log_url = more_url(summary.next_offset) if summary.more?
+    tree_summary = ::Gitlab::TreeSummary.new(
+      @commit, @project, path: @path, offset: params[:offset], limit: 25)
 
     respond_to do |format|
       format.html { render_404 }
       format.json do
-        response.headers["More-Logs-Url"] = @more_log_url if summary.more?
-        response.headers["More-Logs-Offset"] = summary.next_offset if summary.more?
-        render json: @logs
+        logs, next_offset = tree_summary.fetch_logs
+
+        response.headers["More-Logs-Offset"] = next_offset if next_offset
+
+        render json: logs
       end
 
-      # The commit titles must be rendered and redacted before being shown.
-      # Doing it here allows us to apply performance optimizations that avoid
-      # N+1 problems
+      # Deprecated due to https://gitlab.com/gitlab-org/gitlab/-/issues/36863
+      # Will be removed soon https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29895
       format.js do
+        @logs, commits = tree_summary.summarize
+        @more_log_url = more_url(tree_summary.next_offset) if tree_summary.more?
+
         prerender_commit_full_titles!(commits)
       end
     end

changelogs/unreleased/id-cache-logs-tree.yml

+---
+title: Cache TreeSummary response for logs_tree
+merge_request: 29828
+author:
+type: performance

lib/gitlab/tree_summary.rb

@@ -6,6 +6,9 @@ module Gitlab
 
     include ::Gitlab::Utils::StrongMemoize
 
+    CACHE_EXPIRE_IN = 1.hour
+    MAX_OFFSET = 2**31
+
     attr_reader :commit, :project, :path, :offset, :limit
 
     attr_reader :resolved_commits
@@ -16,7 +19,7 @@ module Gitlab
       @project = project
 
       @path = params.fetch(:path, nil).presence
-      @offset = params.fetch(:offset, 0).to_i
+      @offset = [params.fetch(:offset, 0).to_i, MAX_OFFSET].min
       @limit = (params.fetch(:limit, 25) || 25).to_i
 
       # Ensure that if multiple tree entries share the same last commit, they share
@@ -43,6 +46,17 @@ module Gitlab
       [summary, commits]
     end
 
+    def fetch_logs
+      cache_key = ['projects', project.id, 'logs', commit.id, path, offset]
+      Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do
+        logs, _ = summarize
+
+        new_offset = next_offset if more?
+
+        [logs.as_json, new_offset]
+      end
+    end
+
     # Does the tree contain more entries after the given offset + limit?
     def more?
       all_contents[next_offset].present?

spec/controllers/projects/refs_controller_spec.rb

@@ -12,25 +12,27 @@ describe Projects::RefsController do
   end
 
   describe 'GET #logs_tree' do
+    let(:path) { 'foo/bar/baz.html' }
+
     def default_get(format = :html)
       get :logs_tree,
           params: {
             namespace_id: project.namespace.to_param,
             project_id: project,
             id: 'master',
-            path: 'foo/bar/baz.html'
+            path: path
           },
           format: format
     end
 
-    def xhr_get(format = :html)
+    def xhr_get(format = :html, params = {})
       get :logs_tree, params: {
         namespace_id: project.namespace.to_param,
         project_id: project,
         id: 'master',
-        path: 'foo/bar/baz.html',
+        path: path,
         format: format
-      }, xhr: true
+      }.merge(params), xhr: true
     end
 
     it 'never throws MissingTemplate' do
@@ -52,6 +54,7 @@ describe Projects::RefsController do
       expect(response).to be_successful
     end
 
+    context 'when json is requested' do
       it 'renders JSON' do
         expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original
 
@@ -60,5 +63,18 @@ describe Projects::RefsController do
         expect(response).to be_successful
         expect(json_response).to be_kind_of(Array)
       end
+
+      it 'caches tree summary data', :use_clean_rails_memory_store_caching do
+        expect_next_instance_of(::Gitlab::TreeSummary) do |instance|
+          expect(instance).to receive_messages(summarize: ['logs'], next_offset: 50, more?: true)
+        end
+
+        xhr_get(:json, offset: 25)
+
+        cache_key = "projects/#{project.id}/logs/#{project.commit.id}/#{path}/25"
+        expect(Rails.cache.fetch(cache_key)).to eq(['logs', 50])
+        expect(response.headers['More-Logs-Offset']).to eq(50)
+      end
+    end
   end
 end