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..966db96f951 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, project.path_with_namespace) { 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