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

4 files changed, 122 insertions, 56 deletions

diff --git a/changelogs/unreleased/24224-fix-project-ref-cache.yml b/changelogs/unreleased/24224-fix-project-ref-cache.yml
new file mode 100644
index 00000000000..a6824ee44de
--- /dev/null
+++ b/changelogs/unreleased/24224-fix-project-ref-cache.yml
@@ -0,0 +1,4 @@
+---
+title: Fix lookup of project by unknown ref when caching is enabled
+merge_request: 7988
+author:
diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index fd74eeaebe7..6d04f68c8f9 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/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
diff --git a/spec/lib/banzai/filter/abstract_link_filter_spec.rb b/spec/lib/banzai/filter/abstract_link_filter_spec.rb
deleted file mode 100644
index 70a87fbc01e..00000000000
--- a/spec/lib/banzai/filter/abstract_link_filter_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'spec_helper'
-
-describe Banzai::Filter::AbstractReferenceFilter do
-  let(:project) { create(:empty_project) }
-
-  describe '#references_per_project' do
-    it 'returns a Hash containing references grouped per project paths' do
-      doc = Nokogiri::HTML.fragment("#1 #{project.path_with_namespace}#2")
-      filter = described_class.new(doc, project: project)
-
-      expect(filter).to receive(:object_class).exactly(4).times.and_return(Issue)
-      expect(filter).to receive(:object_sym).twice.and_return(:issue)
-
-      refs = filter.references_per_project
-
-      expect(refs).to be_an_instance_of(Hash)
-      expect(refs[project.path_with_namespace]).to eq(Set.new(%w[1 2]))
-    end
-  end
-
-  describe '#projects_per_reference' do
-    it 'returns a Hash containing projects grouped per project paths' do
-      doc = Nokogiri::HTML.fragment('')
-      filter = described_class.new(doc, project: project)
-
-      expect(filter).to receive(:references_per_project).
-        and_return({ project.path_with_namespace => Set.new(%w[1]) })
-
-      expect(filter.projects_per_reference).
-        to eq({ project.path_with_namespace => project })
-    end
-  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)
-
-      expect(filter.find_projects_for_paths([project.path_with_namespace])).
-        to eq([project])
-    end
-  end
-
-  describe '#current_project_path' do
-    it 'returns the path of the current project' do
-      doc = Nokogiri::HTML.fragment('')
-      filter = described_class.new(doc, project: project)
-
-      expect(filter.current_project_path).to eq(project.path_with_namespace)
-    end
-  end
-end
diff --git a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb
new file mode 100644
index 00000000000..27684882435
--- /dev/null
+++ b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb
@@ -0,0 +1,103 @@
+require 'spec_helper'
+
+describe Banzai::Filter::AbstractReferenceFilter do
+  let(:project) { create(:empty_project) }
+
+  describe '#references_per_project' do
+    it 'returns a Hash containing references grouped per project paths' do
+      doc = Nokogiri::HTML.fragment("#1 #{project.path_with_namespace}#2")
+      filter = described_class.new(doc, project: project)
+
+      expect(filter).to receive(:object_class).exactly(4).times.and_return(Issue)
+      expect(filter).to receive(:object_sym).twice.and_return(:issue)
+
+      refs = filter.references_per_project
+
+      expect(refs).to be_an_instance_of(Hash)
+      expect(refs[project.path_with_namespace]).to eq(Set.new(%w[1 2]))
+    end
+  end
+
+  describe '#projects_per_reference' do
+    it 'returns a Hash containing projects grouped per project paths' do
+      doc = Nokogiri::HTML.fragment('')
+      filter = described_class.new(doc, project: project)
+
+      expect(filter).to receive(:references_per_project).
+        and_return({ project.path_with_namespace => Set.new(%w[1]) })
+
+      expect(filter.projects_per_reference).
+        to eq({ project.path_with_namespace => project })
+    end
+  end
+
+  describe '#find_projects_for_paths' do
+    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
+
+          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
+
+  describe '#current_project_path' do
+    it 'returns the path of the current project' do
+      doc = Nokogiri::HTML.fragment('')
+      filter = described_class.new(doc, project: project)
+
+      expect(filter.current_project_path).to eq(project.path_with_namespace)
+    end
+  end
+end