From 79c92677b2442e378a1f905c618369331398b3c8 Mon Sep 17 00:00:00 2001 From: Dmitry Gruzd Date: Fri, 1 Aug 2025 15:52:58 +0200 Subject: [PATCH 1/2] Fix namespace root ancestor preloader when namespace is not present Changelog: fixed --- .../namespace_root_ancestor_preloader.rb | 5 +++- .../namespace_root_ancestor_preloader_spec.rb | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb b/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb index 467413e0c9ae50..c97de31825d5e8 100644 --- a/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb +++ b/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb @@ -17,7 +17,10 @@ def execute root_ancestors_by_id = root_query.group_by(&:source_id) @namespaces.each do |namespace| - namespace.root_ancestor = root_ancestors_by_id[namespace.id].first + root_ancestor = root_ancestors_by_id[namespace.id]&.first + next unless root_ancestor + + namespace.root_ancestor = root_ancestor end end diff --git a/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb b/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb index be6254009f9edd..8a274f9bc3cd30 100644 --- a/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb +++ b/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb @@ -55,6 +55,36 @@ it_behaves_like 'executes N matching DB queries', 2 end + context 'when namespaces have no root ancestor in query results' do + it 'safely handles namespaces without root ancestors' do + # Create a preloader with an empty namespaces array to simulate + # the scenario where the root query returns no matching records + expect { described_class.new([], additional_preloads).execute }.not_to raise_error + end + + it 'handles case where root_ancestors_by_id lookup returns nil' do + namespace = build(:namespace, id: non_existing_record_id) + allow(namespace).to receive(:id).and_return(non_existing_record_id) + + preloader = described_class.new([namespace], additional_preloads) + + # Mock the Namespace query to return empty results, simulating the + # scenario where no root ancestor is found for the namespace + empty_relation = Namespace.none + allow(Namespace).to receive(:joins).and_return(empty_relation) + allow(empty_relation).to receive_messages( + select: empty_relation, + preload: empty_relation, + group_by: {} + ) + + expect { preloader.execute }.not_to raise_error + + # Verify that the namespace's root_ancestor instance variable was not set + expect(namespace.instance_variable_get(:@root_ancestor)).to be_nil + end + end + def preload_ancestors described_class.new(pristine_namespaces, additional_preloads).execute end -- GitLab From 43be4d6f0f686d661fefb56a0c76dd73d56c6d35 Mon Sep 17 00:00:00 2001 From: Dmitry Gruzd Date: Thu, 7 Aug 2025 14:19:05 +0200 Subject: [PATCH 2/2] Address the feedback: add logger --- .../namespace_root_ancestor_preloader.rb | 21 +++++- .../namespace_root_ancestor_preloader_spec.rb | 66 +++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb b/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb index c97de31825d5e8..76f297e61cf418 100644 --- a/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb +++ b/app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb @@ -3,6 +3,8 @@ module Namespaces module Preloaders class NamespaceRootAncestorPreloader + include Gitlab::Loggable + def initialize(namespaces, root_ancestor_preloads = []) @namespaces = namespaces @root_ancestor_preloads = root_ancestor_preloads @@ -18,14 +20,29 @@ def execute @namespaces.each do |namespace| root_ancestor = root_ancestors_by_id[namespace.id]&.first - next unless root_ancestor - namespace.root_ancestor = root_ancestor + if root_ancestor + namespace.root_ancestor = root_ancestor + else + log_orphaned_namespace(namespace) + end end end private + def log_orphaned_namespace(namespace) + Gitlab::AppLogger.warn( + build_structured_payload( + message: 'Orphaned namespace detected. Unable to find root ancestor', + namespace_id: namespace.id, + namespace_type: namespace.type, + namespace_path: namespace.path, + traversal_ids: namespace.traversal_ids + ) + ) + end + def join_sql Namespace.select('id, traversal_ids[1] as root_id').where(id: @namespaces.map(&:id)).to_sql end diff --git a/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb b/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb index 8a274f9bc3cd30..301a927156531a 100644 --- a/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb +++ b/spec/models/namespaces/preloaders/namespace_root_ancestor_preloader_spec.rb @@ -83,6 +83,72 @@ # Verify that the namespace's root_ancestor instance variable was not set expect(namespace.instance_variable_get(:@root_ancestor)).to be_nil end + + it 'logs orphaned namespace with structured payload when root ancestor is not found' do + orphaned_namespace = build(:namespace, + id: non_existing_record_id, + path: 'orphaned-namespace', + type: 'Group', + traversal_ids: [123, 456] + ) + + preloader = described_class.new([orphaned_namespace], additional_preloads) + + # Mock the Namespace query to return empty results + empty_relation = Namespace.none + allow(Namespace).to receive(:joins).and_return(empty_relation) + allow(empty_relation).to receive_messages( + select: empty_relation, + preload: empty_relation, + group_by: {} + ) + + expected_payload = { + 'class' => 'Namespaces::Preloaders::NamespaceRootAncestorPreloader', + 'message' => 'Orphaned namespace detected. Unable to find root ancestor', + 'namespace_id' => non_existing_record_id, + 'namespace_type' => 'Group', + 'namespace_path' => 'orphaned-namespace', + 'traversal_ids' => [123, 456] + } + + expect(Gitlab::AppLogger).to receive(:warn).with(expected_payload) + + preloader.execute + end + + context 'when multiple orphaned namespaces exist' do + it 'logs each orphaned namespace separately' do + orphaned_namespace1 = build(:namespace, + id: 9999, + path: 'orphaned-1', + type: 'Group', + traversal_ids: [111] + ) + + orphaned_namespace2 = build(:namespace, + id: 8888, + path: 'orphaned-2', + type: 'Project', + traversal_ids: [222] + ) + + preloader = described_class.new([orphaned_namespace1, orphaned_namespace2], additional_preloads) + + # Mock the Namespace query to return empty results + empty_relation = Namespace.none + allow(Namespace).to receive(:joins).and_return(empty_relation) + allow(empty_relation).to receive_messages( + select: empty_relation, + preload: empty_relation, + group_by: {} + ) + + expect(Gitlab::AppLogger).to receive(:warn).twice + + preloader.execute + end + end end def preload_ancestors -- GitLab