fix: handle race condition causing unique constraint violation when c…

…reating pacticipant versions

@mefellows

lib/pact_broker/versions/repository.rb

@@ -51,10 +51,19 @@ def find_by_pacticipant_name_and_number pacticipant_name, number
           .single_record
       end
 
+      # There may be a race condition if two simultaneous requests come in to create the same version
       def create args
-        PactBroker.logger.info "Creating version #{args[:number]} for pacticipant_id=#{args[:pacticipant_id]}"
-        version = PactBroker::Domain::Version.new(number: args[:number], pacticipant_id: args[:pacticipant_id]).save
-        PactBroker::Domain::Version.find(id: version.id) # Need to reload with populated order
+        PactBroker.logger.info "Upserting version #{args[:number]} for pacticipant_id=#{args[:pacticipant_id]}"
+        version_params = {
+          number: args[:number],
+          pacticipant_id: args[:pacticipant_id],
+          created_at: Sequel.datetime_class.now,
+          updated_at: Sequel.datetime_class.now
+        }
+        id = PactBroker::Domain::Version.db[:versions].insert_ignore.insert(version_params)
+        version = PactBroker::Domain::Version.find(number: args[:number], pacticipant_id: args[:pacticipant_id])
+        PactBroker::Domain::OrderVersions.(version)
+        version.refresh # reload with the populated order
       end
 
       def find_by_pacticipant_id_and_number_or_create pacticipant_id, number

spec/lib/pact_broker/versions/repository_spec.rb

@@ -34,17 +34,25 @@ module Versions
 
       describe "#create" do
         context "when a previous version exists" do
-
-          let!(:existing_order) do
-            TestDataBuilder.new.create_version_with_hierarchy pacticipant_name, version_number
+          let!(:existing_version) do
+            TestDataBuilder.new.create_version_with_hierarchy(pacticipant_name, version_number).and_return(:version)
           end
 
-          subject { Repository.new.create pacticipant_id: existing_order.pacticipant_id, number: "1.2.4" }
+          subject { Repository.new.create pacticipant_id: existing_version.pacticipant_id, number: "1.2.4" }
 
           it "sets the order to the previous version's order plus one" do
-            expect(subject.order).to eq existing_order.order + 1
+            expect(subject.order).to eq existing_version.order + 1
           end
+        end
+
+        context "when the same version already exists" do
+          let!(:existing_version) { TestDataBuilder.new.create_version_with_hierarchy(pacticipant_name, version_number).and_return(:version) }
 
+          subject { Repository.new.create pacticipant_id: existing_version.pacticipant_id, number: version_number }
+
+          it "returns the pre-existing version" do
+            expect(subject.id).to eq existing_version.id
+          end
         end
       end
 

spec/support/test_data_builder.rb

@@ -121,12 +121,14 @@ def create_pact_with_hierarchy consumer_name = "Consumer", consumer_version = "1
   def create_version_with_hierarchy pacticipant_name, pacticipant_version
     pacticipant = PactBroker::Domain::Pacticipant.create(:name => pacticipant_name)
     version = PactBroker::Domain::Version.create(:number => pacticipant_version, :pacticipant => pacticipant)
-    PactBroker::Domain::Version.find(id: version.id) # Get version with populated order
+    @version = PactBroker::Domain::Version.find(id: version.id) # Get version with populated order
+    self
   end
 
   def create_tag_with_hierarchy pacticipant_name, pacticipant_version, tag_name
-    version = create_version_with_hierarchy pacticipant_name, pacticipant_version
-    PactBroker::Domain::Tag.create(name: tag_name, version: version)
+    create_version_with_hierarchy pacticipant_name, pacticipant_version
+    PactBroker::Domain::Tag.create(name: tag_name, version: @version)
+    self
   end
 
   def create_pacticipant pacticipant_name, params = {}