diff options
36 files changed, 429 insertions, 225 deletions
diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 57b45f335fa..3c64fd964ff 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -39,7 +39,7 @@ module NotesActions @note = Notes::CreateService.new(note_project, current_user, create_params).execute if @note.is_a?(Note) - Banzai::NoteRenderer.render([@note], @project, current_user) + Notes::RenderService.new(current_user).execute([@note], @project) end respond_to do |format| @@ -52,7 +52,7 @@ module NotesActions @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) if @note.is_a?(Note) - Banzai::NoteRenderer.render([@note], @project, current_user) + Notes::RenderService.new(current_user).execute([@note], @project) end respond_to do |format| diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb index 4791bc561a4..824ad06465c 100644 --- a/app/controllers/concerns/renders_notes.rb +++ b/app/controllers/concerns/renders_notes.rb @@ -3,7 +3,7 @@ module RendersNotes preload_noteable_for_regular_notes(notes) preload_max_access_for_authors(notes, @project) preload_first_time_contribution_for_authors(noteable, notes) - Banzai::NoteRenderer.render(notes, @project, current_user) + Notes::RenderService.new(current_user).execute(notes, @project) notes end diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index cd94a36a6e7..d9884a47ec4 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -57,5 +57,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController @events = EventCollection .new(projects, offset: params[:offset].to_i, filter: event_filter) .to_a + + Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) end end diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 19a5db6fd17..280ed93faf8 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -32,6 +32,8 @@ class DashboardController < Dashboard::ApplicationController @events = EventCollection .new(projects, offset: params[:offset].to_i, filter: @event_filter) .to_a + + Events::RenderService.new(current_user).execute(@events) end def set_show_full_reference diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index bc3e95f1aed..eb53a522f90 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -155,6 +155,8 @@ class GroupsController < Groups::ApplicationController @events = EventCollection .new(@projects, offset: params[:offset].to_i, filter: event_filter) .to_a + + Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) end def user_actions diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index db543d688a0..1688121e27e 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -300,6 +300,8 @@ class ProjectsController < Projects::ApplicationController @events = EventCollection .new(projects, offset: params[:offset].to_i, filter: event_filter) .to_a + + Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) end def project_params diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4ee855806ab..5fca31b4956 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -108,6 +108,8 @@ class UsersController < ApplicationController .references(:project) .with_associations .limit_recent(20, params[:offset]) + + Events::RenderService.new(current_user).execute(@events, atom_request: request.format.atom?) end def load_projects diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index fd88e0d794a..079b3cd3aa0 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -172,16 +172,6 @@ module EventsHelper end end - def event_note(text, options = {}) - text = first_line_in_markdown(text, 150, options) - - sanitize( - text, - tags: %w(a img gl-emoji b pre code p span), - attributes: Rails::Html::WhiteListSanitizer.allowed_attributes + ['style', 'data-src', 'data-name', 'data-unicode-version'] - ) - end - def event_commit_title(message) message ||= '' (message.split("\n").first || "").truncate(70) diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index 420622399f3..2c85d7d7720 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -69,10 +69,16 @@ module MarkupHelper # as Markdown. HTML tags in the parsed output are not counted toward the # +max_chars+ limit. If the length limit falls within a tag's contents, then # the tag contents are truncated without removing the closing tag. - def first_line_in_markdown(text, max_chars = nil, options = {}) - md = markdown(text, options).strip + def first_line_in_markdown(object, attribute, max_chars = nil, options = {}) + md = markdown_field(object, attribute, options) - truncate_visible(md, max_chars || md.length) if md.present? + text = truncate_visible(md, max_chars || md.length) if md.present? + + sanitize( + text, + tags: %w(a img gl-emoji b pre code p span), + attributes: Rails::Html::WhiteListSanitizer.allowed_attributes + ['style', 'data-src', 'data-name', 'data-unicode-version'] + ) end def markdown(text, context = {}) @@ -83,15 +89,17 @@ module MarkupHelper prepare_for_rendering(html, context) end - def markdown_field(object, field) + def markdown_field(object, field, context = {}) 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)) + html = Banzai.render_field(object, field, context) + context.reverse_merge!(object.banzai_render_context(field)) if object.respond_to?(:banzai_render_context) + + prepare_for_rendering(html, context) end def markup(file_name, text, context = {}) diff --git a/app/services/base_renderer.rb b/app/services/base_renderer.rb new file mode 100644 index 00000000000..d6e30bd7008 --- /dev/null +++ b/app/services/base_renderer.rb @@ -0,0 +1,7 @@ +class BaseRenderer + attr_reader :current_user + + def initialize(current_user = nil) + @current_user = current_user + end +end diff --git a/app/services/events/render_service.rb b/app/services/events/render_service.rb new file mode 100644 index 00000000000..0b62d8aedf1 --- /dev/null +++ b/app/services/events/render_service.rb @@ -0,0 +1,21 @@ +module Events + class RenderService < BaseRenderer + def execute(events, atom_request: false) + events.map(&:note).compact.group_by(&:project).each do |project, notes| + render_notes(notes, project, atom_request) + end + end + + private + + def render_notes(notes, project, atom_request) + Notes::RenderService.new(current_user).execute(notes, project, render_options(atom_request)) + end + + def render_options(atom_request) + return {} unless atom_request + + { only_path: false, xhtml: true } + end + end +end diff --git a/app/services/notes/render_service.rb b/app/services/notes/render_service.rb new file mode 100644 index 00000000000..a77e98c2b07 --- /dev/null +++ b/app/services/notes/render_service.rb @@ -0,0 +1,21 @@ +module Notes + class RenderService < BaseRenderer + # Renders a collection of Note instances. + # + # notes - The notes to render. + # project - The project to use for redacting. + # user - The user viewing the notes. + + # Possible options: + # requested_path - The request path. + # project_wiki - The project's wiki. + # ref - The current Git reference. + # only_path - flag to turn relative paths into absolute ones. + # xhtml - flag to save the html in XHTML + def execute(notes, project, **opts) + renderer = Banzai::ObjectRenderer.new(project, current_user, **opts) + + renderer.render(notes, :note) + end + end +end diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml index 38fd053ae65..efe1fb99efc 100644 --- a/app/views/dashboard/todos/_todo.html.haml +++ b/app/views/dashboard/todos/_todo.html.haml @@ -36,7 +36,7 @@ .todo-body .todo-note .md - = event_note(todo.body, project: todo.project) + = first_line_in_markdown(todo, :body, 150, project: todo.project) - if todo.pending? .todo-actions diff --git a/app/views/events/_event_note.atom.haml b/app/views/events/_event_note.atom.haml index 6fa2f9bd4db..7e264eb5575 100644 --- a/app/views/events/_event_note.atom.haml +++ b/app/views/events/_event_note.atom.haml @@ -1,2 +1,2 @@ %div{ xmlns: "http://www.w3.org/1999/xhtml" } - = markdown(note.note, pipeline: :atom, project: note.project, author: note.author) + = markdown_field(note, :note) diff --git a/app/views/events/event/_note.html.haml b/app/views/events/event/_note.html.haml index df4b9562215..de6383e4097 100644 --- a/app/views/events/event/_note.html.haml +++ b/app/views/events/event/_note.html.haml @@ -10,7 +10,7 @@ .event-body .event-note .md - = event_note(event.target.note, project: event.project) + = first_line_in_markdown(event.target, :note, 150, project: event.project) - note = event.target - if note.attachment.url - if note.attachment.image? diff --git a/changelogs/unreleased/27375-dashboard-activity-performance.yml b/changelogs/unreleased/27375-dashboard-activity-performance.yml new file mode 100644 index 00000000000..87c6197a24d --- /dev/null +++ b/changelogs/unreleased/27375-dashboard-activity-performance.yml @@ -0,0 +1,5 @@ +--- +title: Improve DashboardController#activity.json performance +merge_request: 14985 +author: +type: performance diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 2d8704622b6..dc242c747b9 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -77,7 +77,6 @@ def instrument_classes(instrumentation) instrumentation.instrument_instance_methods(Banzai::ObjectRenderer) instrumentation.instrument_instance_methods(Banzai::Redactor) - instrumentation.instrument_methods(Banzai::NoteRenderer) [Issuable, Mentionable, Participable].each do |klass| instrumentation.instrument_instance_methods(klass) diff --git a/lib/banzai.rb b/lib/banzai.rb index 35ca234c1ba..5df98f66f3b 100644 --- a/lib/banzai.rb +++ b/lib/banzai.rb @@ -3,8 +3,8 @@ module Banzai Renderer.render(text, context) end - def self.render_field(object, field) - Renderer.render_field(object, field) + def self.render_field(object, field, context = {}) + Renderer.render_field(object, field, context) end def self.cache_collection_render(texts_and_contexts) diff --git a/lib/banzai/filter/absolute_link_filter.rb b/lib/banzai/filter/absolute_link_filter.rb new file mode 100644 index 00000000000..1ec6201523f --- /dev/null +++ b/lib/banzai/filter/absolute_link_filter.rb @@ -0,0 +1,34 @@ +require 'uri' + +module Banzai + module Filter + # HTML filter that converts relative urls into absolute ones. + class AbsoluteLinkFilter < HTML::Pipeline::Filter + def call + return doc unless context[:only_path] == false + + doc.search('a.gfm').each do |el| + process_link_attr el.attribute('href') + end + + doc + end + + protected + + def process_link_attr(html_attr) + return if html_attr.blank? + return if html_attr.value.start_with?('//') + + uri = URI(html_attr.value) + html_attr.value = absolute_link_attr(uri) if uri.relative? + rescue URI::Error + # noop + end + + def absolute_link_attr(uri) + URI.join(Gitlab.config.gitlab.url, uri).to_s + end + end + end +end diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index a0f7e4e5ad5..9fef386de16 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -311,30 +311,6 @@ module Banzai def project_refs_cache RequestStore[:banzai_project_refs] ||= {} end - - def cached_call(request_store_key, cache_key, path: []) - if RequestStore.active? - cache = RequestStore[request_store_key] ||= Hash.new do |hash, key| - hash[key] = Hash.new { |h, k| h[k] = {} } - end - - cache = cache.dig(*path) if path.any? - - get_or_set_cache(cache, cache_key) { yield } - else - yield - end - end - - def get_or_set_cache(cache, key) - if cache.key?(key) - cache[key] - else - value = yield - cache[key] = value if key.present? - value - end - end end end end diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index c6ae28adf87..b9d5ecf70ec 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/lib/banzai/filter/reference_filter.rb @@ -8,6 +8,8 @@ module Banzai # :project (required) - Current project, ignored if reference is cross-project. # :only_path - Generate path-only links. class ReferenceFilter < HTML::Pipeline::Filter + include RequestStoreReferenceCache + class << self attr_accessor :reference_type end diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb index afb6e25963c..c7fa8a8119f 100644 --- a/lib/banzai/filter/user_reference_filter.rb +++ b/lib/banzai/filter/user_reference_filter.rb @@ -60,10 +60,14 @@ module Banzai self.class.references_in(text) do |match, username| if username == 'all' && !skip_project_check? link_to_all(link_content: link_content) - elsif namespace = namespaces[username.downcase] - link_to_namespace(namespace, link_content: link_content) || match else - match + cached_call(:banzai_url_for_object, match, path: [User, username.downcase]) do + if namespace = namespaces[username.downcase] + link_to_namespace(namespace, link_content: link_content) || match + else + match + end + end end end end @@ -74,7 +78,10 @@ module Banzai # The keys of this Hash are the namespace paths, the values the # corresponding Namespace objects. def namespaces - @namespaces ||= Namespace.where_full_path_in(usernames).index_by(&:full_path).transform_keys(&:downcase) + @namespaces ||= Namespace.eager_load(:owner, :route) + .where_full_path_in(usernames) + .index_by(&:full_path) + .transform_keys(&:downcase) end # Returns all usernames referenced in the current document. diff --git a/lib/banzai/note_renderer.rb b/lib/banzai/note_renderer.rb deleted file mode 100644 index 2b7c10f1a0e..00000000000 --- a/lib/banzai/note_renderer.rb +++ /dev/null @@ -1,21 +0,0 @@ -module Banzai - module NoteRenderer - # Renders a collection of Note instances. - # - # notes - The notes to render. - # project - The project to use for redacting. - # user - The user viewing the notes. - # path - The request path. - # wiki - The project's wiki. - # git_ref - The current Git reference. - def self.render(notes, project, user = nil, path = nil, wiki = nil, git_ref = nil) - renderer = ObjectRenderer.new(project, - user, - requested_path: path, - project_wiki: wiki, - ref: git_ref) - - renderer.render(notes, :note) - end - end -end diff --git a/lib/banzai/object_renderer.rb b/lib/banzai/object_renderer.rb index e40556e869c..9bb8ed913d8 100644 --- a/lib/banzai/object_renderer.rb +++ b/lib/banzai/object_renderer.rb @@ -37,7 +37,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.__send__("redacted_#{attribute}_html=", redacted_data[:document].to_html(save_options).html_safe) # rubocop:disable GitlabSecurity/PublicSend object.user_visible_reference_count = redacted_data[:visible_reference_count] if object.respond_to?(:user_visible_reference_count) end end @@ -83,5 +83,10 @@ module Banzai skip_redaction: true ) end + + def save_options + return {} unless base_context[:xhtml] + { save_with: Nokogiri::XML::Node::SaveOptions::AS_XHTML } + end end end diff --git a/lib/banzai/pipeline/post_process_pipeline.rb b/lib/banzai/pipeline/post_process_pipeline.rb index 131ac3b0eec..dcd52bc03c7 100644 --- a/lib/banzai/pipeline/post_process_pipeline.rb +++ b/lib/banzai/pipeline/post_process_pipeline.rb @@ -3,9 +3,10 @@ module Banzai class PostProcessPipeline < BasePipeline def self.filters FilterArray[ + Filter::RedactorFilter, Filter::RelativeLinkFilter, Filter::IssuableStateFilter, - Filter::RedactorFilter + Filter::AbsoluteLinkFilter ] end diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb index 5f91884a878..5cb9adf52b0 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -32,12 +32,9 @@ module Banzai # Convert a Markdown-containing field on an object into an HTML-safe String # of HTML. This method is analogous to calling render(object.field), but it # can cache the rendered HTML in the object, rather than Redis. - # - # 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) + def self.render_field(object, field, context = {}) unless object.respond_to?(:cached_markdown_fields) - return cacheless_render_field(object, field) + return cacheless_render_field(object, field, context) end object.refresh_markdown_cache! unless object.cached_html_up_to_date?(field) @@ -46,9 +43,9 @@ module Banzai end # Same as +render_field+, but without consulting or updating the cache field - def self.cacheless_render_field(object, field, options = {}) + def self.cacheless_render_field(object, field, context = {}) text = object.__send__(field) # rubocop:disable GitlabSecurity/PublicSend - context = object.banzai_render_context(field).merge(options) + context = context.reverse_merge(object.banzai_render_context(field)) if object.respond_to?(:banzai_render_context) cacheless_render(text, context) end diff --git a/lib/banzai/request_store_reference_cache.rb b/lib/banzai/request_store_reference_cache.rb new file mode 100644 index 00000000000..426131442a2 --- /dev/null +++ b/lib/banzai/request_store_reference_cache.rb @@ -0,0 +1,27 @@ +module Banzai + module RequestStoreReferenceCache + def cached_call(request_store_key, cache_key, path: []) + if RequestStore.active? + cache = RequestStore[request_store_key] ||= Hash.new do |hash, key| + hash[key] = Hash.new { |h, k| h[k] = {} } + end + + cache = cache.dig(*path) if path.any? + + get_or_set_cache(cache, cache_key) { yield } + else + yield + end + end + + def get_or_set_cache(cache, key) + if cache.key?(key) + cache[key] + else + value = yield + cache[key] = value if key.present? + value + end + end + end +end diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index d5536fcb22b..8a80b88da5d 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -1,96 +1,6 @@ require 'spec_helper' describe EventsHelper do - describe '#event_note' do - let(:user) { build(:user) } - - before do - allow(helper).to receive(:current_user).and_return(user) - end - - it 'displays one line of plain text without alteration' do - input = 'A short, plain note' - expect(helper.event_note(input)).to match(input) - expect(helper.event_note(input)).not_to match(/\.\.\.\z/) - end - - it 'displays inline code' do - input = 'A note with `inline code`' - expected = 'A note with <code>inline code</code>' - - expect(helper.event_note(input)).to match(expected) - end - - it 'truncates a note with multiple paragraphs' do - input = "Paragraph 1\n\nParagraph 2" - expected = 'Paragraph 1...' - - expect(helper.event_note(input)).to match(expected) - end - - it 'displays the first line of a code block' do - input = "```\nCode block\nwith two lines\n```" - expected = %r{<pre.+><code><span class="line">Code block\.\.\.</span>\n</code></pre>} - - expect(helper.event_note(input)).to match(expected) - end - - it 'truncates a single long line of text' do - text = 'The quick brown fox jumped over the lazy dog twice' # 50 chars - input = text * 4 - expected = (text * 2).sub(/.{3}/, '...') - - expect(helper.event_note(input)).to match(expected) - end - - it 'preserves a link href when link text is truncated' do - text = 'The quick brown fox jumped over the lazy dog' # 44 chars - input = "#{text}#{text}#{text} " # 133 chars - link_url = 'http://example.com/foo/bar/baz' # 30 chars - input << link_url - expected_link_text = 'http://example...</a>' - - expect(helper.event_note(input)).to match(link_url) - expect(helper.event_note(input)).to match(expected_link_text) - end - - it 'preserves code color scheme' do - input = "```ruby\ndef test\n 'hello world'\nend\n```" - expected = "\n<pre class=\"code highlight js-syntax-highlight ruby\">" \ - "<code><span class=\"line\"><span class=\"k\">def</span> <span class=\"nf\">test</span>...</span>\n" \ - "</code></pre>" - expect(helper.event_note(input)).to eq(expected) - end - - it 'preserves data-src for lazy images' do - input = "" - image_url = "data-src=\"/uploads/test.png\"" - expect(helper.event_note(input)).to match(image_url) - end - - context 'labels formatting' do - let(:input) { 'this should be ~label_1' } - - def format_event_note(project) - create(:label, title: 'label_1', project: project) - - helper.event_note(input, { project: project }) - end - - it 'preserves style attribute for a label that can be accessed by current_user' do - project = create(:project, :public) - - expect(format_event_note(project)).to match(/span class=.*style=.*/) - end - - it 'does not style a label that can not be accessed by current_user' do - project = create(:project, :private) - - expect(format_event_note(project)).to eq("<p>#{input}</p>") - end - end - end - describe '#event_commit_title' do let(:message) { "foo & bar " + "A" * 70 + "\n" + "B" * 80 } subject { helper.event_commit_title(message) } diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index 03d706062b7..62ea6d48542 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -67,7 +67,7 @@ describe MarkupHelper do describe 'without redacted attribute' do it 'renders the markdown value' do - expect(Banzai).to receive(:render_field).with(commit, attribute).and_call_original + expect(Banzai).to receive(:render_field).with(commit, attribute, {}).and_call_original helper.markdown_field(commit, attribute) end @@ -252,38 +252,141 @@ describe MarkupHelper do end describe '#first_line_in_markdown' do - it 'truncates Markdown properly' do - text = "@#{user.username}, can you look at this?\nHello world\n" - actual = first_line_in_markdown(text, 100, project: project) + shared_examples_for 'common markdown examples' do + let(:project_base) { build(:project, :repository) } - doc = Nokogiri::HTML.parse(actual) + it 'displays inline code' do + object = create_object('Text with `inline code`') + expected = 'Text with <code>inline code</code>' - # Make sure we didn't create invalid markup - expect(doc.errors).to be_empty + expect(first_line_in_markdown(object, attribute, 100, project: project)).to match(expected) + end - # Leading user link - expect(doc.css('a').length).to eq(1) - expect(doc.css('a')[0].attr('href')).to eq user_path(user) - expect(doc.css('a')[0].text).to eq "@#{user.username}" + it 'truncates the text with multiple paragraphs' do + object = create_object("Paragraph 1\n\nParagraph 2") + expected = 'Paragraph 1...' - expect(doc.content).to eq "@#{user.username}, can you look at this?..." - end + expect(first_line_in_markdown(object, attribute, 100, project: project)).to match(expected) + end - it 'truncates Markdown with emoji properly' do - text = "foo :wink:\nbar :grinning:" - actual = first_line_in_markdown(text, 100, project: project) + it 'displays the first line of a code block' do + object = create_object("```\nCode block\nwith two lines\n```") + expected = %r{<pre.+><code><span class="line">Code block\.\.\.</span>\n</code></pre>} - doc = Nokogiri::HTML.parse(actual) + expect(first_line_in_markdown(object, attribute, 100, project: project)).to match(expected) + end - # Make sure we didn't create invalid markup - # But also account for the 2 errors caused by the unknown `gl-emoji` elements - expect(doc.errors.length).to eq(2) + it 'truncates a single long line of text' do + text = 'The quick brown fox jumped over the lazy dog twice' # 50 chars + object = create_object(text * 4) + expected = (text * 2).sub(/.{3}/, '...') + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(expected) + end + + it 'preserves a link href when link text is truncated' do + text = 'The quick brown fox jumped over the lazy dog' # 44 chars + input = "#{text}#{text}#{text} " # 133 chars + link_url = 'http://example.com/foo/bar/baz' # 30 chars + input << link_url + object = create_object(input) + expected_link_text = 'http://example...</a>' + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(link_url) + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(expected_link_text) + end + + it 'preserves code color scheme' do + object = create_object("```ruby\ndef test\n 'hello world'\nend\n```") + expected = "\n<pre class=\"code highlight js-syntax-highlight ruby\">" \ + "<code><span class=\"line\"><span class=\"k\">def</span> <span class=\"nf\">test</span>...</span>\n" \ + "</code></pre>" + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to eq(expected) + end + + it 'preserves data-src for lazy images' do + object = create_object("") + image_url = "data-src=\".*/uploads/test.png\"" + + expect(first_line_in_markdown(object, attribute, 150, project: project)).to match(image_url) + end + + context 'labels formatting' do + let(:label_title) { 'this should be ~label_1' } + + def create_and_format_label(project) + create(:label, title: 'label_1', project: project) + object = create_object(label_title, project: project) - expect(doc.css('gl-emoji').length).to eq(2) - expect(doc.css('gl-emoji')[0].attr('data-name')).to eq 'wink' - expect(doc.css('gl-emoji')[1].attr('data-name')).to eq 'grinning' + first_line_in_markdown(object, attribute, 150, project: project) + end - expect(doc.content).to eq "foo 😉\nbar 😀" + it 'preserves style attribute for a label that can be accessed by current_user' do + project = create(:project, :public) + + expect(create_and_format_label(project)).to match(/span class=.*style=.*/) + end + + it 'does not style a label that can not be accessed by current_user' do + project = create(:project, :private) + + expect(create_and_format_label(project)).to eq("<p>#{label_title}</p>") + end + end + + it 'truncates Markdown properly' do + object = create_object("@#{user.username}, can you look at this?\nHello world\n") + actual = first_line_in_markdown(object, attribute, 100, project: project) + + doc = Nokogiri::HTML.parse(actual) + + # Make sure we didn't create invalid markup + expect(doc.errors).to be_empty + + # Leading user link + expect(doc.css('a').length).to eq(1) + expect(doc.css('a')[0].attr('href')).to eq user_path(user) + expect(doc.css('a')[0].text).to eq "@#{user.username}" + + expect(doc.content).to eq "@#{user.username}, can you look at this?..." + end + + it 'truncates Markdown with emoji properly' do + object = create_object("foo :wink:\nbar :grinning:") + actual = first_line_in_markdown(object, attribute, 100, project: project) + + doc = Nokogiri::HTML.parse(actual) + + # Make sure we didn't create invalid markup + # But also account for the 2 errors caused by the unknown `gl-emoji` elements + expect(doc.errors.length).to eq(2) + + expect(doc.css('gl-emoji').length).to eq(2) + expect(doc.css('gl-emoji')[0].attr('data-name')).to eq 'wink' + expect(doc.css('gl-emoji')[1].attr('data-name')).to eq 'grinning' + + expect(doc.content).to eq "foo 😉\nbar 😀" + end + end + + context 'when the asked attribute can be redacted' do + include_examples 'common markdown examples' do + let(:attribute) { :note } + def create_object(title, project: project_base) + build(:note, note: title, project: project) + end + end + end + + context 'when the asked attribute can not be redacted' do + include_examples 'common markdown examples' do + let(:attribute) { :body } + def create_object(title, project: project_base) + issue = build(:issue, title: title) + build(:todo, :done, project: project_base, author: user, target: issue) + end + end end end diff --git a/spec/lib/banzai/commit_renderer_spec.rb b/spec/lib/banzai/commit_renderer_spec.rb index 049d025a5b9..84adaebdcbe 100644 --- a/spec/lib/banzai/commit_renderer_spec.rb +++ b/spec/lib/banzai/commit_renderer_spec.rb @@ -10,7 +10,7 @@ describe Banzai::CommitRenderer do 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) + expect(Banzai::Renderer).to receive(:cacheless_render_field).with(project.commit, attr, {}) end described_class.render([project.commit], project, user) diff --git a/spec/lib/banzai/filter/absolute_link_filter_spec.rb b/spec/lib/banzai/filter/absolute_link_filter_spec.rb new file mode 100644 index 00000000000..a3ad056efcd --- /dev/null +++ b/spec/lib/banzai/filter/absolute_link_filter_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Banzai::Filter::AbsoluteLinkFilter do + def filter(doc, context = {}) + described_class.call(doc, context) + end + + context 'with html links' do + context 'if only_path is false' do + let(:only_path_context) do + { only_path: false } + end + let(:fake_url) { 'http://www.example.com' } + + before do + allow(Gitlab.config.gitlab).to receive(:url).and_return(fake_url) + end + + context 'has the .gfm class' do + it 'converts a relative url into absolute' do + doc = filter(link('/foo', 'gfm'), only_path_context) + expect(doc.at_css('a')['href']).to eq "#{fake_url}/foo" + end + + it 'does not change the url if it already absolute' do + doc = filter(link("#{fake_url}/foo", 'gfm'), only_path_context) + expect(doc.at_css('a')['href']).to eq "#{fake_url}/foo" + end + + context 'if relative_url_root is set' do + it 'joins the url without without doubling the path' do + allow(Gitlab.config.gitlab).to receive(:url).and_return("#{fake_url}/gitlab/") + doc = filter(link("/gitlab/foo", 'gfm'), only_path_context) + expect(doc.at_css('a')['href']).to eq "#{fake_url}/gitlab/foo" + end + end + end + + context 'has not the .gfm class' do + it 'does not convert a relative url into absolute' do + doc = filter(link('/foo'), only_path_context) + expect(doc.at_css('a')['href']).to eq '/foo' + end + end + end + + context 'if only_path is not false' do + it 'does not convert a relative url into absolute' do + expect(filter(link('/foo', 'gfm')).at_css('a')['href']).to eq '/foo' + expect(filter(link('/foo')).at_css('a')['href']).to eq '/foo' + end + end + end + + def link(path, css_class = '') + %(<a class="#{css_class}" href="#{path}">example</a>) + end +end diff --git a/spec/lib/banzai/note_renderer_spec.rb b/spec/lib/banzai/note_renderer_spec.rb deleted file mode 100644 index 32764bee5eb..00000000000 --- a/spec/lib/banzai/note_renderer_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -require 'spec_helper' - -describe Banzai::NoteRenderer do - describe '.render' do - it 'renders a Note' do - note = double(:note) - project = double(:project) - wiki = double(:wiki) - user = double(:user) - - expect(Banzai::ObjectRenderer).to receive(:new) - .with(project, user, - requested_path: 'foo', - project_wiki: wiki, - ref: 'bar') - .and_call_original - - expect_any_instance_of(Banzai::ObjectRenderer) - .to receive(:render).with([note], :note) - - described_class.render([note], project, user, 'foo', wiki, 'bar') - end - end -end diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index b172a1b718c..074d521a5c6 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -22,7 +22,7 @@ describe Banzai::ObjectRenderer do end it 'retrieves field content using Banzai::Renderer.render_field' do - expect(Banzai::Renderer).to receive(:render_field).with(object, :note).and_call_original + expect(Banzai::Renderer).to receive(:render_field).with(object, :note, {}).and_call_original renderer.render([object], :note) end @@ -68,7 +68,7 @@ describe Banzai::ObjectRenderer do 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 + expect(Banzai::Renderer).to receive(:cacheless_render_field).with(commit, :title, {}).and_call_original renderer.render([commit], :title) end diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb index 81a04a2d46d..650cecfc778 100644 --- a/spec/lib/banzai/renderer_spec.rb +++ b/spec/lib/banzai/renderer_spec.rb @@ -18,7 +18,7 @@ describe Banzai::Renderer do let(:commit) { create(:project, :repository).commit } it 'returns cacheless render field' do - expect(renderer).to receive(:cacheless_render_field).with(commit, :title) + expect(renderer).to receive(:cacheless_render_field).with(commit, :title, {}) renderer.render_field(commit, :title) end diff --git a/spec/services/events/render_service_spec.rb b/spec/services/events/render_service_spec.rb new file mode 100644 index 00000000000..b4a4a44d07b --- /dev/null +++ b/spec/services/events/render_service_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Events::RenderService do + describe '#execute' do + let!(:note) { build(:note) } + let!(:event) { build(:event, target: note, project: note.project) } + let!(:user) { build(:user) } + + context 'when the request format is atom' do + it 'renders the note inside events' do + expect(Banzai::ObjectRenderer).to receive(:new) + .with(event.project, user, + only_path: false, + xhtml: true) + .and_call_original + + expect_any_instance_of(Banzai::ObjectRenderer) + .to receive(:render).with([note], :note) + + described_class.new(user).execute([event], atom_request: true) + end + end + + context 'when the request format is not atom' do + it 'renders the note inside events' do + expect(Banzai::ObjectRenderer).to receive(:new) + .with(event.project, user, {}) + .and_call_original + + expect_any_instance_of(Banzai::ObjectRenderer) + .to receive(:render).with([note], :note) + + described_class.new(user).execute([event], atom_request: false) + end + end + end +end diff --git a/spec/services/notes/render_service_spec.rb b/spec/services/notes/render_service_spec.rb new file mode 100644 index 00000000000..faac498037f --- /dev/null +++ b/spec/services/notes/render_service_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Notes::RenderService do + describe '#execute' do + it 'renders a Note' do + note = double(:note) + project = double(:project) + wiki = double(:wiki) + user = double(:user) + + expect(Banzai::ObjectRenderer).to receive(:new) + .with(project, user, + requested_path: 'foo', + project_wiki: wiki, + ref: 'bar', + only_path: nil, + xhtml: false) + .and_call_original + + expect_any_instance_of(Banzai::ObjectRenderer) + .to receive(:render).with([note], :note) + + described_class.new(user).execute([note], project, + requested_path: 'foo', + project_wiki: wiki, + ref: 'bar', + only_path: nil, + xhtml: false) + end + end +end |