From d9833890cca2c8fb388cc020f626f9d2c09871a4 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 4 Sep 2018 13:51:02 +0100 Subject: [PATCH] Bulk-render commit titles in the tree view to improve performance --- app/controllers/projects/refs_controller.rb | 12 +++++++++++- app/models/commit.rb | 8 +++++++- .../projects/tree/_tree_commit_column.html.haml | 2 +- spec/features/projects/tree/tree_show_spec.rb | 16 +++++++++++++++- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb index 48a09e1ddb8..4a4e9ec1d7d 100644 --- a/app/controllers/projects/refs_controller.rb +++ b/app/controllers/projects/refs_controller.rb @@ -59,13 +59,18 @@ class Projects::RefsController < Projects::ApplicationController commit_path = project_commit_path(@project, last_commit) if last_commit { file_name: content.name, - commit: last_commit, + # TODO: deduplicate commits so we don't render the same one multiple times + commit: (Commit.new(last_commit, project) if last_commit), type: content.type, commit_path: commit_path } end end + # The commit titles must be passed through Banzai before being shown. + # Doing this here in bulk allows significant database work to be skipped. + prerender_commits!(@logs.map { |log| log[:commit] }) + offset = (@offset + @limit) if contents.size > offset @more_log_url = logs_file_project_ref_path(@project, @ref, @path || '', offset: offset) @@ -84,6 +89,11 @@ class Projects::RefsController < Projects::ApplicationController private + def prerender_commits!(commits) + renderer = Banzai::ObjectRenderer.new(user: current_user, default_project: @project) + renderer.render(commits, :full_title) # modifies the commit objects inplace + end + def validate_ref_id return not_found! if params[:id].present? && params[:id] !~ Gitlab::PathRegex.git_reference_regex end diff --git a/app/models/commit.rb b/app/models/commit.rb index 594972ad344..c993f3ed507 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -22,6 +22,7 @@ class Commit attr_accessor :project, :author attr_accessor :redacted_description_html attr_accessor :redacted_title_html + attr_accessor :redacted_full_title_html attr_reader :gpg_commit DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] @@ -38,7 +39,12 @@ class Commit def banzai_render_context(field) pipeline = field == :description ? :commit_description : :single_line context = { pipeline: pipeline, project: self.project } - context[:author] = self.author if self.author + + # The author is only needed when rendering the description + if field == :description + author = self.author + context[:author] = author if author + end context end diff --git a/app/views/projects/tree/_tree_commit_column.html.haml b/app/views/projects/tree/_tree_commit_column.html.haml index abb3e918e87..406dccb74fb 100644 --- a/app/views/projects/tree/_tree_commit_column.html.haml +++ b/app/views/projects/tree/_tree_commit_column.html.haml @@ -1,2 +1,2 @@ %span.str-truncated - = link_to_markdown commit.full_title, project_commit_path(@project, commit.id), class: "tree-commit-link" + = link_to_html commit.redacted_full_title_html, project_commit_path(@project, commit.id), class: 'tree-commit-link' diff --git a/spec/features/projects/tree/tree_show_spec.rb b/spec/features/projects/tree/tree_show_spec.rb index 8ae036cd29f..45e81e1c040 100644 --- a/spec/features/projects/tree/tree_show_spec.rb +++ b/spec/features/projects/tree/tree_show_spec.rb @@ -4,16 +4,30 @@ describe 'Projects tree', :js do let(:user) { create(:user) } let(:project) { create(:project, :repository) } + # This commit has a known state on the master branch of gitlab-test + let(:test_sha) { '7975be0116940bf2ad4321f79d02a55c5f7779aa' } + before do project.add_maintainer(user) sign_in(user) end it 'renders tree table without errors' do - visit project_tree_path(project, 'master') + visit project_tree_path(project, test_sha) wait_for_requests expect(page).to have_selector('.tree-item') + expect(page).to have_content('add tests for .gitattributes custom highlighting') + expect(page).not_to have_selector('.flash-alert') + expect(page).not_to have_selector('.label-lfs', text: 'LFS') + end + + it 'renders tree table for a subtree without errors' do + visit project_tree_path(project, File.join(test_sha, 'files')) + wait_for_requests + + expect(page).to have_selector('.tree-item') + expect(page).to have_content('add spaces in whitespace file') expect(page).not_to have_selector('.label-lfs', text: 'LFS') expect(page).not_to have_selector('.flash-alert') end