Refactor MarkupHelper

11 files changed, 67 insertions, 99 deletions

diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb
index c39b630aec6..95036ccf57d 100644
--- a/app/helpers/markup_helper.rb
+++ b/app/helpers/markup_helper.rb
@@ -59,18 +59,22 @@ module MarkupHelper
     fragment.to_html.html_safe
   end
 
-  def markdown(text, context = {})
-    html = markdown_render(text, context)
+  # Return the first line of +text+, up to +max_chars+, after parsing the line
+  # 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
 
-    markup_postprocess(html, context)
+    truncate_visible(md, max_chars || md.length) if md.present?
   end
 
-  def markdown_render(text, context = {})
+  def markdown(text, context = {})
     return "" unless text.present?
 
     context[:project] ||= @project
-
-    Banzai.render(text, context)
+    html = context.delete(:rendered) || markdown_unsafe(text, context)
+    banzai_postprocess(html, context)
   end
 
   def markdown_field(object, field)
@@ -81,85 +85,59 @@ module MarkupHelper
     banzai_postprocess(html, object.banzai_render_context(field))
   end
 
-  def asciidoc_render(text)
-    Gitlab::Asciidoc.render(
-      text,
-      project:      @project,
-      current_user: (current_user if defined?(current_user)),
-
-      # RelativeLinkFilter
-      project_wiki:   @project_wiki,
-      requested_path: @path,
-      ref:            @ref,
-      commit:         @commit
-    )
-  end
-
-  def other_markup_render(file_name, text)
-    Gitlab::OtherMarkup.render(
-      file_name,
-      text,
-      project:      @project,
-      current_user: (current_user if defined?(current_user)),
-
-      # RelativeLinkFilter
-      project_wiki:   @project_wiki,
-      requested_path: @path,
-      ref:            @ref,
-      commit:         @commit
-    )
-  end
-
-  def markup_postprocess(html, context = {})
-    return "" unless html.present?
-
+  def markup(file_name, text, context = {})
     context[:project] ||= @project
-
+    html = context.delete(:rendered) || markup_unsafe(file_name, text, context)
     banzai_postprocess(html, context)
   end
 
-  # Return the first line of +text+, up to +max_chars+, after parsing the line
-  # 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 render_wiki_content(wiki_page)
+    text = wiki_page.content
+    return "" unless text.present?
 
-    truncate_visible(md, max_chars || md.length) if md.present?
-  end
+    context = { pipeline: :wiki, project: @project, project_wiki: @project_wiki, page_slug: wiki_page.slug }
 
-  def render_wiki_content(wiki_page)
-    context = { pipeline: :wiki, project_wiki: @project_wiki, page_slug: wiki_page.slug }
-    case wiki_page.format
-    when :markdown
-      html = markdown_render(wiki_page.content, context)
-    when :asciidoc
-      html = asciidoc_render(wiki_page.content)
-    else
-      return wiki_page.formatted_content.html_safe
-    end
-    markup_postprocess(html, context)
-  end
+    html =
+      case wiki_page.format
+      when :markdown
+        markdown_unsafe(text, context)
+      when :asciidoc
+        asciidoc_unsafe(text)
+      else
+        wiki_page.formatted_content.html_safe
+      end
 
-  def render_markup(file_name, file_content)
-    html = markup_render(file_name, file_content)
-    markup_postprocess(html)
+    banzai_postprocess(html, context)
   end
 
-  def markup_render(file_name, file_content)
+  def markup_unsafe(file_name, text, context = {})
+    return "" unless text.present?
+
     if gitlab_markdown?(file_name)
-      Hamlit::RailsHelpers.preserve(markdown_render(file_content))
+      Hamlit::RailsHelpers.preserve(markdown_unsafe(text, context))
     elsif asciidoc?(file_name)
-      asciidoc_render(file_content)
+      asciidoc_unsafe(text)
     elsif plain?(file_name)
       content_tag :pre, class: 'plain-readme' do
-        file_content
+        text
       end
     else
-      other_markup_render(file_name, file_content)
+      other_markup_unsafe(file_name, text)
     end
   rescue RuntimeError
-    simple_format(file_content)
+    simple_format(text)
+  end
+
+  def markdown_unsafe(text, context = {})
+    Banzai.render(text, context)
+  end
+
+  def asciidoc_unsafe(text)
+    Gitlab::Asciidoc.render(text)
+  end
+
+  def other_markup_unsafe(file_name, text)
+    Gitlab::OtherMarkup.render(file_name, text)
   end
 
   # Returns the text necessary to reference `entity` across projects
