Replacing validate uniqueness with db constraint on RevisionProcessCommand model

app/models/runtime/revision_process_command_model.rb

@@ -6,9 +6,13 @@ class RevisionProcessCommandModel < Sequel::Model(:revision_process_commands)
                 key: :revision_guid,
                 without_guid_generation: true
 
-    def validate
-      super
-      validates_unique(%i[revision_guid process_type])
+    def around_save
+      yield
+    rescue Sequel::UniqueConstraintViolation => e
+      raise e unless e.message.include?('revision_process_commands_revision_guid_process_type_index')
+
+      errors.add(%i[revision_guid process_type], Sequel.lit("Process type '#{process_type}' already exists for given revision"))
+      raise validation_failed_error
     end
   end
 end

app/models/services/service_key.rb

@@ -49,6 +49,7 @@ def around_save
     def validate
       validates_presence :name
       validates_presence :service_instance
+      # Keep validates_unique as a pre-guard to prevent broker HTTP call when key already exists
       validates_unique %i[name service_instance_id]
 
       return unless service_instance

db/migrations/20260323144429_add_unique_constraint_to_revision_process_commands.rb

@@ -0,0 +1,61 @@
+Sequel.migration do # rubocop:disable Metrics/BlockLength
+  no_transaction # required for concurrently option on postgres
+
+  up do
+    transaction do
+      duplicates = self[:revision_process_commands].
+                   select(:revision_guid, :process_type).
+                   group(:revision_guid, :process_type).
+                   having { count(1) > 1 }
+
+      duplicates.each do |dup|
+        ids_to_remove = self[:revision_process_commands].
+                        where(revision_guid: dup[:revision_guid], process_type: dup[:process_type]).
+                        select(:id).
+                        order(:id).
+                        offset(1).
+                        map(:id)
+        self[:revision_process_commands].where(id: ids_to_remove).delete
+      end
+    end
+
+    if database_type == :postgres
+      VCAP::Migration.with_concurrent_timeout(self) do
+        add_index :revision_process_commands, %i[revision_guid process_type],
+                  name: :revision_process_commands_revision_guid_process_type_index,
+                  unique: true,
+                  concurrently: true,
+                  if_not_exists: true
+      end
+    else
+      alter_table(:revision_process_commands) do
+        # rubocop:disable Sequel/ConcurrentIndex -- MySQL does not support concurrent index operations
+        unless @db.indexes(:revision_process_commands).key?(:revision_process_commands_revision_guid_process_type_index)
+          add_index %i[revision_guid process_type], unique: true,
+                                                    name: :revision_process_commands_revision_guid_process_type_index
+        end
+        # rubocop:enable Sequel/ConcurrentIndex
+      end
+    end
+  end
+
+  down do
+    if database_type == :postgres
+      VCAP::Migration.with_concurrent_timeout(self) do
+        drop_index :revision_process_commands, nil,
+                   name: :revision_process_commands_revision_guid_process_type_index,
+                   concurrently: true,
+                   if_exists: true
+      end
+    else
+      alter_table(:revision_process_commands) do
+        # rubocop:disable Sequel/ConcurrentIndex -- MySQL does not support concurrent index operations
+        if @db.indexes(:revision_process_commands).key?(:revision_process_commands_revision_guid_process_type_index)
+          drop_index %i[revision_guid process_type],
+                     name: :revision_process_commands_revision_guid_process_type_index
+        end
+        # rubocop:enable Sequel/ConcurrentIndex
+      end
+    end
+  end
+end

spec/migrations/20260323144429_add_unique_constraint_to_revision_process_commands_spec.rb

@@ -0,0 +1,51 @@
+require 'spec_helper'
+require 'migrations/helpers/migration_shared_context'
+
+RSpec.describe 'add unique constraint to revision_process_commands', isolation: :truncation, type: :migration do
+  include_context 'migration' do
+    let(:migration_filename) { '20260323144429_add_unique_constraint_to_revision_process_commands.rb' }
+  end
+
+  let!(:revision) { VCAP::CloudController::RevisionModel.make }
+
+  it 'removes duplicates, adds constraint and reverts migration' do
+    # =========================================================================================
+    # SETUP: Create duplicate entries to test the de-duplication logic.
+    # =========================================================================================
+    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+    expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(2)
+
+    # =========================================================================================
+    # UP MIGRATION: Run the migration to apply the unique constraints.
+    # =========================================================================================
+    Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
+
+    # =========================================================================================
+    # ASSERT UP MIGRATION: Verify that duplicates are removed and constraints are enforced.
+    # =========================================================================================
+    expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(1)
+    expect(db.indexes(:revision_process_commands)).to include(:revision_process_commands_revision_guid_process_type_index)
+    expect do
+      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+    end.to raise_error(Sequel::UniqueConstraintViolation)
+
+    # =========================================================================================
+    # TEST IDEMPOTENCY: Running the migration again should not cause any errors.
+    # =========================================================================================
+    expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
+
+    # =========================================================================================
+    # DOWN MIGRATION: Roll back the migration to remove the constraints.
+    # =========================================================================================
+    Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true)
+
+    # =========================================================================================
+    # ASSERT DOWN MIGRATION: Verify that constraints are removed and duplicates can be re-inserted.
+    # =========================================================================================
+    expect(db.indexes(:revision_process_commands)).not_to include(:revision_process_commands_revision_guid_process_type_index)
+    expect do
+      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+    end.not_to raise_error
+  end
+end

spec/unit/models/runtime/revision_process_command_model_spec.rb

@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+module VCAP::CloudController
+  RSpec.describe RevisionProcessCommandModel do
+    describe 'revision_process_command_model #around_save' do
+      let(:revision) { RevisionModel.make }
+      let!(:revision_process_command) { RevisionProcessCommandModel.make(revision_guid: revision.guid, process_type: 'worker') }
+
+      it 'raises validation error on unique constraint violation' do
+        expect do
+          RevisionProcessCommandModel.make(
+            revision_guid: revision_process_command.revision_guid,
+            process_type: revision_process_command.process_type
+          )
+        end.to raise_error(Sequel::ValidationFailed) { |error|
+          expect(error.message).to include('already exists for given revision')
+        }
+      end
+
+      it 'raises the original error on other unique constraint violations' do
+        expect do
+          RevisionProcessCommandModel.make(guid: revision_process_command.guid, revision_guid: revision_process_command.revision_guid, process_type: 'worker')
+        end.to raise_error(Sequel::UniqueConstraintViolation)
+      end
+    end
+  end
+end