diff --git a/ee/app/models/ai/catalog/item.rb b/ee/app/models/ai/catalog/item.rb index a380da17b0590fd106032d88089ddca7d0a99b1a..415352fcec6310c105e8dd60bc5d2c2e35a8c777 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 5978de80d45cc18146af800b3cdf2854cebfeb6c..4de24d113194281ea7f40e17e034618ce5b89955 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,21 @@ 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) + + # Hard delete items that have no remaining consumers + project_owned_items.without_consumers.delete_all + + # Soft delete all items owned by the project + project_owned_items.update_all( + project_id: nil, + deleted_at: Time.zone.now + ) + end end end end diff --git a/ee/spec/models/ai/catalog/item_spec.rb b/ee/spec/models/ai/catalog/item_spec.rb index 14b90300ccc1e82834a38e6294c07bf85ae0f298..7b85ee0aff335b34cd9880bae42600e015d86429 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 787025c1fce8c17dafd8f0cc9481f99878362694..b2dd2c9584d7c1b5196feaccc448178f524426b1 100644 --- a/ee/spec/services/projects/destroy_service_spec.rb +++ b/ee/spec/services/projects/destroy_service_spec.rb @@ -540,4 +540,73 @@ project_destroy_service.execute end end + + context 'when project has AI catalog items with consumers' do + 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) } + 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 + + 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