Commit 6bbe6848 authored by Igor Drozdov's avatar Igor Drozdov

Move preloading commits titles into TreeSummary

That will save us from N + 1 requests on rendering
parent cd1994c1
......@@ -45,7 +45,8 @@ class Projects::RefsController < Projects::ApplicationController
def logs_tree
tree_summary = ::Gitlab::TreeSummary.new(
@commit, @project, path: @path, offset: params[:offset], limit: 25)
@commit, @project, current_user,
path: @path, offset: params[:offset], limit: 25)
respond_to do |format|
format.html { render_404 }
......@@ -60,10 +61,8 @@ class Projects::RefsController < Projects::ApplicationController
# 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
@logs, _ = tree_summary.summarize
@more_log_url = more_url(tree_summary.next_offset) if tree_summary.more?
prerender_commit_full_titles!(commits)
end
end
end
......@@ -74,14 +73,6 @@ class Projects::RefsController < Projects::ApplicationController
logs_file_project_ref_path(@project, @ref, @path, offset: offset)
end
def prerender_commit_full_titles!(commits)
# Preload commit authors as they are used in rendering
commits.each(&:lazy_author)
renderer = Banzai::ObjectRenderer.new(user: current_user, default_project: @project)
renderer.render(commits, :full_title)
end
def validate_ref_id
return not_found! if params[:id].present? && params[:id] !~ Gitlab::PathRegex.git_reference_regex
end
......
......@@ -5,10 +5,11 @@ require 'spec_helper'
describe Gitlab::TreeSummary do
let_it_be(:project) { create(:project, :custom_repo, files: { 'a.txt' => '' }) }
let_it_be(:path_lock) { create(:path_lock, project: project, path: 'a.txt') }
let_it_be(:user) { create(:user) }
let(:commit) { project.repository.head_commit }
subject { described_class.new(commit, project).summarize.first }
subject { described_class.new(commit, project, user).summarize.first }
describe '#summarize (entries)' do
it 'includes path locks in entries' do
......
......@@ -8,14 +8,15 @@ module Gitlab
CACHE_EXPIRE_IN = 1.hour
MAX_OFFSET = 2**31
attr_reader :commit, :project, :path, :offset, :limit
attr_reader :commit, :project, :path, :offset, :limit, :user
attr_reader :resolved_commits
private :resolved_commits
def initialize(commit, project, params = {})
def initialize(commit, project, user, params = {})
@commit = commit
@project = project
@user = user
@path = params.fetch(:path, nil).presence
@offset = [params.fetch(:offset, 0).to_i, MAX_OFFSET].min
......@@ -97,6 +98,7 @@ module Gitlab
end
commits_hsh = repository.list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit)
prerender_commit_full_titles!(commits_hsh.values)
entries.each do |entry|
path_key = entry_path(entry)
......@@ -105,7 +107,7 @@ module Gitlab
if commit
entry[:commit] = commit
entry[:commit_path] = commit_path(commit)
entry[:commit_title_html] = markdown_field(commit, :title)
entry[:commit_title_html] = markdown_field(commit, :full_title)
end
end
end
......@@ -133,6 +135,14 @@ module Gitlab
def tree
strong_memoize(:tree) { repository.tree(commit.id, path) }
end
def prerender_commit_full_titles!(commits)
# Preload commit authors as they are used in rendering
commits.each(&:lazy_author)
renderer = Banzai::ObjectRenderer.new(user: user, default_project: project)
renderer.render(commits, :full_title)
end
end
end
......
......@@ -8,12 +8,13 @@ describe Gitlab::TreeSummary do
let(:project) { create(:project, :empty_repo) }
let(:repo) { project.repository }
let(:commit) { repo.head_commit }
let_it_be(:user) { create(:user) }
let(:path) { nil }
let(:offset) { nil }
let(:limit) { nil }
subject(:summary) { described_class.new(commit, project, path: path, offset: offset, limit: limit) }
subject(:summary) { described_class.new(commit, project, user, path: path, offset: offset, limit: limit) }
describe '#initialize' do
it 'defaults offset to 0' do
......@@ -141,6 +142,16 @@ describe Gitlab::TreeSummary do
expect(entry).to include(:commit)
end
end
context 'rendering commits' do
it 'does not perform N + 1 request' do
summary
queries = ActiveRecord::QueryRecorder.new { summary.summarize }
expect(queries.count).to be <= 3
end
end
end
describe '#more?' do
......
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