From 6c8e38fb05b14c4148e85b18fec756df86536a20 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 5 Jun 2019 15:14:03 +1000 Subject: [PATCH] fix: duplicate key value violates unique constraint "cv_prov_revision_unq" error when publishing identical pact resources at the same time --- lib/pact_broker/pacts/pact_publication.rb | 7 +++ lib/pact_broker/pacts/repository.rb | 13 ++-- lib/pact_broker/repositories/helpers.rb | 9 +-- .../pacts/pact_publication_spec.rb | 60 +++++++++++++++++++ 4 files changed, 77 insertions(+), 12 deletions(-) diff --git a/lib/pact_broker/pacts/pact_publication.rb b/lib/pact_broker/pacts/pact_publication.rb index 05f4a3432..beddeb12b 100644 --- a/lib/pact_broker/pacts/pact_publication.rb +++ b/lib/pact_broker/pacts/pact_publication.rb @@ -5,6 +5,7 @@ module PactBroker module Pacts class PactPublication < Sequel::Model(:pact_publications) + UNIQUE_CONSTRAINT_KEYS = [:consumer_version_id, :provider_id, :revision_number].freeze extend Forwardable @@ -57,6 +58,12 @@ def to_version_domain OpenStruct.new(number: consumer_version.number, pacticipant: consumer_version.pacticipant, tags: consumer_version.tags, order: consumer_version.order) end + def upsert + params = to_hash.merge(created_at: Sequel.datetime_class.now) + self.id = PactPublication.upsert(params, UNIQUE_CONSTRAINT_KEYS).id + self.refresh + end + private def cached_domain_for_delegation diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb index 7db682fc0..e6e712fa1 100644 --- a/lib/pact_broker/pacts/repository.rb +++ b/lib/pact_broker/pacts/repository.rb @@ -31,8 +31,9 @@ def create params consumer_version_id: params[:version_id], provider_id: params[:provider_id], consumer_id: params[:consumer_id], - pact_version: pact_version - ).save + pact_version: pact_version, + revision_number: 1 + ).upsert update_latest_pact_publication_ids(pact_publication) pact_publication.to_domain end @@ -46,18 +47,14 @@ def update id, params params.fetch(:json_content) ) if existing_model.pact_version_id != pact_version.id - key = { + pact_publication = PactPublication.new( consumer_version_id: existing_model.consumer_version_id, provider_id: existing_model.provider_id, revision_number: next_revision_number(existing_model), - } - new_params = key.merge( consumer_id: existing_model.consumer_id, pact_version_id: pact_version.id, created_at: Sequel.datetime_class.now - ) - PactPublication.upsert(new_params, key.keys) - pact_publication = PactPublication.where(key).single_record + ).upsert update_latest_pact_publication_ids(pact_publication) pact_publication.to_domain else diff --git a/lib/pact_broker/repositories/helpers.rb b/lib/pact_broker/repositories/helpers.rb index c78e130a2..99cb2f3bf 100644 --- a/lib/pact_broker/repositories/helpers.rb +++ b/lib/pact_broker/repositories/helpers.rb @@ -36,21 +36,22 @@ def select_for_subquery column end end - def upsert row, key_names, columns_to_update = nil + def upsert row, unique_key_names, columns_to_update = nil if postgres? - insert_conflict(update: row, target: key_names).insert(row) + insert_conflict(update: row, target: unique_key_names).insert(row) elsif mysql? - update_cols = columns_to_update || (row.keys - key_names) + update_cols = columns_to_update || (row.keys - unique_key_names) on_duplicate_key_update(*update_cols).insert(row) else # Sqlite - key = row.reject{ |k, v| !key_names.include?(k) } + key = row.reject{ |k, v| !unique_key_names.include?(k) } if where(key).count == 0 insert(row) else where(key).update(row) end end + model.find(row.select{ |key, _| unique_key_names.include?(key)} ) end end end diff --git a/spec/lib/pact_broker/pacts/pact_publication_spec.rb b/spec/lib/pact_broker/pacts/pact_publication_spec.rb index 29dab9112..5690bb857 100644 --- a/spec/lib/pact_broker/pacts/pact_publication_spec.rb +++ b/spec/lib/pact_broker/pacts/pact_publication_spec.rb @@ -3,6 +3,66 @@ module PactBroker module Pacts describe PactPublication do + describe "save and upsert" do + before do + td.create_consumer + .create_provider + .create_consumer_version + .create_pact + end + + let(:params) do + { + consumer_id: td.consumer.id, + provider_id: td.provider.id, + consumer_version_id: td.consumer_version.id, + pact_version_id: PactVersion.first.id, + revision_number: 1 + } + end + + let(:pact_publication) do + PactPublication.new(params) + end + + context "when using a PactPublication with the same provider/consumer version/revision number as an existing PactPublication" do + describe "save" do + it "raises a constraint exception" do + expect { pact_publication.save }.to raise_error Sequel::UniqueConstraintViolation + end + end + + describe "upsert" do + it "does not raise an error" do + pact_publication.upsert + end + + it "sets the relationship objects" do + pact_publication.upsert + expect(pact_publication.id).to_not be nil + expect(pact_publication.consumer.id).to eq td.consumer.id + expect(pact_publication.consumer.name).to eq td.consumer.name + end + + context "with objects instead of ids" do + let(:params) do + { + consumer: td.consumer, + provider: td.provider, + consumer_version: td.consumer_version, + pact_version: PactVersion.first, + revision_number: 1 + } + end + + it "also works" do + pact_publication.upsert + expect(pact_publication.consumer_id).to eq td.consumer.id + end + end + end + end + end describe "#latest_tag_names" do before do