Merge branch 'banzai-user-filter-queries'

5 files changed, 99 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6e11626e389..ebbce3315ad 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -36,6 +36,7 @@ v 8.9.0 (unreleased)
 
 v 8.8.4
   - Fix todos page throwing errors when you have a project pending deletion
+  - Reduce number of SQL queries when rendering user references
 
 v 8.8.3
   - Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312
diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb
index 41ae0e1f9cc..2d6f34c9cd8 100644
--- a/lib/banzai/filter/reference_filter.rb
+++ b/lib/banzai/filter/reference_filter.rb
@@ -68,6 +68,8 @@ module Banzai
       # by `ignore_ancestor_query`. Link tags are not processed if they have a
       # "gfm" class or the "href" attribute is empty.
       def each_node
+        return to_enum(__method__) unless block_given?
+
         query = %Q{descendant-or-self::text()[not(#{ignore_ancestor_query})]
         | descendant-or-self::a[
           not(contains(concat(" ", @class, " "), " gfm ")) and not(@href = "")
@@ -78,6 +80,11 @@ module Banzai
         end
       end
 
+      # Returns an Array containing all HTML nodes.
+      def nodes
+        @nodes ||= each_node.to_a
+      end
+
       # Yields the link's URL and text whenever the node is a valid <a> tag.
       def yield_valid_link(node)
         link = CGI.unescape(node.attr('href').to_s)
diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb
index 331d8007257..5b0a6d8541b 100644
--- a/lib/banzai/filter/user_reference_filter.rb
+++ b/lib/banzai/filter/user_reference_filter.rb
@@ -29,7 +29,7 @@ module Banzai
         ref_pattern = User.reference_pattern
         ref_pattern_start = /\A#{ref_pattern}\z/
 
-        each_node do |node|
+        nodes.each do |node|
           if text_node?(node)
             replace_text_when_pattern_matches(node, ref_pattern) do |content|
               user_link_filter(content)
@@ -59,7 +59,7 @@ module Banzai
         self.class.references_in(text) do |match, username|
           if username == 'all'
             link_to_all(link_text: link_text)
-          elsif namespace = Namespace.find_by(path: username)
+          elsif namespace = namespaces[username]
             link_to_namespace(namespace, link_text: link_text) || match
           else
             match
@@ -67,6 +67,31 @@ module Banzai
         end
       end
 
+      # Returns a Hash containing all Namespace objects for the username
+      # references in the current document.
+      #
+      # The keys of this Hash are the namespace paths, the values the
+      # corresponding Namespace objects.
+      def namespaces
+        @namespaces ||=
+          Namespace.where(path: usernames).each_with_object({}) do |row, hash|
+            hash[row.path] = row
+          end
+      end
+
+      # Returns all usernames referenced in the current document.
+      def usernames
+        refs = Set.new
+
+        nodes.each do |node|
+          node.to_html.scan(User.reference_pattern) do
+            refs << $~[:user]
+          end
+        end
+
+        refs.to_a
+      end
+
       private
 
       def urls
diff --git a/spec/lib/banzai/filter/reference_filter_spec.rb b/spec/lib/banzai/filter/reference_filter_spec.rb
new file mode 100644
index 00000000000..55e681f6faf
--- /dev/null
+++ b/spec/lib/banzai/filter/reference_filter_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+describe Banzai::Filter::ReferenceFilter, lib: true do
+  let(:project) { build(:project) }
+
+  describe '#each_node' do
+    it 'iterates over the nodes in a document' do
+      document = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect { |b| filter.each_node(&b) }.
+        to yield_with_args(an_instance_of(Nokogiri::XML::Element))
+    end
+
+    it 'returns an Enumerator when no block is given' do
+      document = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect(filter.each_node).to be_an_instance_of(Enumerator)
+    end
+
+    it 'skips links with a "gfm" class' do
+      document = Nokogiri::HTML.fragment('<a href="foo" class="gfm">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect { |b| filter.each_node(&b) }.not_to yield_control
+    end
+
+    it 'skips text nodes in pre elements' do
+      document = Nokogiri::HTML.fragment('<pre>foo</pre>')
+      filter = described_class.new(document, project: project)
+
+      expect { |b| filter.each_node(&b) }.not_to yield_control
+    end
+  end
+
+  describe '#nodes' do
+    it 'returns an Array of the HTML nodes' do
+      document = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect(filter.nodes).to eq([document.children[0]])
+    end
+  end
+end
diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb
index d7dfd6699ef..108b36a97cc 100644
--- a/spec/lib/banzai/filter/user_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb
@@ -136,4 +136,23 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do
       expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s
     end
   end
+
+  describe '#namespaces' do
+    it 'returns a Hash containing all Namespaces' do
+      document = Nokogiri::HTML.fragment("<p>#{user.to_reference}</p>")
+      filter = described_class.new(document, project: project)
+      ns = user.namespace
+
+      expect(filter.namespaces).to eq({ ns.path => ns })
+    end
+  end
+
+  describe '#usernames' do
+    it 'returns the usernames mentioned in a document' do
+      document = Nokogiri::HTML.fragment("<p>#{user.to_reference}</p>")
+      filter = described_class.new(document, project: project)
+
+      expect(filter.usernames).to eq([user.username])
+    end
+  end
 end