From 4a894ec8c7a3891a367f521c60e1de0ab43bd21a Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Mon, 15 Dec 2025 18:08:53 +0100 Subject: [PATCH 1/2] Fix FK violation when deleting projects with AI catalog items **Problem** When deleting a project that owns AI catalog items with consumers, a foreign key constraint violation occurred: ```` ERROR: update or delete on table "ai_catalog_items" violates foreign key constraint "fk_bba1649fa5" on table "ai_catalog_item_consumers" (PG::ForeignKeyViolation) ``` ` The issue occurs because: - The project_id FK on ai_catalog_item_consumers has ON DELETE CASCADE - The ai_catalog_item_id FK has ON DELETE RESTRICT - When project deletion cascades to delete consumers, it violates the restrict constraint on the catalog item This blocks deletion of projects that have published public catalog items used by other projects. **Solution** Handle AI catalog data cleanup in Projects::DestroyService: 1. Explicitly delete all item consumers where the project is the consumer BEFORE the project deletion cascade begins 2. For catalog items owned by the project: - If the item has consumers from other projects: soft delete the item and unset the project association (making it organization-owned only) to preserve functionality for other consumers - If the item only has consumers from the same project or no consumers: delete the item normally This allows projects to be deleted without breaking other customers' workflows that depend on public catalog items. Changelog: fixed EE: true --- .../services/ee/projects/destroy_service.rb | 17 +++++++ .../services/projects/destroy_service_spec.rb | 45 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/ee/app/services/ee/projects/destroy_service.rb b/ee/app/services/ee/projects/destroy_service.rb index 5978de80d45cc1..0da79413db0042 100644 --- a/ee/app/services/ee/projects/destroy_service.rb +++ b/ee/app/services/ee/projects/destroy_service.rb @@ -40,6 +40,7 @@ def geo_replicate override :destroy_project_related_records def destroy_project_related_records(project) destroy_compliance_requirement_statuses! + destroy_ai_catalog_items! with_scheduling_epic_cache_update do super && log_destroy_events @@ -125,6 +126,22 @@ def destroy_compliance_requirement_statuses! ::ComplianceManagement::ComplianceFramework::ProjectRequirementComplianceStatus .delete_all_project_statuses(project.id) end + + def destroy_ai_catalog_items! + ::Ai::Catalog::ItemConsumer.for_projects(project).delete_all + + project_owned_items = ::Ai::Catalog::Item.for_project(project) + + project_owned_items.find_each do |item| + external_consumers = item.consumers.not_for_projects(project.id) + + if external_consumers.any? + item.update(project_id: nil, deleted_at: Time.zone.now) + else + item.delete + end + end + end end end end diff --git a/ee/spec/services/projects/destroy_service_spec.rb b/ee/spec/services/projects/destroy_service_spec.rb index 787025c1fce8c1..d51256a0ca6c65 100644 --- a/ee/spec/services/projects/destroy_service_spec.rb +++ b/ee/spec/services/projects/destroy_service_spec.rb @@ -540,4 +540,49 @@ project_destroy_service.execute end end + + context 'when project has AI catalog items with consumers' do + let(:project2) { create(:project, namespace: user.namespace) } + + let!(:ai_catalog_item) { create(:ai_catalog_item, :public, project: project) } + let!(:project_consumer) { create(:ai_catalog_item_consumer, item: ai_catalog_item, project: project) } + let!(:another_project_consumer) { create(:ai_catalog_item_consumer, item: ai_catalog_item, project: project2) } + + context 'when the item has consumers from other projects' do + it 'deletes project consumers, soft deletes catalog item and unsets the project association' do + expect { project_destroy_service.execute } + .to change { Ai::Catalog::ItemConsumer.where(id: project_consumer.id).count }.by(-1) + .and change { ai_catalog_item.reload.project_id }.from(project.id).to(nil) + .and change { ai_catalog_item.reload.deleted_at }.from(nil) + + expect(another_project_consumer.reload).to be_present + + expect(Project.unscoped.all).not_to include(project) + end + end + + context 'when the item only has consumers from the same project' do + let(:another_project_consumer) { nil } + + it 'deletes the item and its consumers via cascade' do + expect { project_destroy_service.execute } + .to change { Ai::Catalog::Item.where(id: ai_catalog_item.id).count }.by(-1) + .and change { Ai::Catalog::ItemConsumer.where(id: project_consumer.id).count }.by(-1) + + expect(Project.unscoped.all).not_to include(project) + end + end + + context 'when the item has no consumers' do + let(:project_consumer) { nil } + let(:another_project_consumer) { nil } + + it 'deletes the item along with the project' do + expect { project_destroy_service.execute } + .to change { Ai::Catalog::Item.where(id: ai_catalog_item.id).count }.by(-1) + + expect(Project.unscoped.all).not_to include(project) + end + end + end end -- GitLab From f92afc527a658b126e87ab599eef0f93bd3144e7 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Tue, 16 Dec 2025 11:27:44 +0100 Subject: [PATCH 2/2] Apply N+1 optimization for queries --- ee/app/models/ai/catalog/item.rb | 2 ++ .../services/ee/projects/destroy_service.rb | 15 +++++---- ee/spec/models/ai/catalog/item_spec.rb | 31 +++++++++++++++++++ .../services/projects/destroy_service_spec.rb | 26 +++++++++++++++- 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/ee/app/models/ai/catalog/item.rb b/ee/app/models/ai/catalog/item.rb index a380da17b0590f..415352fcec6310 100644 --- a/ee/app/models/ai/catalog/item.rb +++ b/ee/app/models/ai/catalog/item.rb @@ -49,6 +49,8 @@ class Item < ApplicationRecord scope :with_ids, ->(ids) { where(id: ids) } scope :with_item_type, ->(item_type) { where(item_type: item_type) } + scope :without_consumers, -> { left_joins(:consumers).where(ai_catalog_item_consumers: { id: nil }) } + scope :order_by_id_desc, -> do return reorder(id: :desc) unless ::Gitlab::Saas.feature_available?(:gitlab_duo_saas_only) diff --git a/ee/app/services/ee/projects/destroy_service.rb b/ee/app/services/ee/projects/destroy_service.rb index 0da79413db0042..4de24d11319428 100644 --- a/ee/app/services/ee/projects/destroy_service.rb +++ b/ee/app/services/ee/projects/destroy_service.rb @@ -132,15 +132,14 @@ def destroy_ai_catalog_items! project_owned_items = ::Ai::Catalog::Item.for_project(project) - project_owned_items.find_each do |item| - external_consumers = item.consumers.not_for_projects(project.id) + # Hard delete items that have no remaining consumers + project_owned_items.without_consumers.delete_all - if external_consumers.any? - item.update(project_id: nil, deleted_at: Time.zone.now) - else - item.delete - end - end + # Soft delete all items owned by the project + project_owned_items.update_all( + project_id: nil, + deleted_at: Time.zone.now + ) end end end diff --git a/ee/spec/models/ai/catalog/item_spec.rb b/ee/spec/models/ai/catalog/item_spec.rb index 14b90300ccc1e8..7b85ee0aff335b 100644 --- a/ee/spec/models/ai/catalog/item_spec.rb +++ b/ee/spec/models/ai/catalog/item_spec.rb @@ -383,6 +383,37 @@ expect(result).not_to include(item_without_reference) end end + + describe '.without_consumers' do + let_it_be(:project) { create(:project) } + let_it_be(:item_with_consumers) { create(:ai_catalog_item, :public, project: project) } + let_it_be(:item_without_consumers) { create(:ai_catalog_item, :public, project: project) } + let_it_be(:consumer) { create(:ai_catalog_item_consumer, project: project, item: item_with_consumers) } + + it 'returns only items without any consumers' do + expect(described_class.without_consumers).to contain_exactly(item_without_consumers) + end + + context 'when an item has multiple consumers' do + let_it_be(:another_consumer) do + create(:ai_catalog_item_consumer, project: create(:project), item: item_with_consumers) + end + + it 'still excludes the item' do + expect(described_class.without_consumers).to contain_exactly(item_without_consumers) + end + end + + context 'when all items have consumers' do + before do + create(:ai_catalog_item_consumer, project: project, item: item_without_consumers) + end + + it 'returns an empty collection' do + expect(described_class.without_consumers).to be_empty + end + end + end end describe 'callbacks' do diff --git a/ee/spec/services/projects/destroy_service_spec.rb b/ee/spec/services/projects/destroy_service_spec.rb index d51256a0ca6c65..b2dd2c9584d7c1 100644 --- a/ee/spec/services/projects/destroy_service_spec.rb +++ b/ee/spec/services/projects/destroy_service_spec.rb @@ -542,7 +542,7 @@ end context 'when project has AI catalog items with consumers' do - let(:project2) { create(:project, namespace: user.namespace) } + let_it_be(:project2) { create(:project) } let!(:ai_catalog_item) { create(:ai_catalog_item, :public, project: project) } let!(:project_consumer) { create(:ai_catalog_item_consumer, item: ai_catalog_item, project: project) } @@ -584,5 +584,29 @@ expect(Project.unscoped.all).not_to include(project) end end + + it 'does not cause N+1 queries when checking for external consumers' do + # Baseline: project with 1 catalog item + baseline_project = create(:project, namespace: user.namespace) + baseline_item = create(:ai_catalog_item, :public, project: baseline_project) + create(:ai_catalog_item_consumer, item: baseline_item, project: baseline_project) + create(:ai_catalog_item_consumer, item: baseline_item, project: project2) + + baseline = ActiveRecord::QueryRecorder.new do + described_class.new(baseline_project, user, {}).execute + end + + # Test: project with 3 catalog items (should not cause N+1) + test_project = create(:project, namespace: user.namespace) + 3.times do + item = create(:ai_catalog_item, :public, project: test_project) + create(:ai_catalog_item_consumer, item: item, project: test_project) + create(:ai_catalog_item_consumer, item: item, project: project2) + end + + expect do + described_class.new(test_project, user, {}).execute + end.not_to exceed_query_limit(baseline) + end end end -- GitLab