diff options
27 files changed, 268 insertions, 94 deletions
diff --git a/app/controllers/concerns/renders_commit_description.rb b/app/controllers/concerns/renders_commit_description.rb deleted file mode 100644 index ada80eda230..00000000000 --- a/app/controllers/concerns/renders_commit_description.rb +++ /dev/null @@ -1,7 +0,0 @@ -module RendersCommitDescription - def prepare_commit_descriptions_for_rendering(commit_descriptions) - Banzai::CommitDescriptionRenderer.render(commit_descriptions, @project, current_user) - - commit_descriptions - end -end diff --git a/app/controllers/concerns/renders_commits.rb b/app/controllers/concerns/renders_commits.rb new file mode 100644 index 00000000000..bb2c1dfa00a --- /dev/null +++ b/app/controllers/concerns/renders_commits.rb @@ -0,0 +1,7 @@ +module RendersCommits + def prepare_commits_for_rendering(commits) + Banzai::CommitRenderer.render(commits, @project, current_user) + + commits + end +end diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index 2de9900d449..4a841bf2073 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -2,6 +2,7 @@ require "base64" class Projects::CommitsController < Projects::ApplicationController include ExtractsPath + include RendersCommits before_action :require_non_empty_project before_action :assign_ref_vars @@ -56,5 +57,7 @@ class Projects::CommitsController < Projects::ApplicationController else @repository.commits(@ref, path: @path, limit: @limit, offset: @offset) end + + @commits = prepare_commits_for_rendering(@commits) end end diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index c8613c0d634..193549663ac 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -3,6 +3,7 @@ require 'addressable/uri' class Projects::CompareController < Projects::ApplicationController include DiffForPath include DiffHelper + include RendersCommits # Authorize before_action :require_non_empty_project @@ -50,7 +51,7 @@ class Projects::CompareController < Projects::ApplicationController .execute(@project, @start_ref) if @compare - @commits = @compare.commits + @commits = prepare_commits_for_rendering(@compare.commits) @diffs = @compare.diffs(diff_options) environment_params = @repository.branch_exists?(@head_ref) ? { ref: @head_ref } : { commit: @compare.commit } diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index f35d53896ba..1096afbb798 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -1,6 +1,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::ApplicationController include DiffForPath include DiffHelper + include RendersCommits skip_before_action :merge_request skip_before_action :ensure_ref_fetched @@ -107,7 +108,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap @target_project = @merge_request.target_project @source_project = @merge_request.source_project - @commits = @merge_request.commits + @commits = prepare_commits_for_rendering(@merge_request.commits) @commit = @merge_request.diff_head_commit @note_counts = Note.where(commit_id: @commits.map(&:id)) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 5095d7fd445..dc7a94d824a 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -2,6 +2,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo include ToggleSubscriptionAction include IssuableActions include RendersNotes + include RendersCommits include ToggleAwardEmoji include IssuableCollections @@ -94,7 +95,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo def commits # Get commits from repository # or from cache if already merged - @commits = @merge_request.commits + @commits = prepare_commits_for_rendering(@merge_request.commits) @note_counts = Note.where(commit_id: @commits.map(&:id)) .group(:commit_id).count diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index d58c8d14a75..fbad9ba7db8 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -2,6 +2,7 @@ class SearchController < ApplicationController skip_before_action :authenticate_user! include SearchHelper + include RendersCommits layout 'search' @@ -20,6 +21,8 @@ class SearchController < ApplicationController @search_results = search_service.search_results @search_objects = search_service.search_objects + render_commits if @scope == 'commits' + check_single_commit_result end @@ -38,6 +41,10 @@ class SearchController < ApplicationController private + def render_commits + @search_objects = prepare_commits_for_rendering(@search_objects) + end + def check_single_commit_result if @search_results.single_commit_result? only_commit = @search_results.objects('commits').first diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index 941cfce8370..46bced00c72 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -21,25 +21,28 @@ module MarkupHelper end # Use this in places where you would normally use link_to(gfm(...), ...). - # + def link_to_markdown(body, url, html_options = {}) + return '' if body.blank? + + link_to_html(markdown(body, pipeline: :single_line), url, html_options) + end + + def link_to_markdown_field(object, field, url, html_options = {}) + rendered_field = markdown_field(object, field) + + link_to_html(rendered_field, url, html_options) + end + # It solves a problem occurring with nested links (i.e. # "<a>outer text <a>gfm ref</a> more outer text</a>"). This will not be # interpreted as intended. Browsers will parse something like # "<a>outer text </a><a>gfm ref</a> more outer text" (notice the last part is - # not linked any more). link_to_gfm corrects that. It wraps all parts to + # not linked any more). link_to_html corrects that. It wraps all parts to # explicitly produce the correct linking behavior (i.e. # "<a>outer text </a><a>gfm ref</a><a> more outer text</a>"). - def link_to_gfm(body, url, html_options = {}) - return '' if body.blank? + def link_to_html(redacted, url, html_options = {}) + fragment = Nokogiri::HTML::DocumentFragment.parse(redacted) - context = { - project: @project, - current_user: (current_user if defined?(current_user)), - pipeline: :single_line - } - gfm_body = Banzai.render(body, context) - - fragment = Nokogiri::HTML::DocumentFragment.parse(gfm_body) if fragment.children.size == 1 && fragment.children[0].name == 'a' # Fragment has only one node, and it's a link generated by `gfm`. # Replace it with our requested link. @@ -82,7 +85,10 @@ module MarkupHelper def markdown_field(object, field) object = object.for_display if object.respond_to?(:for_display) + redacted_field_html = object.try(:"redacted_#{field}_html") + return '' unless object.present? + return redacted_field_html if redacted_field_html html = Banzai.render_field(object, field) prepare_for_rendering(html, object.banzai_render_context(field)) diff --git a/app/models/commit.rb b/app/models/commit.rb index ba3845df867..2ae8890c1b3 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -16,6 +16,8 @@ class Commit participant :notes_with_associations attr_accessor :project, :author + attr_accessor :redacted_description_html + attr_accessor :redacted_title_html DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] @@ -26,6 +28,13 @@ class Commit # The SHA can be between 7 and 40 hex characters. COMMIT_SHA_PATTERN = '\h{7,40}'.freeze + def banzai_render_context(field) + context = { pipeline: :single_line, project: self.project } + context[:author] = self.author if self.author + + context + end + class << self def decorate(commits, project) commits.map do |commit| diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index c7359d873d9..60ac202bde0 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -22,7 +22,7 @@ = author_avatar(commit, size: 36) .commit-row-title %span.item-title.str-truncated-100 - = link_to_gfm commit.title, project_commit_path(@project, commit.id), class: "cdark", title: commit.title + = link_to_markdown commit.title, project_commit_path(@project, commit.id), class: "cdark", title: commit.title .pull-right = link_to commit.short_id, project_commit_path(@project, commit), class: "commit-sha" diff --git a/app/views/projects/branches/_commit.html.haml b/app/views/projects/branches/_commit.html.haml index 18fbb81c167..7892019bb15 100644 --- a/app/views/projects/branches/_commit.html.haml +++ b/app/views/projects/branches/_commit.html.haml @@ -4,6 +4,6 @@ = link_to commit.short_id, project_commit_path(project, commit.id), class: "commit-sha" · %span.str-truncated - = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" + = link_to_markdown commit.title, project_commit_path(project, commit.id), class: "commit-row-message" · #{time_ago_with_tooltip(commit.committed_date)} diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 1214aabe837..b8655808d89 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -16,7 +16,7 @@ .commit-detail .commit-content - = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message item-title" + = link_to_markdown_field(commit, :title, project_commit_path(project, commit.id), class: "commit-row-message item-title") %span.commit-row-message.visible-xs-inline · = commit.short_id @@ -28,7 +28,8 @@ - if commit.description? %pre.commit-row-description.js-toggle-content - = preserve(markdown(commit.description, pipeline: :single_line, author: commit.author)) + = preserve(markdown_field(commit, :description)) + .commiter - commit_author_link = commit_author_link(commit, avatar: false, size: 24) - commit_timeago = time_ago_with_tooltip(commit.committed_date) diff --git a/app/views/projects/commits/_inline_commit.html.haml b/app/views/projects/commits/_inline_commit.html.haml index 48cefbe45f2..26385d2f534 100644 --- a/app/views/projects/commits/_inline_commit.html.haml +++ b/app/views/projects/commits/_inline_commit.html.haml @@ -3,6 +3,6 @@ = link_to commit.short_id, project_commit_path(project, commit), class: "commit-sha" %span.str-truncated - = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" + = link_to_markdown_field(commit, :title, project_commit_path(project, commit.id), class: "commit-row-message") .pull-right #{time_ago_with_tooltip(commit.committed_date)} diff --git a/app/views/projects/deployments/_commit.html.haml b/app/views/projects/deployments/_commit.html.haml index 4c22166c256..014486be868 100644 --- a/app/views/projects/deployments/_commit.html.haml +++ b/app/views/projects/deployments/_commit.html.haml @@ -12,6 +12,6 @@ %span.flex-truncate-child - if commit_title = deployment.commit_title = author_avatar(deployment.commit, size: 20) - = link_to_gfm commit_title, project_commit_path(@project, deployment.sha), class: "commit-row-message" + = link_to_markdown commit_title, project_commit_path(@project, deployment.sha), class: "commit-row-message" - else Cant find HEAD commit for this branch diff --git a/app/views/projects/tree/_tree_commit_column.html.haml b/app/views/projects/tree/_tree_commit_column.html.haml index f3d4706809f..abb3e918e87 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_gfm commit.full_title, project_commit_path(@project, commit.id), class: "tree-commit-link" + = link_to_markdown commit.full_title, project_commit_path(@project, commit.id), class: "tree-commit-link" diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 7174855e176..4f00a9f2759 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -28,7 +28,7 @@ commented - if note.system %span.system-note-message - = note.redacted_note_html + = markdown_field(note, :note) %a{ href: "##{dom_id(note)}" } = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') - unless note.system? @@ -39,7 +39,7 @@ = render 'projects/notes/actions', note: note, note_editable: note_editable .note-body{ class: note_editable ? 'js-task-list-container' : '' } .note-text.md - = note.redacted_note_html + = markdown_field(note, :note) = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago') .original-note-content.hidden{ data: { post_url: note_url(note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } } #{note.note} diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index f4f155c8d94..52a8fe8bb67 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -31,8 +31,7 @@ - if show_last_commit_as_description .description.prepend-top-5 - = link_to_gfm project.commit.title, project_commit_path(project, project.commit), - class: "commit-row-message" + = link_to_markdown(project.commit.title, project_commit_path(project, project.commit), class: "commit-row-message") - elsif project.description.present? .description.prepend-top-5 = markdown_field(project, :description) diff --git a/changelogs/unreleased/34509-improves-markdown-rendering-performance-for-commits-list.yml b/changelogs/unreleased/34509-improves-markdown-rendering-performance-for-commits-list.yml new file mode 100644 index 00000000000..a61d703bacd --- /dev/null +++ b/changelogs/unreleased/34509-improves-markdown-rendering-performance-for-commits-list.yml @@ -0,0 +1,5 @@ +--- +title: Improves markdown rendering performance for commit lists. +merge_request: +author: +type: other diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 5b455a8065a..e1a59d8c152 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -114,9 +114,6 @@ def instrument_classes(instrumentation) # This is a Rails scope so we have to instrument it manually. instrumentation.instrument_method(Project, :visible_to_user) - # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/34509 - instrumentation.instrument_method(MarkupHelper, :link_to_gfm) - # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/30224#note_32306159 instrumentation.instrument_instance_method(MergeRequestDiff, :load_commits) diff --git a/lib/banzai/commit_description_renderer.rb b/lib/banzai/commit_description_renderer.rb deleted file mode 100644 index 9600ab918a4..00000000000 --- a/lib/banzai/commit_description_renderer.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Banzai - module CommitDescriptionRenderer - def self.render(commit_descriptions, project, user = nil) - ObjectRenderer.new(project, user).render(commit_descriptions, :commit_description) - end - end -end diff --git a/lib/banzai/commit_renderer.rb b/lib/banzai/commit_renderer.rb new file mode 100644 index 00000000000..f5ff95e3eb3 --- /dev/null +++ b/lib/banzai/commit_renderer.rb @@ -0,0 +1,11 @@ +module Banzai + module CommitRenderer + ATTRIBUTES = [:description, :title].freeze + + def self.render(commits, project, user = nil) + obj_renderer = ObjectRenderer.new(project, user) + + ATTRIBUTES.each { |attr| obj_renderer.render(commits, attr) } + end + end +end diff --git a/lib/banzai/object_renderer.rb b/lib/banzai/object_renderer.rb index 2196a92474c..e40556e869c 100644 --- a/lib/banzai/object_renderer.rb +++ b/lib/banzai/object_renderer.rb @@ -38,7 +38,7 @@ module Banzai objects.each_with_index do |object, index| redacted_data = redacted[index] object.__send__("redacted_#{attribute}_html=", redacted_data[:document].to_html.html_safe) # rubocop:disable GitlabSecurity/PublicSend - object.user_visible_reference_count = redacted_data[:visible_reference_count] + object.user_visible_reference_count = redacted_data[:visible_reference_count] if object.respond_to?(:user_visible_reference_count) end end diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb index 95d82d17658..ceca9296851 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -36,6 +36,10 @@ module Banzai # The context to use is managed by the object and cannot be changed. # Use #render, passing it the field text, if a custom rendering is needed. def self.render_field(object, field) + unless object.respond_to?(:cached_markdown_fields) + return cacheless_render_field(object, field) + end + object.refresh_markdown_cache!(do_update: update_object?(object)) unless object.cached_html_up_to_date?(field) object.cached_html_for(field) diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index 70eb01c9c44..03d706062b7 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -52,12 +52,71 @@ describe MarkupHelper do end end - describe '#link_to_gfm' do + describe '#markdown_field' do + let(:attribute) { :title } + + describe 'with already redacted attribute' do + it 'returns the redacted attribute' do + commit.redacted_title_html = 'commit title' + + expect(Banzai).not_to receive(:render_field) + + expect(helper.markdown_field(commit, attribute)).to eq('commit title') + end + end + + describe 'without redacted attribute' do + it 'renders the markdown value' do + expect(Banzai).to receive(:render_field).with(commit, attribute).and_call_original + + helper.markdown_field(commit, attribute) + end + end + end + + describe '#link_to_markdown_field' do + let(:link) { '/commits/0a1b2c3d' } + let(:issues) { create_list(:issue, 2, project: project) } + + it 'handles references nested in links with all the text' do + allow(commit).to receive(:title).and_return("This should finally fix #{issues[0].to_reference} and #{issues[1].to_reference} for real") + + actual = helper.link_to_markdown_field(commit, :title, link) + doc = Nokogiri::HTML.parse(actual) + + # Make sure we didn't create invalid markup + expect(doc.errors).to be_empty + + # Leading commit link + expect(doc.css('a')[0].attr('href')).to eq link + expect(doc.css('a')[0].text).to eq 'This should finally fix ' + + # First issue link + expect(doc.css('a')[1].attr('href')) + .to eq project_issue_path(project, issues[0]) + expect(doc.css('a')[1].text).to eq issues[0].to_reference + + # Internal commit link + expect(doc.css('a')[2].attr('href')).to eq link + expect(doc.css('a')[2].text).to eq ' and ' + + # Second issue link + expect(doc.css('a')[3].attr('href')) + .to eq project_issue_path(project, issues[1]) + expect(doc.css('a')[3].text).to eq issues[1].to_reference + + # Trailing commit link + expect(doc.css('a')[4].attr('href')).to eq link + expect(doc.css('a')[4].text).to eq ' for real' + end + end + + describe '#link_to_markdown' do let(:link) { '/commits/0a1b2c3d' } let(:issues) { create_list(:issue, 2, project: project) } it 'handles references nested in links with all the text' do - actual = helper.link_to_gfm("This should finally fix #{issues[0].to_reference} and #{issues[1].to_reference} for real", link) + actual = helper.link_to_markdown("This should finally fix #{issues[0].to_reference} and #{issues[1].to_reference} for real", link) doc = Nokogiri::HTML.parse(actual) # Make sure we didn't create invalid markup @@ -87,7 +146,7 @@ describe MarkupHelper do end it 'forwards HTML options' do - actual = helper.link_to_gfm("Fixed in #{commit.id}", link, class: 'foo') + actual = helper.link_to_markdown("Fixed in #{commit.id}", link, class: 'foo') doc = Nokogiri::HTML.parse(actual) expect(doc.css('a')).to satisfy do |v| @@ -98,23 +157,43 @@ describe MarkupHelper do it "escapes HTML passed in as the body" do actual = "This is a <h1>test</h1> - see #{issues[0].to_reference}" - expect(helper.link_to_gfm(actual, link)) + expect(helper.link_to_markdown(actual, link)) .to match('<h1>test</h1>') end it 'ignores reference links when they are the entire body' do text = issues[0].to_reference - act = helper.link_to_gfm(text, '/foo') + act = helper.link_to_markdown(text, '/foo') expect(act).to eq %Q(<a href="/foo">#{issues[0].to_reference}</a>) end it 'replaces commit message with emoji to link' do - actual = link_to_gfm(':book: Book', '/foo') + actual = link_to_markdown(':book: Book', '/foo') expect(actual) .to eq '<gl-emoji title="open book" data-name="book" data-unicode-version="6.0">📖</gl-emoji><a href="/foo"> Book</a>' end end + describe '#link_to_html' do + it 'wraps the rendered content in a link' do + link = '/commits/0a1b2c3d' + issue = create(:issue, project: project) + + rendered = helper.markdown("This should finally fix #{issue.to_reference} for real", pipeline: :single_line) + doc = Nokogiri::HTML.parse(rendered) + + expect(doc.css('a')[0].attr('href')) + .to eq project_issue_path(project, issue) + expect(doc.css('a')[0].text).to eq issue.to_reference + + wrapped = helper.link_to_html(rendered, link) + doc = Nokogiri::HTML.parse(wrapped) + + expect(doc.css('a')[0].attr('href')).to eq link + expect(doc.css('a')[0].text).to eq 'This should finally fix ' + end + end + describe '#render_wiki_content' do before do @wiki = double('WikiPage') diff --git a/spec/lib/banzai/commit_renderer_spec.rb b/spec/lib/banzai/commit_renderer_spec.rb new file mode 100644 index 00000000000..049d025a5b9 --- /dev/null +++ b/spec/lib/banzai/commit_renderer_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Banzai::CommitRenderer do + describe '.render' do + it 'renders a commit description and title' do + user = double(:user) + project = create(:project, :repository) + + expect(Banzai::ObjectRenderer).to receive(:new).with(project, user).and_call_original + + described_class::ATTRIBUTES.each do |attr| + expect_any_instance_of(Banzai::ObjectRenderer).to receive(:render).with([project.commit], attr).once.and_call_original + expect(Banzai::Renderer).to receive(:cacheless_render_field).with(project.commit, attr) + end + + described_class.render([project.commit], project, user) + end + end +end diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index 7f5d481c36c..b172a1b718c 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -1,53 +1,77 @@ require 'spec_helper' describe Banzai::ObjectRenderer do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { project.owner } let(:renderer) { described_class.new(project, user, custom_value: 'value') } let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_VERSION) } describe '#render' do - it 'renders and redacts an Array of objects' do - renderer.render([object], :note) + context 'with cache' do + it 'renders and redacts an Array of objects' do + renderer.render([object], :note) - expect(object.redacted_note_html).to eq '<p dir="auto">hello</p>' - expect(object.user_visible_reference_count).to eq 0 - end + expect(object.redacted_note_html).to eq '<p dir="auto">hello</p>' + expect(object.user_visible_reference_count).to eq 0 + end - it 'calls Banzai::Redactor to perform redaction' do - expect_any_instance_of(Banzai::Redactor).to receive(:redact).and_call_original + it 'calls Banzai::Redactor to perform redaction' do + expect_any_instance_of(Banzai::Redactor).to receive(:redact).and_call_original - renderer.render([object], :note) - end + renderer.render([object], :note) + end - it 'retrieves field content using Banzai.render_field' do - expect(Banzai).to receive(:render_field).with(object, :note).and_call_original + it 'retrieves field content using Banzai::Renderer.render_field' do + expect(Banzai::Renderer).to receive(:render_field).with(object, :note).and_call_original - renderer.render([object], :note) - end + renderer.render([object], :note) + end - it 'passes context to PostProcessPipeline' do - another_user = create(:user) - another_project = create(:project) - object = Note.new( - note: 'hello', - note_html: 'hello', - author: another_user, - project: another_project - ) - - expect(Banzai::Pipeline::PostProcessPipeline).to receive(:to_document).with( - anything, - hash_including( - skip_redaction: true, - current_user: user, - project: another_project, + it 'passes context to PostProcessPipeline' do + another_user = create(:user) + another_project = create(:project) + object = Note.new( + note: 'hello', + note_html: 'hello', author: another_user, - custom_value: 'value' + project: another_project ) - ).and_call_original - renderer.render([object], :note) + expect(Banzai::Pipeline::PostProcessPipeline).to receive(:to_document).with( + anything, + hash_including( + skip_redaction: true, + current_user: user, + project: another_project, + author: another_user, + custom_value: 'value' + ) + ).and_call_original + + renderer.render([object], :note) + end + end + + context 'without cache' do + let(:commit) { project.commit } + + it 'renders and redacts an Array of objects' do + renderer.render([commit], :title) + + expect(commit.redacted_title_html).to eq("Merge branch 'branch-merged' into 'master'") + end + + it 'calls Banzai::Redactor to perform redaction' do + expect_any_instance_of(Banzai::Redactor).to receive(:redact).and_call_original + + renderer.render([commit], :title) + end + + it 'retrieves field content using Banzai::Renderer.cacheless_render_field' do + expect(Banzai::Renderer).to receive(:cacheless_render_field).with(commit, :title).and_call_original + + renderer.render([commit], :title) + end end end end diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb index 0e094405e33..da42272bbef 100644 --- a/spec/lib/banzai/renderer_spec.rb +++ b/spec/lib/banzai/renderer_spec.rb @@ -4,6 +4,7 @@ describe Banzai::Renderer do def fake_object(fresh:) object = double('object') + allow(object).to receive(:respond_to?).with(:cached_markdown_fields).and_return(true) allow(object).to receive(:cached_html_up_to_date?).with(:field).and_return(fresh) allow(object).to receive(:cached_html_for).with(:field).and_return('field_html') @@ -12,25 +13,38 @@ describe Banzai::Renderer do describe '#render_field' do let(:renderer) { described_class } - subject { renderer.render_field(object, :field) } - context 'with a stale cache' do - let(:object) { fake_object(fresh: false) } + context 'without cache' do + let(:commit) { create(:project, :repository).commit } - it 'caches and returns the result' do - expect(object).to receive(:refresh_markdown_cache!).with(do_update: true) + it 'returns cacheless render field' do + expect(renderer).to receive(:cacheless_render_field).with(commit, :title) - is_expected.to eq('field_html') + renderer.render_field(commit, :title) end end - context 'with an up-to-date cache' do - let(:object) { fake_object(fresh: true) } + context 'with cache' do + subject { renderer.render_field(object, :field) } - it 'uses the cache' do - expect(object).to receive(:refresh_markdown_cache!).never + context 'with a stale cache' do + let(:object) { fake_object(fresh: false) } - is_expected.to eq('field_html') + it 'caches and returns the result' do + expect(object).to receive(:refresh_markdown_cache!).with(do_update: true) + + is_expected.to eq('field_html') + end + end + + context 'with an up-to-date cache' do + let(:object) { fake_object(fresh: true) } + + it 'uses the cache' do + expect(object).to receive(:refresh_markdown_cache!).never + + is_expected.to eq('field_html') + end end end end |