Merge branch 'split_ci_upstream_project_subscriptions_out_of_project_transfer' into 'master'

Delete upstream_project_subscriptions outside of project transaction

See merge request gitlab-org/gitlab!75065

config/sidekiq_queues.yml

@@ -73,6 +73,8 @@
   - 1
 - - ci_delete_objects
   - 1
+- - ci_upstream_projects_subscriptions_cleanup
+  - 1
 - - container_repository
   - 1
 - - create_commit_signature

ee/app/services/ee/projects/transfer_service.rb

@@ -61,10 +61,14 @@ module EE
           .update_all(revoked: true)
       end
 
+      # This method is within a transaction
       def delete_pipeline_subscriptions
         return if new_namespace.licensed_feature_available?(:ci_project_subscriptions)
 
-        project.upstream_project_subscriptions.destroy_all # rubocop: disable Cop/DestroyAll
+        project_id = project.id
+        project.run_after_commit do
+          ::Ci::UpstreamProjectsSubscriptionsCleanupWorker.perform_async(project_id)
+        end
       end
 
       def delete_test_cases

ee/app/workers/all_queues.yml

@@ -912,6 +912,15 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: ci_upstream_projects_subscriptions_cleanup
+  :worker_name: Ci::UpstreamProjectsSubscriptionsCleanupWorker
+  :feature_category: :continuous_integration
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: create_github_webhook
   :worker_name: CreateGithubWebhookWorker
   :feature_category: :integrations

ee/app/workers/ci/upstream_projects_subscriptions_cleanup_worker.rb

+# frozen_string_literal: true
+
+module Ci
+  class UpstreamProjectsSubscriptionsCleanupWorker
+    include ApplicationWorker
+
+    data_consistency :always
+
+    feature_category :continuous_integration
+    idempotent!
+
+    def perform(project_id)
+      project = Project.find_by_id(project_id)
+
+      return unless project
+      return if project.licensed_feature_available?(:ci_project_subscriptions)
+
+      project.upstream_project_subscriptions.destroy_all # rubocop: disable Cop/DestroyAll
+    end
+  end
+end

ee/spec/services/projects/transfer_service_spec.rb

@@ -169,26 +169,25 @@ RSpec.describe Projects::TransferService do
       let_it_be(:project_2) { create(:project, :public) }
 
       before do
-        project.upstream_project_subscriptions.create!(upstream_project: project_2)
+        create(:license, plan: License::PREMIUM_PLAN)
         stub_ee_application_setting(should_check_namespace_plan: true)
+
         premium_group.add_owner(user)
       end
 
       context 'when target namespace has a premium plan' do
-        it 'does not delete the pipeline subscriptions' do
-          subject.execute(premium_group)
+        it 'does not schedule cleanup for upstream project subscription' do
+          expect(::Ci::UpstreamProjectsSubscriptionsCleanupWorker).not_to receive(:perform_async)
 
-          expect { subject.execute(premium_group) }.not_to change { project.upstream_project_subscriptions.count }
+          subject.execute(premium_group)
         end
       end
 
       context 'when target namespace has a free plan' do
-        it 'deletes the upstream subscriptions' do
-          expect { subject.execute(free_group) }.to change { project.upstream_project_subscriptions.count }.from(1).to(0)
-        end
+        it 'schedules cleanup for upstream project subscription' do
+          expect(::Ci::UpstreamProjectsSubscriptionsCleanupWorker).to receive(:perform_async).with(project.id).and_call_original
 
-        it 'deletes the downstream subscriptions' do
-          expect { subject.execute(free_group) }.to change { project_2.downstream_project_subscriptions.count }.from(1).to(0)
+          subject.execute(free_group)
         end
       end
     end

ee/spec/workers/ci/upstream_projects_subscriptions_cleanup_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::UpstreamProjectsSubscriptionsCleanupWorker do
+  describe '#perform' do
+    let(:group) { create(:group, :public) }
+    let(:project) { create(:project, :repository, :public, group: group) }
+
+    let_it_be(:project_2) { create(:project, :public) }
+
+    before do
+      project.upstream_project_subscriptions.create!(upstream_project: project_2)
+    end
+
+    it_behaves_like 'an idempotent worker' do
+      let(:job_args) { [project.id] }
+    end
+
+    context 'project does not exist' do
+      it 'does nothing' do
+        expect { described_class.new.perform(non_existing_record_id) }.not_to change { Ci::Subscriptions::Project.count }
+      end
+    end
+
+    context 'ci_project_subscriptions licensed feature available' do
+      before do
+        stub_licensed_features(ci_project_subscriptions: true)
+      end
+
+      it 'does not delete the pipeline subscriptions' do
+        expect { described_class.new.perform(project.id) }.not_to change { project.upstream_project_subscriptions.count }
+      end
+    end
+
+    context 'ci_project_subscriptions licensed feature not available' do
+      before do
+        stub_licensed_features(ci_project_subscriptions: false)
+      end
+
+      it 'deletes the upstream subscriptions' do
+        expect { described_class.new.perform(project.id) }.to change { project.upstream_project_subscriptions.count }.from(1).to(0)
+          .and change { project_2.downstream_project_subscriptions.count }.from(1).to(0)
+      end
+    end
+  end
+end

spec/support/database/cross-database-modification-allowlist.yml

@@ -14,7 +14,6 @@
 - "./ee/spec/services/deployments/auto_rollback_service_spec.rb"
 - "./ee/spec/services/ee/ci/job_artifacts/destroy_all_expired_service_spec.rb"
 - "./ee/spec/services/ee/users/destroy_service_spec.rb"
-- "./ee/spec/services/projects/transfer_service_spec.rb"
 - "./ee/spec/services/security/security_orchestration_policies/rule_schedule_service_spec.rb"
 - "./spec/controllers/abuse_reports_controller_spec.rb"
 - "./spec/controllers/admin/spam_logs_controller_spec.rb"