Skip to content

Commit 3f22556

Browse files
committed
feat: unique constraint added on revicion_process_commands table
- unique constraint on revision_guid and process type columns are added on the table. - validate_uniqueness check in model side is removed. - respective test cases are added
1 parent d16757b commit 3f22556

4 files changed

Lines changed: 120 additions & 3 deletions

File tree

app/models/runtime/revision_process_command_model.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@ class RevisionProcessCommandModel < Sequel::Model(:revision_process_commands)
66
key: :revision_guid,
77
without_guid_generation: true
88

9-
def validate
10-
super
11-
validates_unique(%i[revision_guid process_type])
9+
def around_save
10+
yield
11+
rescue Sequel::UniqueConstraintViolation => e
12+
raise e unless e.message.include?('revision_process_commands_revision_guid_process_type_index')
13+
14+
errors.add(%i[revision_guid process_type], Sequel.lit("Process type '#{process_type}' already exists for given revision"))
15+
raise validation_failed_error
1216
end
1317
end
1418
end
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
Sequel.migration do
2+
up do
3+
transaction do
4+
duplicates = self[:revision_process_commands].
5+
select(:revision_guid, :process_type).
6+
group(:revision_guid, :process_type).
7+
having { count(1) > 1 }
8+
9+
duplicates.each do |dup|
10+
ids_to_remove = self[:revision_process_commands].
11+
where(revision_guid: dup[:revision_guid], process_type: dup[:process_type]).
12+
select(:id).
13+
order(:id).
14+
offset(1).
15+
map(:id)
16+
self[:revision_process_commands].where(id: ids_to_remove).delete
17+
end
18+
19+
alter_table(:revision_process_commands) do
20+
unless @db.indexes(:revision_process_commands).key?(:revision_process_commands_revision_guid_process_type_index)
21+
add_unique_constraint %i[revision_guid process_type],
22+
name: :revision_process_commands_revision_guid_process_type_index
23+
end
24+
end
25+
end
26+
end
27+
28+
down do
29+
alter_table(:revision_process_commands) do
30+
if @db.indexes(:revision_process_commands).key?(:revision_process_commands_revision_guid_process_type_index)
31+
drop_constraint(:revision_process_commands_revision_guid_process_type_index, type: :unique)
32+
end
33+
end
34+
end
35+
end
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
require 'spec_helper'
2+
require 'migrations/helpers/migration_shared_context'
3+
4+
RSpec.describe 'add unique constraint to revision_process_commands', isolation: :truncation, type: :migration do
5+
include_context 'migration' do
6+
let(:migration_filename) { '20260323144429_add_unique_constraint_to_revision_process_commands.rb' }
7+
end
8+
9+
let!(:revision) { VCAP::CloudController::RevisionModel.make }
10+
11+
it 'removes duplicates, adds constraint and reverts migration' do
12+
# =========================================================================================
13+
# SETUP: Create duplicate entries to test the de-duplication logic.
14+
# =========================================================================================
15+
db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
16+
db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
17+
expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(2)
18+
19+
# =========================================================================================
20+
# UP MIGRATION: Run the migration to apply the unique constraints.
21+
# =========================================================================================
22+
Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
23+
24+
# =========================================================================================
25+
# ASSERT UP MIGRATION: Verify that duplicates are removed and constraints are enforced.
26+
# =========================================================================================
27+
expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(1)
28+
expect(db.indexes(:revision_process_commands)).to include(:revision_process_commands_revision_guid_process_type_index)
29+
expect do
30+
db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
31+
end.to raise_error(Sequel::UniqueConstraintViolation)
32+
33+
# =========================================================================================
34+
# TEST IDEMPOTENCY: Running the migration again should not cause any errors.
35+
# =========================================================================================
36+
expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
37+
38+
# =========================================================================================
39+
# DOWN MIGRATION: Roll back the migration to remove the constraints.
40+
# =========================================================================================
41+
Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true)
42+
43+
# =========================================================================================
44+
# ASSERT DOWN MIGRATION: Verify that constraints are removed and duplicates can be re-inserted.
45+
# =========================================================================================
46+
expect(db.indexes(:revision_process_commands)).not_to include(:revision_process_commands_revision_guid_process_type_index)
47+
expect do
48+
db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
49+
end.not_to raise_error
50+
end
51+
end
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
require 'spec_helper'
2+
3+
module VCAP::CloudController
4+
RSpec.describe RevisionProcessCommandModel do
5+
describe 'revision_process_command_model #around_save' do
6+
let(:revision) { RevisionModel.make }
7+
let!(:revision_process_command) { RevisionProcessCommandModel.make(revision_guid: revision.guid, process_type: 'worker') }
8+
9+
it 'raises validation error on unique constraint violation' do
10+
expect do
11+
RevisionProcessCommandModel.make(
12+
revision_guid: revision_process_command.revision_guid,
13+
process_type: revision_process_command.process_type
14+
)
15+
end.to raise_error(Sequel::ValidationFailed) { |error|
16+
expect(error.message).to include('already exists for given revision')
17+
}
18+
end
19+
20+
it 'raises the original error on other unique constraint violations' do
21+
expect do
22+
RevisionProcessCommandModel.make(guid: revision_process_command.guid, revision_guid: revision_process_command.revision_guid, process_type: 'worker')
23+
end.to raise_error(Sequel::UniqueConstraintViolation)
24+
end
25+
end
26+
end
27+
end

0 commit comments

Comments
 (0)