Cache rendered contents of issues, MRs and notes

8 files changed, 51 insertions, 31 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index feee8460b86..e260eae8a43 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -46,7 +46,8 @@ module Issuable
              allow_nil: true,
              prefix: true
 
-    attr_mentionable :title, :description
+    attr_mentionable :title 
+    attr_mentionable :description, cache: true
     participant :author, :assignee, :notes
   end
 
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index 715fc6f689d..ad432144bd9 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -10,8 +10,9 @@ module Mentionable
 
   module ClassMethods
     # Indicate which attributes of the Mentionable to search for GFM references.
-    def attr_mentionable(*attrs)
-      mentionable_attrs.concat(attrs.map(&:to_s))
+    def attr_mentionable(attr, options = {})
+      attr = attr.to_s
+      mentionable_attrs << [attr, options]
     end
 
     # Accessor for attributes marked mentionable.
@@ -37,11 +38,6 @@ module Mentionable
     "#{friendly_name} #{to_reference(from_project)}"
   end
 
-  # Construct a String that contains possible GFM references.
-  def mentionable_text
-    self.class.mentionable_attrs.map { |attr| send(attr) }.compact.join("\n\n")
-  end
-
   # The GFM reference to this Mentionable, which shouldn't be included in its #references.
   def local_reference
     self
@@ -54,20 +50,33 @@ module Mentionable
   end
 
   def mentioned_users(current_user = nil, load_lazy_references: true)
-    return [] if mentionable_text.blank?
-
+    # TODO: Douwe: Will be simplified when the "Simplify ..." MR is merged.
     ext = Gitlab::ReferenceExtractor.new(self.project, current_user, load_lazy_references: load_lazy_references)
-    ext.analyze(mentionable_text)
-    ext.users.uniq
+    self.class.mentionable_attrs.each do |attr, options|
+      text = send(attr)
+      cache_key = [self, attr] if options[:cache]
+      ext.analyze(text, cache_key: cache_key, pipeline: options[:pipeline])
+    end
+    ext.users
   end
 
   # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
-  def references(p = project, current_user = self.author, text = mentionable_text, load_lazy_references: true)
+  def references(p = project, current_user = self.author, text = nil, load_lazy_references: true)
     return [] if text.blank?
 
     ext = Gitlab::ReferenceExtractor.new(p, current_user, load_lazy_references: load_lazy_references)
-    ext.analyze(text)
-    (ext.issues + ext.merge_requests + ext.commits).uniq - [local_reference]
+
+    if text
+      ext.analyze(text)
+    else
+      self.class.mentionable_attrs.each do |attr, options|
+        text = send(attr)
+        cache_key = [self, attr] if options[:cache]
+        ext.analyze(text, cache_key: cache_key)
+      end
+    end
+
+    (ext.issues + ext.merge_requests + ext.commits) - [local_reference]
   end
 
   # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+.
@@ -111,7 +120,7 @@ module Mentionable
   def detect_mentionable_changes
     source = (changes.present? ? changes : previous_changes).dup
 
-    mentionable = self.class.mentionable_attrs
+    mentionable = self.class.mentionable_attrs.map { |attr, options| attr }
 
     # Only include changed fields that are mentionable
     source.select { |key, val| mentionable.include?(key) }
diff --git a/app/models/note.rb b/app/models/note.rb
index 2fbe4784159..b4d6ddba703 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -28,7 +28,7 @@ class Note < ActiveRecord::Base
 
   default_value_for :system, false
 
-  attr_mentionable :note
+  attr_mentionable :note, cache: true, pipeline: :note
   participant :author
 
   belongs_to :project
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 3233c6884cc..e27f20bdfb8 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -43,7 +43,7 @@
           .description{class: can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : ''}
             .wiki
               = preserve do
-                = markdown(@issue.description)
+                = markdown(@issue.description, cache_key: [@issue, "description"])
             %textarea.hidden.js-task-list-field
               = @issue.description
 
diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml
index 448230a377c..9bfe202589e 100644
--- a/app/views/projects/merge_requests/show/_mr_box.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_box.html.haml
@@ -7,6 +7,6 @@
       .description{class: can?(current_user, :update_merge_request, @merge_request) ? 'js-task-list-container' : ''}
         .wiki
           = preserve do
-            = markdown(@merge_request.description)
+            = markdown(@merge_request.description, cache_key: [@merge_request, "description"])
         %textarea.hidden.js-task-list-field
           = @merge_request.description
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 1638ad6891a..bcb42d39c91 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -58,7 +58,7 @@
       .note-body{class: note_editable?(note) ? 'js-task-list-container' : ''}
         .note-text
           = preserve do
-            = markdown(note.note, {no_header_anchors: true})
+            = markdown(note.note, pipeline: :note, cache_key: [note, "note"])
         - unless note.system?
           -# System notes can't be edited
           = render 'projects/notes/edit_form', note: note
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index d1f22070cba..b9615f95012 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -20,6 +20,8 @@ module Gitlab
     #
     # Returns an HTML-safe String
     def self.render(text, context = {})
+      context[:pipeline] ||= :full
+
       cache_key = context.delete(:cache_key)
       cache_key = full_cache_key(cache_key, context[:pipeline])
 
@@ -33,7 +35,7 @@ module Gitlab
     end
 
     def self.render_result(text, context = {})
-      pipeline = context[:pipeline] || :full
+      pipeline = context[:pipeline] ||= :full
 
       html_pipeline = html_pipelines[pipeline]
 
@@ -129,6 +131,7 @@ module Gitlab
         atom:           :full,
         email:          :full,
         description:    :full,
+        note:           :full,
         single_line:    :gfm,
 
         asciidoc: [
@@ -170,6 +173,13 @@ module Gitlab
             only_path: false
           }
         ],
+        note: [
+          :full,
+          {
+            # TableOfContentsFilter
+            no_header_anchors: true
+          }
+        ],
         description: [
           :full,
           { 
diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb
index f34bf7d1d0e..37141efb4c3 100644
--- a/lib/gitlab/reference_extractor.rb
+++ b/lib/gitlab/reference_extractor.rb
@@ -9,13 +9,12 @@ module Gitlab
       @project = project
       @current_user = current_user
       @load_lazy_references = load_lazy_references
-    end
 
-    def analyze(text, cache_key: nil)
-      references.clear
+      @texts = []
+    end
 
-      @pipeline = Gitlab::Markdown.cached?(cache_key, pipeline: :full) ? :full : :plain_markdown
-      @html = Gitlab::Markdown.render(text, project: project, cache_key: cache_key, pipeline: @pipeline)
+    def analyze(text, options = {})
+      @texts << Gitlab::Markdown.render(text, options.merge(project: project))
     end
 
     %i(user label issue merge_request snippet commit commit_range).each do |type|
@@ -46,7 +45,7 @@ module Gitlab
       filter = Gitlab::Markdown.const_get(klass)
 
       context = {
-        pipeline: [:reference_extraction],
+        pipeline: :reference_extraction,
 
         project:      project,
         current_user: current_user,
@@ -56,10 +55,11 @@ module Gitlab
         reference_filter:     filter
       }
 
-      context[:pipeline].unshift(filter) unless @pipeline == :full
-
-      result = Gitlab::Markdown.render_result(@html, context)
-      values = result[:references][filter_type].uniq
+      values = @texts.flat_map do |html|
+        text_context = context.dup
+        result = Gitlab::Markdown.render_result(html, text_context)
+        result[:references][filter_type]
+      end.uniq
 
       if @load_lazy_references
         values = Gitlab::Markdown::ReferenceFilter::LazyReference.load(values).uniq