Merge branch '228626-reduce-cached-queries-for-banzai-filters' into 'master'

Reduce  'cached' calls for Banzai

Closes #228626

See merge request gitlab-org/gitlab!36735

changelogs/unreleased/217580-improve-performance-of-blob.yml

+---
+title: Reduce 'cached' query calls for Banzai
+merge_request: 36735
+author:
+type: performance

lib/banzai/filter/label_reference_filter.rb

@@ -19,7 +19,7 @@ module Banzai
         unescaped_html = unescape_html_entities(text).gsub(pattern) do |match|
           namespace, project = $~[:namespace], $~[:project]
           project_path = full_project_path(namespace, project)
-          label = find_label(project_path, $~[:label_id], $~[:label_name])
+          label = find_label_cached(project_path, $~[:label_id], $~[:label_name])
 
           if label
             labels[label.id] = yield match, label.id, project, namespace, $~
@@ -34,6 +34,12 @@ module Banzai
         escape_with_placeholders(unescaped_html, labels)
       end
 
+      def find_label_cached(parent_ref, label_id, label_name)
+        cached_call(:banzai_find_label_cached, label_name&.tr('"', '') || label_id, path: [object_class, parent_ref]) do
+          find_label(parent_ref, label_id, label_name)
+        end
+      end
+
       def find_label(parent_ref, label_id, label_name)
         parent = parent_from_ref(parent_ref)
         return unless parent

lib/banzai/reference_parser/base_parser.rb

@@ -169,7 +169,8 @@ module Banzai
       # been queried the object is returned from the cache.
       def collection_objects_for_ids(collection, ids)
         if Gitlab::SafeRequestStore.active?
-          ids = ids.map(&:to_i)
+          ids = ids.map(&:to_i).uniq
+
           cache = collection_cache[collection_cache_key(collection)]
           to_query = ids - cache.keys
 

lib/banzai/reference_parser/snippet_parser.rb

@@ -9,10 +9,23 @@ module Banzai
         Snippet
       end
 
+      # Returns all the nodes that are visible to the given user.
+      def nodes_visible_to_user(user, nodes)
+        snippets = lazy { grouped_objects_for_nodes(nodes, references_relation, self.class.data_attribute) }
+
+        nodes.select do |node|
+          if node.has_attribute?(self.class.data_attribute)
+            can_read_reference?(user, snippets[node])
+          else
+            true
+          end
+        end
+      end
+
       private
 
-      def can_read_reference?(user, ref_project, node)
-        can?(user, :read_snippet, referenced_by([node]).first)
+      def can_read_reference?(user, snippet)
+        can?(user, :read_snippet, snippet)
       end
     end
   end

spec/lib/banzai/filter/label_reference_filter_spec.rb

@@ -31,6 +31,19 @@ RSpec.describe Banzai::Filter::LabelReferenceFilter do
     expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-label has-tooltip gl-link gl-label-link'
   end
 
+  it 'avoids N+1 cached queries', :use_sql_query_cache, :request_store do
+    # Run this once to establish a baseline
+    reference_filter("Label #{reference}")
+
+    control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+      reference_filter("Label #{reference}")
+    end
+
+    labels_markdown = Array.new(10, "Label #{reference}").join('\n')
+
+    expect { reference_filter(labels_markdown) }.not_to exceed_all_query_limit(control_count.count)
+  end
+
   it 'includes a data-project attribute' do
     doc = reference_filter("Label #{reference}")
     link = doc.css('a').first

spec/lib/banzai/reference_parser/snippet_parser_spec.rb

@@ -33,6 +33,17 @@ RSpec.describe Banzai::ReferenceParser::SnippetParser do
         project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED)
       end
 
+      it 'avoids N+1 cached queries', :use_sql_query_cache do
+        # Run this once to establish a baseline
+        visible_references(:public)
+
+        control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+          subject.nodes_visible_to_user(user, [link])
+        end
+
+        expect { subject.nodes_visible_to_user(user, Array.new(10, link)) }.not_to exceed_all_query_limit(control_count.count)
+      end
+
       it 'creates a reference for guest for a public snippet' do
         expect(visible_references(:public)).to eq([link])
       end