Merge branch '24224-fix-project-ref-cache' into 'master'

Fix lookup of project by unknown ref when caching is enabled

Closes #24224

See merge request !7988

changelogs/unreleased/24224-fix-project-ref-cache.yml

+---
+title: Fix lookup of project by unknown ref when caching is enabled
+merge_request: 7988
+author:

lib/banzai/filter/abstract_reference_filter.rb

@@ -254,15 +254,26 @@ module Banzai
       # Returns projects for the given paths.
       def find_projects_for_paths(paths)
         if RequestStore.active?
-          to_query = paths - project_refs_cache.keys
+          cache = project_refs_cache
+          to_query = paths - cache.keys
 
           unless to_query.empty?
-            projects_relation_for_paths(to_query).each do |project|
-              get_or_set_cache(project_refs_cache, project.path_with_namespace) { project }
+            projects = projects_relation_for_paths(to_query)
+
+            found = []
+            projects.each do |project|
+              ref = project.path_with_namespace
+              get_or_set_cache(cache, ref) { project }
+              found << ref
+            end
+
+            not_found = to_query - found
+            not_found.each do |ref|
+              get_or_set_cache(cache, ref) { nil }
             end
           end
 
-          project_refs_cache.slice(*paths).values
+          cache.slice(*paths).values.compact
         else
           projects_relation_for_paths(paths)
         end

spec/lib/banzai/filter/abstract_link_filter_spec.rb → spec/lib/banzai/filter/abstract_reference_filter_spec.rb

@@ -32,12 +32,63 @@ describe Banzai::Filter::AbstractReferenceFilter do
   end
 
   describe '#find_projects_for_paths' do
-    it 'returns a list of Projects for a list of paths' do
-      doc = Nokogiri::HTML.fragment('')
-      filter = described_class.new(doc, project: project)
+    let(:doc) { Nokogiri::HTML.fragment('') }
+    let(:filter) { described_class.new(doc, project: project) }
+
+    context 'with RequestStore disabled' do
+      it 'returns a list of Projects for a list of paths' do
+        expect(filter.find_projects_for_paths([project.path_with_namespace])).
+          to eq([project])
+      end
+
+      it "return an empty array for paths that don't exist" do
+        expect(filter.find_projects_for_paths(['nonexistent/project'])).
+          to eq([])
+      end
+    end
+
+    context 'with RequestStore enabled' do
+      before do
+        RequestStore.begin!
+      end
+
+      after do
+        RequestStore.end!
+        RequestStore.clear!
+      end
+
+      it 'returns a list of Projects for a list of paths' do
+        expect(filter.find_projects_for_paths([project.path_with_namespace])).
+          to eq([project])
+      end
+
+      context "when no project with that path exists" do
+        it "returns no value" do
+          expect(filter.find_projects_for_paths(['nonexistent/project'])).
+            to eq([])
+        end
+
+        it "adds the ref to the project refs cache" do
+          project_refs_cache = {}
+          allow(filter).to receive(:project_refs_cache).and_return(project_refs_cache)
+
+          filter.find_projects_for_paths(['nonexistent/project'])
+
+          expect(project_refs_cache).to eq({ 'nonexistent/project' => nil })
+        end
+
+        context 'when the project refs cache includes nil values' do
+          before do
+            # adds { 'nonexistent/project' => nil } to cache
+            filter.project_from_ref_cached('nonexistent/project')
+          end
 
-      expect(filter.find_projects_for_paths([project.path_with_namespace])).
-        to eq([project])
+          it "return an empty array for paths that don't exist" do
+            expect(filter.find_projects_for_paths(['nonexistent/project'])).
+              to eq([])
+          end
+        end
+      end
     end
   end