Skip to content

Commit

Permalink
Resolve "DashboardController#activity.json is slow due to SQL"
Browse files Browse the repository at this point in the history
  • Loading branch information
Francisco Javier López authored and Douwe Maan committed Nov 6, 2017
1 parent 34a205b commit bf0331d
Show file tree
Hide file tree
Showing 36 changed files with 429 additions and 225 deletions.
4 changes: 2 additions & 2 deletions app/controllers/concerns/notes_actions.rb
Expand Up @@ -39,7 +39,7 @@ def create
@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|
Expand All @@ -52,7 +52,7 @@ def update
@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|
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/renders_notes.rb
Expand Up @@ -3,7 +3,7 @@ def prepare_notes_for_rendering(notes, noteable = nil)
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
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/dashboard/projects_controller.rb
Expand Up @@ -57,5 +57,7 @@ def load_events
@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
2 changes: 2 additions & 0 deletions app/controllers/dashboard_controller.rb
Expand Up @@ -32,6 +32,8 @@ def load_events
@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
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/groups_controller.rb
Expand Up @@ -155,6 +155,8 @@ def load_events
@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
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/projects_controller.rb
Expand Up @@ -300,6 +300,8 @@ def load_events
@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
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/users_controller.rb
Expand Up @@ -108,6 +108,8 @@ def load_events
.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
Expand Down
10 changes: 0 additions & 10 deletions app/helpers/events_helper.rb
Expand Up @@ -172,16 +172,6 @@ def event_note_title_html(event)
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)
Expand Down
20 changes: 14 additions & 6 deletions app/helpers/markup_helper.rb
Expand Up @@ -69,10 +69,16 @@ def link_to_html(redacted, url, html_options = {})
# 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 = {})
Expand All @@ -83,15 +89,17 @@ def markdown(text, context = {})
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 = {})
Expand Down
7 changes: 7 additions & 0 deletions 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
21 changes: 21 additions & 0 deletions 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
21 changes: 21 additions & 0 deletions 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
2 changes: 1 addition & 1 deletion app/views/dashboard/todos/_todo.html.haml
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion 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)
2 changes: 1 addition & 1 deletion app/views/events/event/_note.html.haml
Expand Up @@ -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?
Expand Down
@@ -0,0 +1,5 @@
---
title: Improve DashboardController#activity.json performance
merge_request: 14985
author:
type: performance
1 change: 0 additions & 1 deletion config/initializers/8_metrics.rb
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/banzai.rb
Expand Up @@ -3,8 +3,8 @@ def self.render(text, context = {})
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)
Expand Down
34 changes: 34 additions & 0 deletions 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
24 changes: 0 additions & 24 deletions lib/banzai/filter/abstract_reference_filter.rb
Expand Up @@ -311,30 +311,6 @@ def full_project_path(namespace, project_ref)
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
2 changes: 2 additions & 0 deletions lib/banzai/filter/reference_filter.rb
Expand Up @@ -8,6 +8,8 @@ module Filter
# :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
Expand Down
15 changes: 11 additions & 4 deletions lib/banzai/filter/user_reference_filter.rb
Expand Up @@ -60,10 +60,14 @@ def user_link_filter(text, link_content: nil)
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
Expand All @@ -74,7 +78,10 @@ def user_link_filter(text, link_content: nil)
# 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.
Expand Down
21 changes: 0 additions & 21 deletions lib/banzai/note_renderer.rb

This file was deleted.

7 changes: 6 additions & 1 deletion lib/banzai/object_renderer.rb
Expand Up @@ -37,7 +37,7 @@ def render(objects, attribute)

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
Expand Down Expand Up @@ -83,5 +83,10 @@ def base_context
skip_redaction: true
)
end

def save_options
return {} unless base_context[:xhtml]
{ save_with: Nokogiri::XML::Node::SaveOptions::AS_XHTML }
end
end
end
3 changes: 2 additions & 1 deletion lib/banzai/pipeline/post_process_pipeline.rb
Expand Up @@ -3,9 +3,10 @@ module Pipeline
class PostProcessPipeline < BasePipeline
def self.filters
FilterArray[
Filter::RedactorFilter,
Filter::RelativeLinkFilter,
Filter::IssuableStateFilter,
Filter::RedactorFilter
Filter::AbsoluteLinkFilter
]
end

Expand Down

0 comments on commit bf0331d

Please sign in to comment.