fix: duplicate key value violates unique constraint uq_ver_ppt_ord

Author: bethesque

db/migrations/20190509_create_version_sequence.rb

@@ -0,0 +1,8 @@
+require 'pact_broker/db/data_migrations/set_latest_version_sequence_value'
+Sequel.migration do
+  change do
+    create_table(:version_sequence_number) do
+      Integer :value, null: false
+    end
+  end
+end

db/migrations/20190510_set_version_sequence.rb

@@ -0,0 +1,9 @@
+require 'pact_broker/db/data_migrations/set_latest_version_sequence_value'
+Sequel.migration do
+  up do
+    PactBroker::DB::DataMigrations::SetLatestVersionSequenceValue.call(self)
+  end
+
+  down do
+  end
+end

lib/pact_broker/db/data_migrations/set_latest_version_sequence_value.rb

@@ -0,0 +1,29 @@
+module PactBroker
+  module DB
+    module DataMigrations
+      class SetLatestVersionSequenceValue
+        def self.call connection
+          if columns_exist?(connection)
+            max_order = connection[:versions].max(:order) || 0
+            sequence_row = connection[:version_sequence_number].first
+            if sequence_row.nil? || sequence_row[:value] <= max_order
+              new_value = max_order + 100
+              connection[:version_sequence_number].insert(value: new_value)
+              # Make sure there is only ever one row in case there is a race condition
+              connection[:version_sequence_number].exclude(value: new_value).delete
+            end
+          end
+        end
+
+        def self.columns_exist?(connection)
+          column_exists?(connection, :versions, :order) &&
+            column_exists?(connection, :version_sequence_number, :value)
+        end
+
+        def self.column_exists?(connection, table, column)
+          connection.table_exists?(table) && connection.schema(table).find{|col| col.first == column }
+        end
+      end
+    end
+  end
+end

lib/pact_broker/db/migrate_data.rb

@@ -16,6 +16,7 @@ class MigrateData
       def self.call database_connection, options = {}
         DataMigrations::SetPacticipantIdsForVerifications.call(database_connection)
         DataMigrations::SetConsumerIdsForPactPublications.call(database_connection)
+        DataMigrations::SetLatestVersionSequenceValue.call(database_connection)
       end
     end
   end

lib/pact_broker/domain/order_versions.rb

@@ -1,13 +1,26 @@
 require 'pact_broker/configuration'
+require 'pact_broker/versions/sequence'
 
 module PactBroker
   module Domain
     class OrderVersions
-
       include PactBroker::Logging
 
       def self.call new_version
         new_version.lock!
+
+        if PactBroker.configuration.order_versions_by_date
+          set_sequential_order(new_version)
+        else
+          set_semantic_order(new_version)
+        end
+      end
+
+      def self.set_sequential_order(new_version)
+        set_order new_version, PactBroker::Versions::Sequence.next_val
+      end
+
+      def self.set_semantic_order(new_version)
         order_set = false
 
         PactBroker::Domain::Version.for_update.where(pacticipant_id: new_version.pacticipant_id).exclude(order: nil).reverse(:order).each do | existing_version |

lib/pact_broker/verifications/sequence.rb

@@ -1,10 +1,8 @@
-
 require 'sequel'
 
 module PactBroker
   module Verifications
     class Sequence < Sequel::Model(:verification_sequence_number)
-
       dataset_module do
         # The easiest way to implement a cross database compatible sequence.
         # Sad, I know.

lib/pact_broker/versions/sequence.rb

@@ -0,0 +1,38 @@
+require 'sequel'
+
+module PactBroker
+  module Versions
+    class Sequence < Sequel::Model(:version_sequence_number)
+
+      dataset_module do
+        # 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, after we've truncated all the
+              # tables after a test.
+              # 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_version_order = PactBroker::Domain::Version.max(:order)
+              value = max_version_order ? max_version_order + 100 : 1
+              insert(value: value)
+              value
+            end
+          end
+        end
+      end
+    end
+  end
+end
+
+# Table: version_sequence_number
+# Columns:
+#  value | integer | NOT NULL

spec/lib/pact_broker/db/data_migrations/set_latest_version_sequence_value_spec.rb

