From cd3778500d21a764ad5413a0429ab34a64ece7c6 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 25 Apr 2019 17:40:10 +1000 Subject: [PATCH] feat: add ids to interactions when pacts are published These will be used to match test results published back to the broker with the correct interaction. --- lib/pact_broker/pacts/content.rb | 23 +++++++ .../pacts/generate_interaction_sha.rb | 23 +++++++ lib/pact_broker/pacts/service.rb | 13 +++- spec/features/merge_pact_spec.rb | 2 +- spec/features/publish_pact_spec.rb | 4 +- spec/lib/pact_broker/pacts/content_spec.rb | 48 +++++++++++++ .../pacts/generate_interaction_sha_spec.rb | 69 +++++++++++++++++++ spec/lib/pact_broker/pacts/service_spec.rb | 32 +++++++++ 8 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 lib/pact_broker/pacts/generate_interaction_sha.rb create mode 100644 spec/lib/pact_broker/pacts/generate_interaction_sha_spec.rb diff --git a/lib/pact_broker/pacts/content.rb b/lib/pact_broker/pacts/content.rb index 05be78a95..5df54dfa3 100644 --- a/lib/pact_broker/pacts/content.rb +++ b/lib/pact_broker/pacts/content.rb @@ -1,9 +1,11 @@ require 'pact_broker/pacts/parse' require 'pact_broker/pacts/sort_content' +require 'pact_broker/pacts/generate_interaction_sha' module PactBroker module Pacts class Content + include GenerateInteractionSha def initialize pact_hash @pact_hash = pact_hash @@ -29,6 +31,18 @@ def sort Content.from_hash(SortContent.call(pact_hash)) end + def with_ids + new_pact_hash = pact_hash.dup + if interactions && interactions.is_a?(Array) + new_pact_hash['interactions'] = add_ids(interactions) + end + + if messages && messages.is_a?(Array) + new_pact_hash['messages'] = add_ids(messages) + end + Content.from_hash(new_pact_hash) + end + # Half thinking this belongs in GenerateSha def content_that_affects_verification_results if interactions || messages @@ -61,6 +75,15 @@ def pact_specification_version attr_reader :pact_hash + def add_ids(interactions) + interactions.map do | interaction | + if interaction.is_a?(Hash) + interaction.merge("id" => generate_interaction_sha(interaction)) + else + interaction + end + end + end end end end diff --git a/lib/pact_broker/pacts/generate_interaction_sha.rb b/lib/pact_broker/pacts/generate_interaction_sha.rb new file mode 100644 index 000000000..9be310bbf --- /dev/null +++ b/lib/pact_broker/pacts/generate_interaction_sha.rb @@ -0,0 +1,23 @@ +require 'digest/sha1' +require 'pact_broker/configuration' +require 'pact_broker/pacts/sort_content' +require 'pact_broker/pacts/parse' +require 'pact_broker/pacts/content' + +module PactBroker + module Pacts + module GenerateInteractionSha + def self.call interaction_hash, options = {} + ordered_interaction_hash = interaction_hash.keys.sort.each_with_object({}) do | key, new_interaction_hash | + new_interaction_hash[key] = interaction_hash[key] unless key == "id" + end + + Digest::SHA1.hexdigest(ordered_interaction_hash.to_json) + end + + def generate_interaction_sha interaction_hash, options = {} + GenerateInteractionSha.call(interaction_hash, options) + end + end + end +end diff --git a/lib/pact_broker/pacts/service.rb b/lib/pact_broker/pacts/service.rb index adf645c1f..9d935b417 100644 --- a/lib/pact_broker/pacts/service.rb +++ b/lib/pact_broker/pacts/service.rb @@ -121,7 +121,9 @@ def update_pact params, existing_pact logger.info "Updating existing pact publication with params #{params.reject{ |k, v| k == :json_content}}" logger.debug "Content #{params[:json_content]}" pact_version_sha = generate_sha(params[:json_content]) - updated_pact = pact_repository.update(existing_pact.id, params.merge(pact_version_sha: pact_version_sha)) + json_content = add_interaction_ids(params[:json_content]) + update_params = { pact_version_sha: pact_version_sha, json_content: json_content } + updated_pact = pact_repository.update(existing_pact.id, update_params) webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED # TODO this should use the sha! @@ -137,12 +139,15 @@ def update_pact params, existing_pact def create_pact params, version, provider logger.info "Creating new pact publication with params #{params.reject{ |k, v| k == :json_content}}" logger.debug "Content #{params[:json_content]}" + pact_version_sha = generate_sha(params[:json_content]) + json_content = add_interaction_ids(params[:json_content]) pact = pact_repository.create( json_content: params[:json_content], version_id: version.id, provider_id: provider.id, consumer_id: version.pacticipant_id, - pact_version_sha: generate_sha(params[:json_content]) + pact_version_sha: pact_version_sha, + json_content: json_content ) trigger_webhooks pact pact @@ -152,6 +157,10 @@ def generate_sha(json_content) PactBroker.configuration.sha_generator.call(json_content) end + def add_interaction_ids(json_content) + Content.from_json(json_content).with_ids.to_json + end + def trigger_webhooks pact # TODO add tests for this webhook_service.trigger_webhooks pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED diff --git a/spec/features/merge_pact_spec.rb b/spec/features/merge_pact_spec.rb index 8b3f8264f..02aef8873 100644 --- a/spec/features/merge_pact_spec.rb +++ b/spec/features/merge_pact_spec.rb @@ -20,7 +20,7 @@ end it "returns the pact in the body" do - expect(response_body_json).to include JSON.parse(pact_content) + expect(response_body_json).to match_pact JSON.parse(pact_content) end end diff --git a/spec/features/publish_pact_spec.rb b/spec/features/publish_pact_spec.rb index 71be0bb42..d8e381354 100644 --- a/spec/features/publish_pact_spec.rb +++ b/spec/features/publish_pact_spec.rb @@ -16,7 +16,7 @@ end it "returns the pact in the body" do - expect(response_body_json).to include JSON.parse(pact_content) + expect(response_body_json).to match_pact JSON.parse(pact_content) end end @@ -35,7 +35,7 @@ end it "returns the pact in the response body" do - expect(response_body_json).to include JSON.parse(pact_content) + expect(response_body_json).to match_pact JSON.parse(pact_content) end end diff --git a/spec/lib/pact_broker/pacts/content_spec.rb b/spec/lib/pact_broker/pacts/content_spec.rb index 54c67f08e..8929b3ae8 100644 --- a/spec/lib/pact_broker/pacts/content_spec.rb +++ b/spec/lib/pact_broker/pacts/content_spec.rb @@ -3,6 +3,54 @@ module PactBroker module Pacts describe Content do + describe "with_ids" do + let(:pact_hash) do + { + 'ignored' => 'foo', + 'interactions' => [interaction], + 'metadata' => { + 'foo' => 'bar' + } + } + end + let(:interaction) { { "foo" => "bar" } } + + before do + allow(GenerateInteractionSha).to receive(:call).and_return("some-id") + end + + context "when the interaction is a hash" do + it "adds ids to the interactions" do + expect(Content.from_hash(pact_hash).with_ids.interactions.first["id"]).to eq "some-id" + end + end + + context "when the interaction is not a hash" do + let(:interaction) { 1 } + + it "does not add an id" do + expect(Content.from_hash(pact_hash).with_ids.interactions.first).to eq interaction + end + end + + context "when the pact is a message pact" do + let(:pact_hash) do + { + 'ignored' => 'foo', + 'messages' => [interaction], + 'metadata' => { + 'foo' => 'bar' + } + } + end + + it "adds ids to the messages" do + expect(Content.from_hash(pact_hash).with_ids.messages.first["id"]).to eq "some-id" + end + end + end + + describe "content_that_affects_verification_results" do subject { Content.from_hash(pact_hash).content_that_affects_verification_results } diff --git a/spec/lib/pact_broker/pacts/generate_interaction_sha_spec.rb b/spec/lib/pact_broker/pacts/generate_interaction_sha_spec.rb new file mode 100644 index 000000000..37326e9f4 --- /dev/null +++ b/spec/lib/pact_broker/pacts/generate_interaction_sha_spec.rb @@ -0,0 +1,69 @@ +require 'pact_broker/pacts/generate_interaction_sha' + +module PactBroker + module Pacts + describe GenerateInteractionSha do + describe ".call" do + let(:interaction_hash) do + { + "description" => "foo", + "providerStates" => [ + "name" => "bar", + "params" => { + "wiffle" => "bar", + "meep" => "eek" + } + ] + } + end + + let(:interaction_hash_with_different_key_order) do + { + "providerStates" => [ + "name" => "bar", + "params" => { + "wiffle" => "bar", + "meep" => "eek" + } + ], + "description" => "foo" + } + end + + let(:interaction_hash_with_different_params_order) do + { + "description" => "foo", + "providerStates" => [ + "name" => "bar", + "params" => { + "meep" => "eek", + "wiffle" => "bar" + } + ] + } + end + + it "generates a SHA based on the sorted keys" do + expect(GenerateInteractionSha.call(interaction_hash)).to eq "5ec1cc12132d3437a5a2ced5537cdab2d4f44521" + end + + it "generates the same SHA if the top level keys are ordered differently" do + expect(GenerateInteractionSha.call(interaction_hash)).to eq GenerateInteractionSha.call(interaction_hash_with_different_key_order) + end + + # This could be a whole lot smarter, but I'm not sure it's worth it. + # eg. order of provider state params doesn't matter, but the ordering + # of the provider states themselves may... who knows. + # Let's not try and be too smart about it until we have a use case to flesh it out. + it "generates a different SHA if any of the other keys are ordered differently" do + expect(GenerateInteractionSha.call(interaction_hash)).to_not eq GenerateInteractionSha.call(interaction_hash_with_different_params_order) + end + + it "ignores any existing id in the hash" do + interaction_hash["id"] = "foo" + expect(GenerateInteractionSha.call(interaction_hash)).to eq "5ec1cc12132d3437a5a2ced5537cdab2d4f44521" + end + end + end + end +end diff --git a/spec/lib/pact_broker/pacts/service_spec.rb b/spec/lib/pact_broker/pacts/service_spec.rb index e4f4fd2bb..87e92e278 100644 --- a/spec/lib/pact_broker/pacts/service_spec.rb +++ b/spec/lib/pact_broker/pacts/service_spec.rb @@ -31,6 +31,7 @@ module Pacts let(:existing_pact) { nil } let(:new_pact) { double('new_pact', json_content: json_content) } let(:json_content) { { the: "contract" }.to_json } + let(:json_content_with_ids) { { the: "contract with ids" }.to_json } let(:previous_pacts) { [] } let(:params) do { @@ -40,10 +41,30 @@ module Pacts json_content: json_content } end + let(:content) { double('content') } + let(:content_with_interaction_ids) { double('content_with_interaction_ids', to_json: json_content_with_ids) } + + before do + allow(Content).to receive(:from_json).and_return(content) + allow(content).to receive(:with_ids).and_return(content_with_interaction_ids) + allow(PactBroker::Pacts::GenerateSha).to receive(:call).and_call_original + end subject { Service.create_or_update_pact(params) } + context "when no pact exists with the same params" do + it "creates the sha before adding the interaction ids" do + expect(PactBroker::Pacts::GenerateSha).to receive(:call).ordered + expect(content).to receive(:with_ids).ordered + subject + end + + it "saves the pact interactions/messages with ids added to them" do + expect(pact_repository).to receive(:create).with hash_including(json_content: json_content_with_ids) + subject + end + it "triggers webhooks for contract publications" do expect(webhook_service).to receive(:trigger_webhooks).with(new_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED) subject @@ -53,6 +74,17 @@ module Pacts context "when a pact exists with the same params" do let(:existing_pact) { double('existing_pact', id: 4, json_content: { the: "contract" }.to_json) } + it "creates the sha before adding the interaction ids" do + expect(PactBroker::Pacts::GenerateSha).to receive(:call).ordered + expect(content).to receive(:with_ids).ordered + subject + end + + it "saves the pact interactions/messages with ids added to them" do + expect(pact_repository).to receive(:update).with(anything, hash_including(json_content: json_content_with_ids)) + subject + end + it "triggers webhooks for contract publications" do expect(webhook_service).to receive(:trigger_webhooks).with(new_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED) subject