From 2fbfb85492401b2a8ac81f22b319b304afecf6c3 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 30 May 2016 15:55:11 +0200 Subject: [PATCH 1/3] Returning enums in ReferenceFilter#each_node This changes ReferenceFilter#each_node so that when it's called without a block an Enumerator is returned. --- lib/banzai/filter/reference_filter.rb | 2 ++ .../banzai/filter/reference_filter_spec.rb | 36 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 spec/lib/banzai/filter/reference_filter_spec.rb diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index 41ae0e1f9cc..5eace574ab4 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 = "") 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..d3e7af0671c --- /dev/null +++ b/spec/lib/banzai/filter/reference_filter_spec.rb @@ -0,0 +1,36 @@ +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('foo') + 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('foo') + 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('foo') + 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('
foo
') + filter = described_class.new(document, project: project) + + expect { |b| filter.each_node(&b) }.not_to yield_control + end + end +end From 8a6c3f27e9dfea2c151657045e17fe66ad81b5e5 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 30 May 2016 15:56:05 +0200 Subject: [PATCH 2/3] Added ReferenceFilter#nodes This method returns an Array of the HTML nodes as yielded by ReferenceFilter#each_node. The method's return value is memoized to allow multiple calls without having to re-query the input document. --- lib/banzai/filter/reference_filter.rb | 5 +++++ spec/lib/banzai/filter/reference_filter_spec.rb | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index 5eace574ab4..2d6f34c9cd8 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/lib/banzai/filter/reference_filter.rb @@ -80,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 tag. def yield_valid_link(node) link = CGI.unescape(node.attr('href').to_s) diff --git a/spec/lib/banzai/filter/reference_filter_spec.rb b/spec/lib/banzai/filter/reference_filter_spec.rb index d3e7af0671c..55e681f6faf 100644 --- a/spec/lib/banzai/filter/reference_filter_spec.rb +++ b/spec/lib/banzai/filter/reference_filter_spec.rb @@ -33,4 +33,13 @@ describe Banzai::Filter::ReferenceFilter, lib: true do 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('foo') + filter = described_class.new(document, project: project) + + expect(filter.nodes).to eq([document.children[0]]) + end + end end From 01575e9966805fa4c12a7a56361f511b3b61e309 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 30 May 2016 15:56:50 +0200 Subject: [PATCH 3/3] Reduce Namespace queries in UserReferenceFilter This changes UserReferenceFilter so it operates using the following steps: 1. Grab all username references from the input document. 2. Query the corresponding Namespace objects using a single query. 3. Iterate over all nodes to build links while re-using the objects queried in step 2. The impact of these changes is that a comment mentioning 5 different usernames no longer runs 5 different queries (1 for every username), instead it only runs a single query. --- CHANGELOG | 1 + lib/banzai/filter/user_reference_filter.rb | 29 +++++++++++++++++-- .../filter/user_reference_filter_spec.rb | 19 ++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d1cde40c1c7..7e224060227 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -26,6 +26,7 @@ v 8.9.0 (unreleased) - Measure queue duration between gitlab-workhorse and Rails - Make authentication service for Container Registry to be compatible with < Docker 1.11 - Add Application Setting to configure Container Registry token expire delay (default 5min) + - Reduce number of SQL queries when rendering user references v 8.8.3 - Fix incorrect links on pipeline page when merge request created from fork 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/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("

#{user.to_reference}

") + 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("

#{user.to_reference}

") + filter = described_class.new(document, project: project) + + expect(filter.usernames).to eq([user.username]) + end + end end