summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrancisco Javier López <fjlopez@gitlab.com>2017-11-06 16:52:56 +0000
committerDouwe Maan <douwe@gitlab.com>2017-11-06 16:52:56 +0000
commitbf0331dc72db658b012d58e33ca47da6d8f99d46 (patch)
treef596c4876cd50bf13afd1f3e60ccd57640b61656
parent34a205b308d80d600c5651cfb9694f978ef01cab (diff)
downloadgitlab-ce-bf0331dc72db658b012d58e33ca47da6d8f99d46.tar.gz
Resolve "DashboardController#activity.json is slow due to SQL"
-rw-r--r--app/controllers/concerns/notes_actions.rb4
-rw-r--r--app/controllers/concerns/renders_notes.rb2
-rw-r--r--app/controllers/dashboard/projects_controller.rb2
-rw-r--r--app/controllers/dashboard_controller.rb2
-rw-r--r--app/controllers/groups_controller.rb2
-rw-r--r--app/controllers/projects_controller.rb2
-rw-r--r--app/controllers/users_controller.rb2
-rw-r--r--app/helpers/events_helper.rb10
-rw-r--r--app/helpers/markup_helper.rb20
-rw-r--r--app/services/base_renderer.rb7
-rw-r--r--app/services/events/render_service.rb21
-rw-r--r--app/services/notes/render_service.rb21
-rw-r--r--app/views/dashboard/todos/_todo.html.haml2
-rw-r--r--app/views/events/_event_note.atom.haml2
-rw-r--r--app/views/events/event/_note.html.haml2
-rw-r--r--changelogs/unreleased/27375-dashboard-activity-performance.yml5
-rw-r--r--config/initializers/8_metrics.rb1
-rw-r--r--lib/banzai.rb4
-rw-r--r--lib/banzai/filter/absolute_link_filter.rb34
-rw-r--r--lib/banzai/filter/abstract_reference_filter.rb24
-rw-r--r--lib/banzai/filter/reference_filter.rb2
-rw-r--r--lib/banzai/filter/user_reference_filter.rb15
-rw-r--r--lib/banzai/note_renderer.rb21
-rw-r--r--lib/banzai/object_renderer.rb7
-rw-r--r--lib/banzai/pipeline/post_process_pipeline.rb3
-rw-r--r--lib/banzai/renderer.rb11
-rw-r--r--lib/banzai/request_store_reference_cache.rb27
-rw-r--r--spec/helpers/events_helper_spec.rb90
-rw-r--r--spec/helpers/markup_helper_spec.rb151
-rw-r--r--spec/lib/banzai/commit_renderer_spec.rb2
-rw-r--r--spec/lib/banzai/filter/absolute_link_filter_spec.rb58
-rw-r--r--spec/lib/banzai/note_renderer_spec.rb24
-rw-r--r--spec/lib/banzai/object_renderer_spec.rb4
-rw-r--r--spec/lib/banzai/renderer_spec.rb2
-rw-r--r--spec/services/events/render_service_spec.rb37
-rw-r--r--spec/services/notes/render_service_spec.rb31
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 = "![ImageTest](/uploads/test.png)"
- 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("![ImageTest](/uploads/test.png)")
+ 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