@@ -0,0 +1,68 @@
+require 'pact_broker/db/data_migrations/set_latest_version_sequence_value'
+
+module PactBroker
+  module DB
+    module DataMigrations
+      describe SetLatestVersionSequenceValue, data_migration: true do
+        include MigrationHelpers
+
+        describe ".call" do
+          before (:all) do
+            PactBroker::Database.migrate(20190509)
+          end
+
+          let(:now) { DateTime.new(2018, 2, 2) }
+
+          subject { SetLatestVersionSequenceValue.call(database) }
+
+          context "when there is no sequence value set" do
+            context "when there are no versions" do
+              it "initializes the sequence value - this is required at start up each time in case someone has changed the ordering configuration (date vs semantic)" do
+                subject
+                expect(database[:version_sequence_number].first[:value]).to eq 100
+              end
+            end
+
+            context "when there are pre-existing versions" do
+              let!(:consumer) { create(:pacticipants, {name: 'Consumer', created_at: now, updated_at: now}) }
+              let!(:consumer_version) { create(:versions, {number: '1.2.3', order: 1, pacticipant_id: consumer[:id], created_at: now, updated_at: now}) }
+              let!(:consumer_version) { create(:versions, {number: '1.2.5', order: 3, pacticipant_id: consumer[:id], created_at: now, updated_at: now}) }
+
+              it "initializes the sequence value to the max version order with a margin of error" do
+                subject
+                expect(database[:version_sequence_number].first[:value]).to eq 103
+              end
+            end
+          end
+
+          context "when a value already exists and it is already higher than the max_order" do
+            before do
+              database[:version_sequence_number].insert(value: 5)
+            end
+
+            it "does not update the value" do
+              subject
+              expect(database[:version_sequence_number].first[:value]).to eq 5
+              expect(database[:version_sequence_number].count).to eq 1
+            end
+          end
+
+          context "when a value already exists and it not higher than the max_order" do
+            before do
+              database[:version_sequence_number].insert(value: 3)
+            end
+
+            let!(:consumer) { create(:pacticipants, {name: 'Consumer', created_at: now, updated_at: now}) }
+            let!(:consumer_version) { create(:versions, {number: '1.2.3', order: 1, pacticipant_id: consumer[:id], created_at: now, updated_at: now}) }
+            let!(:consumer_version) { create(:versions, {number: '1.2.5', order: 3, pacticipant_id: consumer[:id], created_at: now, updated_at: now}) }
+
+            it "updates the value" do
+              subject
+              expect(database[:version_sequence_number].first[:value]).to eq 103
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/pact_broker/domain/order_versions_spec.rb

@@ -24,7 +24,7 @@
     end
   end
 
-  context "when order_versions_by_date is true (not recommended)" do
+  context "when order_versions_by_date is true (recommended)" do
     before do
       allow(PactBroker.configuration).to receive(:order_versions_by_date).and_return(true)
     end
@@ -69,7 +69,6 @@
     end
 
     context "when the new version is considered to be earlier than the previous latest version" do
-
       before do
         Sequel::Model.db[:versions].where(number: '2').update(number: 'z')
         Sequel::Model.db[:versions].where(number: '3').update(number: 'a')
@@ -80,8 +79,6 @@
         PactBroker::Domain::Version.create(number: '2', pacticipant_id: consumer.id)
         expect(ordered_versions).to eq(['1', 'z', 'a', '2', '4'])
       end
-
     end
   end
-
 end

spec/lib/pact_broker/versions/repository_spec.rb

@@ -93,7 +93,7 @@ module Versions
             expect(subject.number).to eq version_number
             expect(subject.pacticipant.name).to eq pacticipant_name
             expect(subject.tags.first.name).to eq "prod"
-            expect(subject.order).to eq 0
+            expect(subject.order).to_not be nil
           end
 
           context "when case sensitivity is turned off and names with different cases are used" do

spec/support/database_cleaner.rb

@@ -3,7 +3,7 @@
 
 RSpec.configure do |config|
 
-  config.include MigrationHelpers, migration: true
+  config.include MigrationHelpers, migration: true, data_migration: true
 
   config.before(:suite) do
     if defined?(::DB)
@@ -20,11 +20,21 @@
     PactBroker::Database.drop_objects
   end
 
+
   config.after :each, migration: true do
     PactBroker::Database.migrate
     PactBroker::Database.truncate
   end
 
+  config.after :each, data_migration: true do
+    PactBroker::Database.truncate
+  end
+
+  config.after :all, data_migration: true do
+    PactBroker::Database.migrate
+    PactBroker::Database.truncate
+  end
+
   config.before(:each) do | example |
     unless example.metadata[:no_db_clean] || example.metadata[:migration]
       DatabaseCleaner.start if defined?(::DB)