From de0d3b7fffc75e7f604c9eb470ab61b0451d9d3e Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Fri, 21 Jun 2019 08:17:01 +1000
Subject: [PATCH] fix(publish pacts): handle race condition when creating pact
 version

---
 lib/pact_broker/pacts/pact_version.rb         | 12 ++++++++++--
 lib/pact_broker/pacts/repository.rb           | 10 ++++------
 lib/pact_broker/repositories/helpers.rb       |  2 +-
 spec/lib/pact_broker/pacts/repository_spec.rb | 11 +++++++++++
 4 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/lib/pact_broker/pacts/pact_version.rb b/lib/pact_broker/pacts/pact_version.rb
index 41b66792b..197f61f62 100644
--- a/lib/pact_broker/pacts/pact_version.rb
+++ b/lib/pact_broker/pacts/pact_version.rb
@@ -1,13 +1,19 @@
 require 'sequel'
+require 'pact_broker/repositories/helpers'
 
 module PactBroker
   module Pacts
     class PactVersion < Sequel::Model(:pact_versions)
+      plugin :timestamps
       one_to_many :pact_publications, reciprocal: :pact_version
       one_to_many :verifications, reciprocal: :verification, order: :id, :class => "PactBroker::Domain::Verification"
       associate(:many_to_one, :provider, class: "PactBroker::Domain::Pacticipant", key: :provider_id, primary_key: :id)
       associate(:many_to_one, :consumer, class: "PactBroker::Domain::Pacticipant", key: :consumer_id, primary_key: :id)
 
+      dataset_module do
+        include PactBroker::Repositories::Helpers
+      end
+
       def name
         "Pact between #{consumer_name} and #{provider_name}"
       end
@@ -45,9 +51,11 @@ def consumer_versions
       def latest_consumer_version_number
         latest_consumer_version.number
       end
-    end
 
-    PactVersion.plugin :timestamps
+      def upsert
+        self.class.upsert(to_hash, [:consumer_id, :provider_id, :sha])
+      end
+    end
   end
 end
 
diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb
index e6e712fa1..40b2095fe 100644
--- a/lib/pact_broker/pacts/repository.rb
+++ b/lib/pact_broker/pacts/repository.rb
@@ -292,15 +292,13 @@ def find_or_create_pact_version consumer_id, provider_id, pact_version_sha, json
       end
 
       def create_pact_version consumer_id, provider_id, sha, json_content
-        logger.debug("Creating new pact version for sha #{sha}")
-        # Content.from_json(json_content).with_ids.to_json
-        pact_version = PactVersion.new(
+        PactVersion.new(
           consumer_id: consumer_id,
           provider_id: provider_id,
           sha: sha,
-          content: json_content
-        )
-        pact_version.save
+          content: json_content,
+          created_at: Sequel.datetime_class.now
+        ).upsert
       end
 
       def find_all_database_versions_between(consumer_name, options, base_class = LatestPactPublicationsByConsumerVersion)
diff --git a/lib/pact_broker/repositories/helpers.rb b/lib/pact_broker/repositories/helpers.rb
index 99cb2f3bf..bb65a4470 100644
--- a/lib/pact_broker/repositories/helpers.rb
+++ b/lib/pact_broker/repositories/helpers.rb
@@ -51,7 +51,7 @@ def upsert row, unique_key_names, columns_to_update = nil
             where(key).update(row)
           end
         end
-        model.find(row.select{ |key, _| unique_key_names.include?(key)} )
+        model.where(row.select{ |key, _| unique_key_names.include?(key)}).single_record
       end
     end
   end
diff --git a/spec/lib/pact_broker/pacts/repository_spec.rb b/spec/lib/pact_broker/pacts/repository_spec.rb
index c68f67098..6e4a32555 100644
--- a/spec/lib/pact_broker/pacts/repository_spec.rb
+++ b/spec/lib/pact_broker/pacts/repository_spec.rb
@@ -88,6 +88,17 @@ module Pacts
           it "reuses the same PactVersion to save room" do
             expect { subject }.to_not change{ PactVersion.count }
           end
+
+          context "when there is a race condition and the pact version gets created by another request in between attempting to find and create" do
+            before do
+              allow(PactVersion).to receive(:find).and_return(nil)
+            end
+
+            it "doesn't blow up" do
+              expect(PactVersion).to receive(:find)
+              subject
+            end
+          end
         end
 
         context "when base_equality_only_on_content_that_affects_verification_results is true" do