Skip to content

Commit

Permalink
feat: use a sequence for the verification number on postgres
Browse files Browse the repository at this point in the history
  • Loading branch information
bethesque committed Oct 23, 2020
1 parent 09e0eba commit b8f029e
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 38 deletions.
20 changes: 20 additions & 0 deletions db/migrations/20201023_create_verification_number_sequence.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
require_relative 'migration_helper'

Sequel.migration do
up do
if PactBroker::MigrationHelper.postgres?
row = from(:verification_sequence_number).select(:value).limit(1).first
start = row ? row[:value] + 1 : 1
run("CREATE SEQUENCE verification_number_sequence START WITH #{start}")
end
end

down do
if PactBroker::MigrationHelper.postgres?
nextval = execute("SELECT nextval('verification_number_sequence') as val") { |v| v.first["val"].to_i }
# Add a safety margin!
from(:verification_sequence_number).update(value: nextval + 100)
run("DROP SEQUENCE verification_number_sequence")
end
end
end
4 changes: 4 additions & 0 deletions lib/db.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ def self.mysql?
!!(PACT_BROKER_DB.adapter_scheme.to_s =~ /mysql/)
end

def self.postgres?
!!(PACT_BROKER_DB.adapter_scheme.to_s == "postgres")
end

PACT_BROKER_DB ||= connection_for_env ENV.fetch('RACK_ENV')

def self.health_check
Expand Down
38 changes: 22 additions & 16 deletions lib/pact_broker/verifications/sequence.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
require 'sequel'
require 'pact_broker/repositories/helpers'


module PactBroker
module Verifications
Expand All @@ -7,22 +9,26 @@ class Sequence < Sequel::Model(:verification_sequence_number)
# The easiest way to implement a cross database compatible sequence.
# Sad, I know.
def next_val
db.transaction do
for_update.first
select_all.update(value: Sequel[:value]+1)
row = first
if row
row.value
else
# The first row should have been created in the migration, so this code
# should only ever be executed in a test context.
# There would be a risk of a race condition creating two rows if this
# code executed in prod, as I don't think you can lock an empty table
# to prevent another record being inserted.
max_verification_number = PactBroker::Domain::Verification.max(:number)
value = max_verification_number ? max_verification_number + 100 : 1
insert(value: value)
value
if PactBroker::Repositories::Helpers.postgres?
db.execute("SELECT nextval('verification_number_sequence') as val") { |v| v.first["val"].to_i }
else
db.transaction do
for_update.first
select_all.update(value: Sequel[:value]+1)
row = first
if row
row.value
else
# The first row should have been created in the migration, so this code
# should only ever be executed in a test context.
# There would be a risk of a race condition creating two rows if this
# code executed in prod, as I don't think you can lock an empty table
# to prevent another record being inserted.
max_verification_number = PactBroker::Domain::Verification.max(:number)
value = max_verification_number ? max_verification_number + 100 : 1
insert(value: value)
value
end
end
end
end
Expand Down
70 changes: 48 additions & 22 deletions spec/lib/pact_broker/verifications/sequence_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,67 @@ module PactBroker
module Verifications
describe Sequence do
describe "#next_val", migration: true do
context "for proper databases with proper sequences", skip: !::DB.postgres? do
it "increments the value each time" do
PactBroker::Database.migrate
expect(Sequence.next_val).to eq 101
expect(Sequence.next_val).to eq 102
end

before do
PactBroker::Database.migrate
end

context "when there is a row in the verification_sequence_number table" do
before do
Sequence.select_all.delete
Sequence.insert(value: 1)
it "can rollback without duplicating a sequence number" do
PactBroker::Database.migrate
row = database.from(:verification_sequence_number).select(:value).limit(1).first
expect(row[:value]).to eq 100
Sequence.next_val
PactBroker::Database.migrate(20201006)
row = database.from(:verification_sequence_number).select(:value).limit(1).first
expect(row[:value]).to eq 202
end

it "increments the value and returns it" do
expect(Sequence.next_val).to eq 2
it "can deal with there not being an existing value in the verification_sequence_number table" do
PactBroker::Database.migrate(20201006)
database.from(:verification_sequence_number).delete
PactBroker::Database.migrate
expect(Sequence.next_val).to eq 1
end
end

context "when there is no row in the verification_sequence_number table and no existing verifications" do
context "for databases without sequences", skip: ::DB.postgres? do
before do
Sequence.select_all.delete
PactBroker::Database.migrate
end

it "inserts and returns the value 1" do
expect(Sequence.next_val).to eq 1
context "when there is a row in the verification_sequence_number table" do
before do
Sequence.select_all.delete
Sequence.insert(value: 1)
end

it "increments the value and returns it" do
expect(Sequence.next_val).to eq 2
end
end
end

context "when there is no row in the verification_sequence_number table and there are existing verifications" do
before do
Sequence.select_all.delete
TestDataBuilder.new.create_pact_with_hierarchy("A", "1", "B")
.create_verification(provider_version: "2")
context "when there is no row in the verification_sequence_number table and no existing verifications" do
before do
Sequence.select_all.delete
end

it "inserts and returns the value 1" do
expect(Sequence.next_val).to eq 1
end
end

it "inserts a number that is guaranteed to be higher than any of the existing verification numbers from when we tried to do this without a sequence" do
expect(Sequence.next_val).to eq 101
context "when there is no row in the verification_sequence_number table and there are existing verifications" do
before do
Sequence.select_all.delete
TestDataBuilder.new.create_pact_with_hierarchy("A", "1", "B")
.create_verification(provider_version: "2")
end

it "inserts a number that is guaranteed to be higher than any of the existing verification numbers from when we tried to do this without a sequence" do
expect(Sequence.next_val).to eq 101
end
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions tasks/database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def ensure_database_dir_exists
def drop_objects
drop_views
drop_tables
drop_sequences
end

def drop_tables
Expand All @@ -56,6 +57,12 @@ def drop_views
end
end

def drop_sequences
if psql?
database.run('DROP SEQUENCE verification_number_sequence')
end
end

def create
if psql?
system('psql postgres -c "create database pact_broker"')
Expand Down

0 comments on commit b8f029e

Please sign in to comment.