diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2018-04-03 15:45:17 +0200 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2018-04-11 14:10:19 +0200 |
commit | daad7144ec7c0173439eeadd61590442e40a6051 (patch) | |
tree | 80a738c17196d0235cbc4c5589d6b04f96688b5c /lib/banzai | |
parent | 23fb465c75d00fd7156a540b7421a79e22df3966 (diff) | |
download | gitlab-ce-daad7144ec7c0173439eeadd61590442e40a6051.tar.gz |
Support Markdown rendering using multiple projectsrendering-markdown-multiple-projects
This refactors the Markdown pipeline so it supports the rendering of
multiple documents that may belong to different projects. An example of
where this happens is when displaying the event feed of a group. In this
case we retrieve events for all projects in the group. Previously we
would group events per project and render these chunks separately, but
this would result in many SQL queries being executed. By extending the
Markdown pipeline to support this out of the box we can drastically
reduce the number of SQL queries.
To achieve this we introduce a new object to the pipeline:
Banzai::RenderContext. This object simply wraps two other objects: an
optional Project instance, and an optional User instance. On its own
this wouldn't be very helpful, but a RenderContext can also be used to
associate HTML documents with specific Project instances. This work is
done in Banzai::ObjectRenderer and allows us to reuse as many queries
(and results) as possible.
Diffstat (limited to 'lib/banzai')
-rw-r--r-- | lib/banzai/commit_renderer.rb | 2 | ||||
-rw-r--r-- | lib/banzai/filter/issuable_state_filter.rb | 3 | ||||
-rw-r--r-- | lib/banzai/filter/redactor_filter.rb | 6 | ||||
-rw-r--r-- | lib/banzai/issuable_extractor.rb | 14 | ||||
-rw-r--r-- | lib/banzai/object_renderer.rb | 32 | ||||
-rw-r--r-- | lib/banzai/redactor.rb | 20 | ||||
-rw-r--r-- | lib/banzai/reference_extractor.rb | 4 | ||||
-rw-r--r-- | lib/banzai/reference_parser/base_parser.rb | 16 | ||||
-rw-r--r-- | lib/banzai/reference_parser/issue_parser.rb | 39 | ||||
-rw-r--r-- | lib/banzai/reference_parser/user_parser.rb | 3 | ||||
-rw-r--r-- | lib/banzai/render_context.rb | 32 |
11 files changed, 123 insertions, 48 deletions
diff --git a/lib/banzai/commit_renderer.rb b/lib/banzai/commit_renderer.rb index f5ff95e3eb3..c351a155ae5 100644 --- a/lib/banzai/commit_renderer.rb +++ b/lib/banzai/commit_renderer.rb @@ -3,7 +3,7 @@ module Banzai ATTRIBUTES = [:description, :title].freeze def self.render(commits, project, user = nil) - obj_renderer = ObjectRenderer.new(project, user) + obj_renderer = ObjectRenderer.new(user: user, default_project: project) ATTRIBUTES.each { |attr| obj_renderer.render(commits, attr) } end diff --git a/lib/banzai/filter/issuable_state_filter.rb b/lib/banzai/filter/issuable_state_filter.rb index 8f541dcfdb2..1a415232545 100644 --- a/lib/banzai/filter/issuable_state_filter.rb +++ b/lib/banzai/filter/issuable_state_filter.rb @@ -11,7 +11,8 @@ module Banzai def call return doc unless context[:issuable_state_filter_enabled] - extractor = Banzai::IssuableExtractor.new(project, current_user) + context = RenderContext.new(project, current_user) + extractor = Banzai::IssuableExtractor.new(context) issuables = extractor.extract([doc]) issuables.each do |node, issuable| diff --git a/lib/banzai/filter/redactor_filter.rb b/lib/banzai/filter/redactor_filter.rb index 9f9882b3b40..caf11fe94c4 100644 --- a/lib/banzai/filter/redactor_filter.rb +++ b/lib/banzai/filter/redactor_filter.rb @@ -7,7 +7,11 @@ module Banzai # class RedactorFilter < HTML::Pipeline::Filter def call - Redactor.new(project, current_user).redact([doc]) unless context[:skip_redaction] + unless context[:skip_redaction] + context = RenderContext.new(project, current_user) + + Redactor.new(context).redact([doc]) + end doc end diff --git a/lib/banzai/issuable_extractor.rb b/lib/banzai/issuable_extractor.rb index 49603d0b363..ae7dc71e7eb 100644 --- a/lib/banzai/issuable_extractor.rb +++ b/lib/banzai/issuable_extractor.rb @@ -12,11 +12,11 @@ module Banzai [@data-reference-type="issue" or @data-reference-type="merge_request"] ).freeze - attr_reader :project, :user + attr_reader :context - def initialize(project, user) - @project = project - @user = user + # context - An instance of Banzai::RenderContext. + def initialize(context) + @context = context end # Returns Hash in the form { node => issuable_instance } @@ -25,8 +25,10 @@ module Banzai document.xpath(QUERY) end - issue_parser = Banzai::ReferenceParser::IssueParser.new(project, user) - merge_request_parser = Banzai::ReferenceParser::MergeRequestParser.new(project, user) + issue_parser = Banzai::ReferenceParser::IssueParser.new(context) + + merge_request_parser = + Banzai::ReferenceParser::MergeRequestParser.new(context) issuables_for_nodes = issue_parser.records_for_nodes(nodes).merge( merge_request_parser.records_for_nodes(nodes) diff --git a/lib/banzai/object_renderer.rb b/lib/banzai/object_renderer.rb index 2691be81623..a176f1e261b 100644 --- a/lib/banzai/object_renderer.rb +++ b/lib/banzai/object_renderer.rb @@ -13,14 +13,13 @@ module Banzai # As an example, rendering the attribute `note` would place the unredacted # HTML into `note_html` and the redacted HTML into `redacted_note_html`. class ObjectRenderer - attr_reader :project, :user + attr_reader :context - # project - A Project to use for redacting Markdown. + # default_project - A default Project to use for redacting Markdown. # user - The user viewing the Markdown/HTML documents, if any. # redaction_context - A Hash containing extra attributes to use during redaction - def initialize(project, user = nil, redaction_context = {}) - @project = project - @user = user + def initialize(default_project: nil, user: nil, redaction_context: {}) + @context = RenderContext.new(default_project, user) @redaction_context = base_context.merge(redaction_context) end @@ -48,17 +47,21 @@ module Banzai pipeline = HTML::Pipeline.new([]) objects.map do |object| - pipeline.to_document(Banzai.render_field(object, attribute)) + document = pipeline.to_document(Banzai.render_field(object, attribute)) + + context.associate_document(document, object) + + document end end def post_process_documents(documents, objects, attribute) # Called here to populate cache, refer to IssuableExtractor docs - IssuableExtractor.new(project, user).extract(documents) + IssuableExtractor.new(context).extract(documents) documents.zip(objects).map do |document, object| - context = context_for(object, attribute) - Banzai::Pipeline[:post_process].to_document(document, context) + pipeline_context = context_for(document, object, attribute) + Banzai::Pipeline[:post_process].to_document(document, pipeline_context) end end @@ -66,20 +69,21 @@ module Banzai # # Returns an Array containing the redacted documents. def redact_documents(documents) - redactor = Redactor.new(project, user) + redactor = Redactor.new(context) redactor.redact(documents) end # Returns a Banzai context for the given object and attribute. - def context_for(object, attribute) - @redaction_context.merge(object.banzai_render_context(attribute)) + def context_for(document, object, attribute) + @redaction_context.merge(object.banzai_render_context(attribute)).merge( + project: context.project_for_node(document) + ) end def base_context { - current_user: user, - project: project, + current_user: context.current_user, skip_redaction: true } end diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb index fd457bebf03..28928d6f376 100644 --- a/lib/banzai/redactor.rb +++ b/lib/banzai/redactor.rb @@ -2,13 +2,15 @@ module Banzai # Class for removing Markdown references a certain user is not allowed to # view. class Redactor - attr_reader :user, :project + attr_reader :context - # project - A Project to use for redacting links. - # user - The currently logged in user (if any). - def initialize(project, user = nil) - @project = project - @user = user + # context - An instance of `Banzai::RenderContext`. + def initialize(context) + @context = context + end + + def user + context.current_user end # Redacts the references in the given Array of documents. @@ -70,11 +72,11 @@ module Banzai end def redact_cross_project_references(documents) - extractor = Banzai::IssuableExtractor.new(project, user) + extractor = Banzai::IssuableExtractor.new(context) issuables = extractor.extract(documents) issuables.each do |node, issuable| - next if issuable.project == project + next if issuable.project == context.project_for_node(node) node['class'] = node['class'].gsub('has-tooltip', '') node['title'] = nil @@ -95,7 +97,7 @@ module Banzai end per_type.each do |type, nodes| - parser = Banzai::ReferenceParser[type].new(project, user) + parser = Banzai::ReferenceParser[type].new(context) visible.merge(parser.nodes_visible_to_user(user, nodes)) end diff --git a/lib/banzai/reference_extractor.rb b/lib/banzai/reference_extractor.rb index 7e6357f8a00..78588299c18 100644 --- a/lib/banzai/reference_extractor.rb +++ b/lib/banzai/reference_extractor.rb @@ -10,8 +10,8 @@ module Banzai end def references(type, project, current_user = nil) - processor = Banzai::ReferenceParser[type] - .new(project, current_user) + context = RenderContext.new(project, current_user) + processor = Banzai::ReferenceParser[type].new(context) processor.process(html_documents) end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index 279fca8d043..68752f5bb5a 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -45,9 +45,13 @@ module Banzai @data_attribute ||= "data-#{reference_type.to_s.dasherize}" end - def initialize(project = nil, current_user = nil) - @project = project - @current_user = current_user + # context - An instance of `Banzai::RenderContext`. + def initialize(context) + @context = context + end + + def project_for_node(node) + context.project_for_node(node) end # Returns all the nodes containing references that the user can refer to. @@ -224,7 +228,11 @@ module Banzai private - attr_reader :current_user, :project + attr_reader :context + + def current_user + context.current_user + end # When a feature is disabled or visible only for # team members we should not allow team members diff --git a/lib/banzai/reference_parser/issue_parser.rb b/lib/banzai/reference_parser/issue_parser.rb index 230827129b6..6bee5ea15b9 100644 --- a/lib/banzai/reference_parser/issue_parser.rb +++ b/lib/banzai/reference_parser/issue_parser.rb @@ -5,15 +5,10 @@ module Banzai def nodes_visible_to_user(user, nodes) issues = records_for_nodes(nodes) - issues_to_check = issues.values + issues_to_check, cross_project_issues = partition_issues(issues, user) - unless can?(user, :read_cross_project) - issues_to_check, cross_project_issues = issues_to_check.partition do |issue| - issue.project == project - end - end - - readable_issues = Ability.issues_readable_by_user(issues_to_check, user).to_set + readable_issues = + Ability.issues_readable_by_user(issues_to_check, user).to_set nodes.select do |node| issue_in_node = issues[node] @@ -25,7 +20,7 @@ module Banzai # but not the issue. if readable_issues.include?(issue_in_node) true - elsif cross_project_issues&.include?(issue_in_node) + elsif cross_project_issues.include?(issue_in_node) can_read_reference?(user, issue_in_node) else false @@ -33,6 +28,32 @@ module Banzai end end + # issues - A Hash mapping HTML nodes to their corresponding Issue + # instances. + # user - The current User. + def partition_issues(issues, user) + return [issues.values, []] if can?(user, :read_cross_project) + + issues_to_check = [] + cross_project_issues = [] + + # We manually partition the data since our input is a Hash and our + # output has to be an Array of issues; not an Array of (node, issue) + # pairs. + issues.each do |node, issue| + target = + if issue.project == project_for_node(node) + issues_to_check + else + cross_project_issues + end + + target << issue + end + + [issues_to_check, cross_project_issues] + end + def records_for_nodes(nodes) @issues_for_nodes ||= grouped_objects_for_nodes( nodes, diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb index 8932d4f2905..ceb7f1d165c 100644 --- a/lib/banzai/reference_parser/user_parser.rb +++ b/lib/banzai/reference_parser/user_parser.rb @@ -58,7 +58,7 @@ module Banzai def can_read_project_reference?(node) node_id = node.attr('data-project').to_i - project && project.id == node_id + project_for_node(node)&.id == node_id end def nodes_user_can_reference(current_user, nodes) @@ -71,6 +71,7 @@ module Banzai nodes.select do |node| project_id = node.attr(project_attr) user_id = node.attr(author_attr) + project = project_for_node(node) if project && project_id && project.id == project_id.to_i true diff --git a/lib/banzai/render_context.rb b/lib/banzai/render_context.rb new file mode 100644 index 00000000000..e30fc9f469b --- /dev/null +++ b/lib/banzai/render_context.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Banzai + # Object storing the current user, project, and other details used when + # parsing Markdown references. + class RenderContext + attr_reader :current_user + + # default_project - The default project to use for all documents, if any. + # current_user - The user viewing the document, if any. + def initialize(default_project = nil, current_user = nil) + @current_user = current_user + @projects = Hash.new(default_project) + end + + # Associates an HTML document with a Project. + # + # document - The HTML document to map to a Project. + # object - The object that produced the HTML document. + def associate_document(document, object) + # XML nodes respond to "document" but will return a Document instance, + # even when they belong to a DocumentFragment. + document = document.document if document.fragment? + + @projects[document] = object.project if object.respond_to?(:project) + end + + def project_for_node(node) + @projects[node.document] + end + end +end |