Merge branch 'rs-link-to-performance' into 'master'

Speed up the overridden `link_to` helper

Only bothers to check the provided link's external status if it's a String that doesn't begin with a path or anchor character.

See merge request !1767

app/helpers/application_helper.rb

@@ -236,33 +236,35 @@ module ApplicationHelper
     Gitlab::MarkdownHelper.gitlab_markdown?(filename)
   end
 
-  def link_to(name = nil, options = nil, html_options = nil, &block)
-    begin
-      uri = URI(options)
-      host = uri.host
-      absolute_uri = uri.absolute?
-    rescue URI::InvalidURIError, ArgumentError
-      host = nil
-      absolute_uri = nil
-    end
-
-    # Add 'nofollow' only to external links
-    if host && host != Gitlab.config.gitlab.host && absolute_uri
-      if html_options
-        if html_options[:rel]
-          html_options[:rel] << ' nofollow'
-        else
-          html_options.merge!(rel: 'nofollow')
-        end
-      else
-        html_options = Hash.new
-        html_options[:rel] = 'nofollow'
+  # Overrides ActionView::Helpers::UrlHelper#link_to to add `rel="nofollow"` to
+  # external links
+  def link_to(name = nil, options = nil, html_options = {})
+    if options.kind_of?(String)
+      if !options.start_with?('#', '/')
+        html_options = add_nofollow(options, html_options)
       end
     end
 
     super
   end
 
+  # Add `"rel=nofollow"` to external links
+  #
+  # link         - String link to check
+  # html_options - Hash of `html_options` passed to `link_to`
+  #
+  # Returns `html_options`, adding `rel: nofollow` for external links
+  def add_nofollow(link, html_options = {})
+    uri = URI(link)
+
+    if uri && uri.absolute? && uri.host != Gitlab.config.gitlab.host
+      rel = html_options.fetch(:rel, '')
+      html_options[:rel] = (rel + ' nofollow').strip
+    end
+
+    html_options
+  end
+
   def escaped_autolink(text)
     auto_link ERB::Util.html_escape(text), link: :urls
   end

spec/helpers/application_helper_spec.rb

@@ -225,25 +225,29 @@ describe ApplicationHelper do
   end
 
   describe 'link_to' do
-
     it 'should not include rel=nofollow for internal links' do
-      expect(link_to('Home', root_path)).to eq("<a href=\"/\">Home</a>")
+      expect(link_to('Home', root_path)).to eq('<a href="/">Home</a>')
     end
 
     it 'should include rel=nofollow for external links' do
-      expect(link_to('Example', 'http://www.example.com')).to eq("<a href=\"http://www.example.com\" rel=\"nofollow\">Example</a>")
+      expect(link_to('Example', 'http://www.example.com')).
+        to eq '<a href="http://www.example.com" rel="nofollow">Example</a>'
+    end
+
+    it 'should include rel=nofollow for external links and honor existing html_options' do
+      expect(link_to('Example', 'http://www.example.com', class: 'toggle', data: {toggle: 'dropdown'}))
+        .to eq '<a class="toggle" data-toggle="dropdown" href="http://www.example.com" rel="nofollow">Example</a>'
     end
 
-    it 'should include re=nofollow for external links and honor existing html_options' do
-      expect(
-        link_to('Example', 'http://www.example.com', class: 'toggle', data: {toggle: 'dropdown'})
-      ).to eq("<a class=\"toggle\" data-toggle=\"dropdown\" href=\"http://www.example.com\" rel=\"nofollow\">Example</a>")
+    it 'should include rel=nofollow for external links and preserve other rel values' do
+      expect(link_to('Example', 'http://www.example.com', rel: 'noreferrer'))
+        .to eq '<a href="http://www.example.com" rel="noreferrer nofollow">Example</a>'
     end
 
-    it 'should include rel=nofollow for external links and preserver other rel values' do
-      expect(
-        link_to('Example', 'http://www.example.com', rel: 'noreferrer')
-      ).to eq("<a href=\"http://www.example.com\" rel=\"noreferrer nofollow\">Example</a>")
+    it 'should not include rel=nofollow for external links on the same host as GitLab' do
+      expect(Gitlab.config.gitlab).to receive(:host).and_return('example.foo')
+      expect(link_to('Example', 'http://example.foo/bar')).
+        to eq '<a href="http://example.foo/bar">Example</a>'
     end
   end