@@ -249,6 +227,8 @@ module MarkupHelper
 
   # Calls Banzai.post_process with some common context options
   def banzai_postprocess(html, context = {})
+    return "" unless html.present?
+
     context.merge!(
       current_user:   (current_user if defined?(current_user)),
 
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 328e817cc3f..d12c7d4eb27 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -530,7 +530,7 @@ class Repository
   end
 
   def rendered_readme
-    markup_render(readme.name, readme.data) if readme
+    markup_unsafe(readme.name, readme.data, project: project) if readme
   end
   cache_method :rendered_readme
 
diff --git a/app/views/projects/_readme.html.haml b/app/views/projects/_readme.html.haml
index b4615e084e0..c0d12cbc66e 100644
--- a/app/views/projects/_readme.html.haml
+++ b/app/views/projects/_readme.html.haml
@@ -4,7 +4,7 @@
       - if can?(current_user, :push_code, @project)
         = link_to icon('pencil'), namespace_project_edit_blob_path(@project.namespace, @project, tree_join(@repository.root_ref, readme.name)), class: 'light edit-project-readme'
     .file-content.wiki
-      = markup_postprocess(@repository.rendered_readme)
+      = markup(readme.name, readme.data, rendered: @repository.rendered_readme)
 - else
   .row-content-block.second-block.center
     %h3.page-title
diff --git a/app/views/projects/blob/_markup.html.haml b/app/views/projects/blob/_markup.html.haml
index 4ee4b03ff04..0090f7a11df 100644
--- a/app/views/projects/blob/_markup.html.haml
+++ b/app/views/projects/blob/_markup.html.haml
@@ -1,4 +1,4 @@
 - blob.load_all_data!(@repository)
 
 .file-content.wiki
-  = render_markup(blob.name, blob.data)
+  = markup(blob.name, blob.data)
diff --git a/app/views/projects/blob/preview.html.haml b/app/views/projects/blob/preview.html.haml
index 5cafb644b40..fb46ccc7933 100644
--- a/app/views/projects/blob/preview.html.haml
+++ b/app/views/projects/blob/preview.html.haml
@@ -6,7 +6,7 @@
           = markdown(@content)
     - elsif markup?(@blob.name)
       .file-content.wiki
-        = raw render_markup(@blob.name, @content)
+        = raw markup(@blob.name, @content)
     - else
       .file-content.code.js-syntax-highlight
         - unless @diff_lines.empty?
diff --git a/app/views/projects/tree/_readme.html.haml b/app/views/projects/tree/_readme.html.haml
index 30df6fd50d0..01599060844 100644
--- a/app/views/projects/tree/_readme.html.haml
+++ b/app/views/projects/tree/_readme.html.haml
@@ -5,4 +5,4 @@
       %strong
         = readme.name
   .file-content.wiki
-    = render_markup(readme.name, readme.data)
+    = markup(readme.name, readme.data)
diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml
index f84be600df8..f2fe5742c12 100644
--- a/app/views/search/results/_snippet_blob.html.haml
+++ b/app/views/search/results/_snippet_blob.html.haml
@@ -21,7 +21,7 @@
       .file-content.wiki
         - snippet_chunks.each do |chunk|
           - unless chunk[:data].empty?
-            = render_markup(snippet.file_name, chunk[:data])
+            = markup(snippet.file_name, chunk[:data])
           - else
             .file-content.code
               .nothing-here-block Empty file
diff --git a/app/views/shared/snippets/_blob.html.haml b/app/views/shared/snippets/_blob.html.haml
index 74f71e6cbd1..895c3f1e99d 100644
--- a/app/views/shared/snippets/_blob.html.haml
+++ b/app/views/shared/snippets/_blob.html.haml
@@ -24,6 +24,6 @@
       - if gitlab_markdown?(@snippet.file_name)
         = preserve(markdown_field(@snippet, :content))
       - else
-        = render_markup(@snippet.file_name, @snippet.content)
+        = markup(@snippet.file_name, @snippet.content)
   - else
     = render 'shared/file_highlight', blob: @snippet
diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb
index b157b59f9dd..6a671ab2630 100644
--- a/lib/gitlab/asciidoc.rb
+++ b/lib/gitlab/asciidoc.rb
@@ -14,15 +14,9 @@ module Gitlab
     # Public: Converts the provided Asciidoc markup into HTML.
     #
     # input         - the source text in Asciidoc format
-    # context       - a Hash with the template context:
-    #                 :commit
-    #                 :project
-    #                 :project_wiki
-    #                 :requested_path
-    #                 :ref
     # asciidoc_opts - a Hash of options to pass to the Asciidoctor converter
     #
-    def self.render(input, context, asciidoc_opts = {})
+    def self.render(input, asciidoc_opts = {})
       asciidoc_opts.reverse_merge!(
         safe: :secure,
         backend: :gitlab_html5,
diff --git a/lib/gitlab/other_markup.rb b/lib/gitlab/other_markup.rb
index 214dba0fa11..c2adc9aa10b 100644
--- a/lib/gitlab/other_markup.rb
+++ b/lib/gitlab/other_markup.rb
@@ -4,14 +4,8 @@ module Gitlab
     # Public: Converts the provided markup into HTML.
     #
     # input         - the source text in a markup format
-    # context       - a Hash with the template context:
-    #                 :commit
-    #                 :project
-    #                 :project_wiki
-    #                 :requested_path
-    #                 :ref
     #
-    def self.render(file_name, input, context)
+    def self.render(file_name, input)
       html = GitHub::Markup.render(file_name, input).
         force_encoding(input.encoding)
 
diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb
index 59ae6570ae7..bba4f4fdc8e 100644
--- a/spec/helpers/markup_helper_spec.rb
+++ b/spec/helpers/markup_helper_spec.rb
@@ -126,7 +126,7 @@ describe MarkupHelper do
     it "uses Wiki pipeline for markdown files" do
       allow(@wiki).to receive(:format).and_return(:markdown)
 
-      expect(helper).to receive(:markdown_render).with('wiki content', pipeline: :wiki, project_wiki: @wiki, page_slug: "nested/page")
+      expect(helper).to receive(:markdown_unsafe).with('wiki content', pipeline: :wiki, project_wiki: @wiki, page_slug: "nested/page")
 
       helper.render_wiki_content(@wiki)
     end
@@ -135,7 +135,7 @@ describe MarkupHelper do
       allow_any_instance_of(ApplicationSetting).to receive(:current).and_return(::ApplicationSetting.create_from_defaults)
       allow(@wiki).to receive(:format).and_return(:asciidoc)
 
-      expect(helper).to receive(:asciidoc_render).with('wiki content')
+      expect(helper).to receive(:asciidoc_unsafe).with('wiki content')
 
       helper.render_wiki_content(@wiki)
     end
@@ -150,26 +150,26 @@ describe MarkupHelper do
     end
   end
 
-  describe 'render_markup' do
+  describe 'markup' do
     let(:content) { 'Noël' }
 
     it 'preserves encoding' do
       expect(content.encoding.name).to eq('UTF-8')
-      expect(helper.render_markup('foo.rst', content).encoding.name).to eq('UTF-8')
+      expect(helper.markup('foo.rst', content).encoding.name).to eq('UTF-8')
     end
 
-    it "delegates to #markdown_render when file name corresponds to Markdown" do
+    it "delegates to #markdown_unsafe when file name corresponds to Markdown" do
       expect(helper).to receive(:gitlab_markdown?).with('foo.md').and_return(true)
-      expect(helper).to receive(:markdown_render).and_return('NOEL')
+      expect(helper).to receive(:markdown_unsafe).and_return('NOEL')
 
-      expect(helper.render_markup('foo.md', content)).to eq('NOEL')
+      expect(helper.markup('foo.md', content)).to eq('NOEL')
     end
 
-    it "delegates to #asciidoc_render when file name corresponds to AsciiDoc" do
+    it "delegates to #asciidoc_unsafe when file name corresponds to AsciiDoc" do
       expect(helper).to receive(:asciidoc?).with('foo.adoc').and_return(true)
-      expect(helper).to receive(:asciidoc_render).and_return('NOEL')
+      expect(helper).to receive(:asciidoc_unsafe).and_return('NOEL')
 
-      expect(helper.render_markup('foo.adoc', content)).to eq('NOEL')
+      expect(helper.markup('foo.adoc', content)).to eq('NOEL')
     end
   end