diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb index ca15f9cc7..9c4e72d47 100644 --- a/lib/pact_broker/pacts/repository.rb +++ b/lib/pact_broker/pacts/repository.rb @@ -35,13 +35,18 @@ def update id, params existing_model = PactPublication.find(id: id) pact_version = find_or_create_pact_version(existing_model.consumer_version.pacticipant_id, existing_model.provider_id, params[:json_content]) if existing_model.pact_version_id != pact_version.id - pact_publication = PactPublication.new( + key = { consumer_version_id: existing_model.consumer_version_id, - consumer_id: existing_model.consumer_id, provider_id: existing_model.provider_id, - revision_number: (existing_model.revision_number + 1), - pact_version: pact_version, - ).save + 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 update_latest_pact_publication_ids(pact_publication) pact_publication.to_domain else @@ -49,6 +54,11 @@ def update id, params end end + # This logic is a separate method so we can stub it to create a "conflict" scenario + def next_revision_number(existing_model) + existing_model.revision_number + 1 + end + def update_latest_pact_publication_ids(pact_publication) params = { consumer_version_id: pact_publication.consumer_version_id, diff --git a/lib/pact_broker/repositories/helpers.rb b/lib/pact_broker/repositories/helpers.rb index 29f1132a3..c78e130a2 100644 --- a/lib/pact_broker/repositories/helpers.rb +++ b/lib/pact_broker/repositories/helpers.rb @@ -36,11 +36,12 @@ def select_for_subquery column end end - def upsert row, key_names + def upsert row, key_names, columns_to_update = nil if postgres? insert_conflict(update: row, target: key_names).insert(row) elsif mysql? - on_duplicate_key_update.insert(row) + update_cols = columns_to_update || (row.keys - key_names) + on_duplicate_key_update(*update_cols).insert(row) else # Sqlite key = row.reject{ |k, v| !key_names.include?(k) } diff --git a/spec/lib/pact_broker/pacts/repository_spec.rb b/spec/lib/pact_broker/pacts/repository_spec.rb index 237e0e231..19b7b380c 100644 --- a/spec/lib/pact_broker/pacts/repository_spec.rb +++ b/spec/lib/pact_broker/pacts/repository_spec.rb @@ -139,6 +139,7 @@ module Pacts let(:existing_pact) do TestDataBuilder.new.create_pact_with_hierarchy("A Consumer", "1.2.3", "A Provider", original_json_content).and_return(:pact) end + let(:repository) { Repository.new } before do ::DB::PACT_BROKER_DB[:pact_publications] @@ -155,10 +156,8 @@ module Pacts let(:original_json_content) { {some: 'json'}.to_json } let(:json_content) { {some_other: 'json'}.to_json } - context "when the attributes have changed" do - - subject { Repository.new.update existing_pact.id, json_content: json_content } + subject { repository.update existing_pact.id, json_content: json_content } it "creates a new PactVersion" do expect { subject }.to change{ PactBroker::Pacts::PactPublication.count }.by(1) @@ -186,11 +185,27 @@ module Pacts it "increments the revision_number by 1" do expect(subject.revision_number).to eq 2 end + + context "when there is a race condition" do + before do + allow(repository).to receive(:next_revision_number) { | existing_pact | existing_pact.revision_number } + end + + it "updates the existing row - yes this is destructive, by MySQL not supporting inner queries stops us doing a SELECT revision_number + 1" do + # And if we're conflicting the time between the two publications is so small that nobody + # can have depended on the content of the first pact + expect { subject }.to_not change{ PactBroker::Pacts::PactPublication.count } + end + + it "sets the content to the new content" do + expect(subject.json_content).to eq json_content + end + end end context "when the content has not changed" do - subject { Repository.new.update existing_pact.id, json_content: original_json_content } + subject { repository.update existing_pact.id, json_content: original_json_content } it "does not create a new PactVersion" do expect { subject }.to_not change{ PactBroker::Pacts::PactPublication.count } @@ -217,6 +232,7 @@ module Pacts .create_consumer_version("1.2.3") .create_provider(provider_name) .create_pact + .create_webhook .revise_pact .create_consumer_version("2.3.4") .create_pact