Merge branch 'rendering-markdown-multiple-projects' into 'master'

Optimise rendering of Markdown documents that belong to different projects

See merge request gitlab-org/gitlab-ce!18157

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