BBM for removing orphaned notes

Cleaning up notes for which the
associated vulnerability has been deleted.

Changelog: removed

db/docs/batched_background_migrations/remove_orphaned_vulnerability_notes_batched_migration.yml

+---
+migration_job_name: RemoveOrphanedVulnerabilityNotesBatchedMigration
+description: Removes orphaned Notes for which the Vulnerability no longer exists.
+feature_category: vulnerability_management
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180624
+milestone: '17.10'
+queued_migration_version: 20250304184254
+finalized_by: # version of the migration that finalized this BBM

db/post_migrate/20250304184254_queue_remove_orphaned_vulnerability_notes_batched_migration.rb

+# frozen_string_literal: true
+
+class QueueRemoveOrphanedVulnerabilityNotesBatchedMigration < Gitlab::Database::Migration[2.2]
+  milestone '17.10'
+
+  restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+  MIGRATION = "RemoveOrphanedVulnerabilityNotesBatchedMigration"
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 1000
+  SUB_BATCH_SIZE = 100
+
+  def up
+    queue_batched_background_migration(
+      MIGRATION,
+      :notes,
+      :id,
+      job_interval: DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      sub_batch_size: SUB_BATCH_SIZE
+    )
+  end
+
+  def down
+    delete_batched_background_migration(MIGRATION, :notes, :id, [])
+  end
+end

db/schema_migrations/20250304184254

+0c6eba6946c0434d649293804a2ab50c0f45590ca9d583435aa0aea05a668b9d
\ No newline at end of file

lib/gitlab/background_migration/remove_orphaned_vulnerability_notes_batched_migration.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class RemoveOrphanedVulnerabilityNotesBatchedMigration < BatchedMigrationJob
+      operation_name :remove_orphaned_vulnerability_notes
+      feature_category :vulnerability_management
+
+      def perform
+        each_sub_batch do |sub_batch|
+          orphaned_notes = sub_batch
+            .where(noteable_type: 'Vulnerability')
+            .where.not(noteable_id: Vulnerability.where(id: sub_batch.pluck(:noteable_id)).pluck(:id)) # rubocop:disable Rails/PluckInWhere -- we want to use pluck here
+          orphaned_notes.delete_all
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/remove_orphaned_vulnerability_notes_batched_migration_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::RemoveOrphanedVulnerabilityNotesBatchedMigration, feature_category: :vulnerability_management do
+  let(:migration) do
+    described_class.new(
+      batch_table: 'notes',
+      batch_column: 'id',
+      sub_batch_size: 100,
+      pause_ms: 100,
+      connection: ActiveRecord::Base.connection
+    )
+  end
+
+  let(:user_id) do
+    table(:users).create!(email: 'author@example.com', notification_email: 'author@example.com', name: 'Author',
+      username: 'author', projects_limit: 1000).id
+  end
+
+  let(:organization_id) do
+    table(:organizations).create!(
+      name: 'Organization',
+      path: 'organization').id
+  end
+
+  let(:namespace_id) do
+    table(:namespaces).create!(
+      name: 'Namespace',
+      path: 'namespace',
+      organization_id: organization_id).id
+  end
+
+  let(:project_namespace_id) do
+    table(:namespaces).create!(
+      name: 'Project Namespace',
+      path: 'project-namespace',
+      organization_id: organization_id).id
+  end
+
+  let(:project_id) do
+    table(:projects).create!(
+      name: 'Project',
+      path: 'project',
+      namespace_id: namespace_id,
+      organization_id: organization_id,
+      project_namespace_id: project_namespace_id
+    ).id
+  end
+
+  let(:note_attributes) do
+    {
+      author_id: user_id,
+      project_id: project_id,
+      namespace_id: namespace_id,
+      created_at: Time.current,
+      updated_at: Time.current
+    }
+  end
+
+  before do
+    allow(Vulnerability).to receive_message_chain(:where, :pluck).and_return([1, 2, 3])
+  end
+
+  def create_note(noteable_id:, noteable_type:)
+    table(:notes).create!(
+      note: 'Note Content',
+      noteable_type: noteable_type,
+      noteable_id: noteable_id,
+      **note_attributes
+    )
+  end
+
+  describe 'orphaned notes removal' do
+    # These notes are created before the test runs
+    let!(:invalid_note_1) { create_note(noteable_id: 4, noteable_type: 'Vulnerability') }
+
+    let!(:invalid_note_2) { create_note(noteable_id: 28, noteable_type: 'Vulnerability') }
+
+    it 'removes orphaned Vulnerability notes' do
+      expect { migration.perform }.to change { table(:notes).count }.by(-2)
+      expect(table(:notes).find_by(id: invalid_note_1.id)).to be_blank
+      expect(table(:notes).find_by(id: invalid_note_2.id)).to be_blank
+    end
+  end
+
+  describe 'valid notes retention' do
+    let!(:valid_note) { create_note(noteable_id: 1, noteable_type: 'Vulnerability') }
+
+    it 'retains valid Vulnerability notes' do
+      expect { migration.perform }.not_to change { table(:notes).count }
+      expect(table(:notes).find_by(id: valid_note.id)).to be_present
+    end
+  end
+
+  describe 'non-vulnerability notes' do
+    let!(:issue_note_nil_id) { create_note(noteable_id: nil, noteable_type: 'Issue') }
+
+    let!(:issue_note_with_id) { create_note(noteable_id: 2, noteable_type: 'Issue') }
+
+    it 'does not remove non-Vulnerability notes' do
+      expect { migration.perform }.not_to change { table(:notes).count }
+      expect(table(:notes).find_by(id: issue_note_nil_id.id)).to be_present
+      expect(table(:notes).find_by(id: issue_note_with_id.id)).to be_present
+    end
+  end
+end

spec/migrations/20250304184254_queue_remove_orphaned_vulnerability_notes_batched_migration_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe QueueRemoveOrphanedVulnerabilityNotesBatchedMigration, migration: :gitlab_main, feature_category: :vulnerability_management do
+  let(:migration) { described_class.new }
+  let(:batched_migration) { described_class::MIGRATION }
+
+  it 'schedules a new batched migration' do
+    reversible_migration do |migration|
+      migration.before -> {
+        expect(batched_migration).not_to have_scheduled_batched_migration
+      }
+
+      migration.after -> {
+        expect(batched_migration).to have_scheduled_batched_migration(
+          gitlab_schema: :gitlab_main,
+          table_name: :notes,
+          column_name: :id,
+          interval: described_class::DELAY_INTERVAL,
+          batch_size: described_class::BATCH_SIZE,
+          sub_batch_size: described_class::SUB_BATCH_SIZE
+        )
+      }
+    end
+  end
+
+  it 'uses the correct migration class' do
+    expect(described_class::MIGRATION).to eq('RemoveOrphanedVulnerabilityNotesBatchedMigration')
+  end
+
+  it 'sets the correct delay interval' do
+    expect(described_class::DELAY_INTERVAL).to eq(2.minutes)
+  end
+
+  it 'sets the correct batch size' do
+    expect(described_class::BATCH_SIZE).to eq(1000)
+  end
+
+  it 'sets the correct sub-batch size' do
+    expect(described_class::SUB_BATCH_SIZE).to eq(100)
+  end
+end