From 8cab4593e11a26b05f0467ac43531ee0e092413f Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Thu, 25 Apr 2019 17:06:53 +1000 Subject: [PATCH] refactor: move sha generation code out of repository and in to service in preparation for adding ids to interactions --- lib/pact_broker/pacts/repository.rb | 28 +++- lib/pact_broker/pacts/service.rb | 16 ++- lib/pact_broker/test/test_data_builder.rb | 32 ++++- spec/lib/pact_broker/pacts/repository_spec.rb | 121 ++++++++++++++---- 4 files changed, 157 insertions(+), 40 deletions(-) diff --git a/lib/pact_broker/pacts/repository.rb b/lib/pact_broker/pacts/repository.rb index 00a7b6e97..891475065 100644 --- a/lib/pact_broker/pacts/repository.rb +++ b/lib/pact_broker/pacts/repository.rb @@ -21,11 +21,17 @@ class Repository include PactBroker::Repositories def create params + pact_version = find_or_create_pact_version( + params.fetch(:consumer_id), + params.fetch(:provider_id), + params.fetch(:pact_version_sha), + params.fetch(:json_content) + ) pact_publication = PactPublication.new( consumer_version_id: params[:version_id], provider_id: params[:provider_id], consumer_id: params[:consumer_id], - pact_version: find_or_create_pact_version(params.fetch(:consumer_id), params.fetch(:provider_id), params[:json_content]), + pact_version: pact_version ).save update_latest_pact_publication_ids(pact_publication) pact_publication.to_domain @@ -33,7 +39,12 @@ def create params 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]) + pact_version = find_or_create_pact_version( + existing_model.consumer_version.pacticipant_id, + existing_model.provider_id, + params.fetch(:pact_version_sha), + params.fetch(:json_content) + ) if existing_model.pact_version_id != pact_version.id key = { consumer_version_id: existing_model.consumer_version_id, @@ -277,14 +288,19 @@ def different? pact, other_pact Pact::JsonDiffer.(pact.content_hash, other_pact.content_hash, allow_unexpected_keys: false).any? end - def find_or_create_pact_version consumer_id, provider_id, json_content - sha = PactBroker.configuration.sha_generator.call(json_content) - PactVersion.find(sha: sha, consumer_id: consumer_id, provider_id: provider_id) || create_pact_version(consumer_id, provider_id, sha, json_content) + def find_or_create_pact_version consumer_id, provider_id, pact_version_sha, json_content + PactVersion.find(sha: pact_version_sha, consumer_id: consumer_id, provider_id: provider_id) || create_pact_version(consumer_id, provider_id, pact_version_sha, json_content) end def create_pact_version consumer_id, provider_id, sha, json_content logger.debug("Creating new pact version for sha #{sha}") - pact_version = PactVersion.new(consumer_id: consumer_id, provider_id: provider_id, sha: sha, content: json_content) + # Content.from_json(json_content).with_ids.to_json + pact_version = PactVersion.new( + consumer_id: consumer_id, + provider_id: provider_id, + sha: sha, + content: json_content + ) pact_version.save end diff --git a/lib/pact_broker/pacts/service.rb b/lib/pact_broker/pacts/service.rb index 653820481..adf645c1f 100644 --- a/lib/pact_broker/pacts/service.rb +++ b/lib/pact_broker/pacts/service.rb @@ -120,9 +120,11 @@ def pact_is_new_or_pact_has_changed_since_previous_version? pact 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]}" - updated_pact = pact_repository.update existing_pact.id, params + pact_version_sha = generate_sha(params[:json_content]) + updated_pact = pact_repository.update(existing_pact.id, params.merge(pact_version_sha: pact_version_sha)) webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED + # TODO this should use the sha! if existing_pact.json_content != updated_pact.json_content webhook_service.trigger_webhooks updated_pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_CONTENT_CHANGED else @@ -135,11 +137,21 @@ 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 = pact_repository.create json_content: params[:json_content], version_id: version.id, provider_id: provider.id, consumer_id: version.pacticipant_id + 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]) + ) trigger_webhooks pact pact end + def generate_sha(json_content) + PactBroker.configuration.sha_generator.call(json_content) + end + def trigger_webhooks pact # TODO add tests for this webhook_service.trigger_webhooks pact, nil, PactBroker::Webhooks::WebhookEvent::CONTRACT_PUBLISHED diff --git a/lib/pact_broker/test/test_data_builder.rb b/lib/pact_broker/test/test_data_builder.rb index 7fbd6d198..bfe1d41d2 100644 --- a/lib/pact_broker/test/test_data_builder.rb +++ b/lib/pact_broker/test/test_data_builder.rb @@ -77,7 +77,13 @@ def create_contract_email_service_version number end def create_ces_cps_pact - @pact_id = pact_repository.create(version_id: @contract_email_service_version_id, consumer_id: @contract_email_service_id, provider_id: @contract_proposal_service_id, json_content: default_json_content).id + @pact_id = pact_repository.create( + version_id: @contract_email_service_version_id, + consumer_id: @contract_email_service_id, + provider_id: @contract_proposal_service_id, + json_content: default_json_content, + pact_version_sha: PactBroker.configuration.sha_generator.call(default_json_content) + ).id self end @@ -97,7 +103,13 @@ def create_pricing_service_version number end def create_condor_pricing_service_pact - @pact_id = pact_repository.create(version_id: @condor_version_id, consumer_id: @condor_id, provider_id: @pricing_service_id, json_content: default_json_content).id + @pact_id = pact_repository.create( + version_id: @condor_version_id, + consumer_id: @condor_id, + provider_id: @pricing_service_id, + json_content: default_json_content, + pact_version_sha: PactBroker.configuration.sha_generator.call(default_json_content) + ).id self end @@ -214,7 +226,15 @@ def create_label label_name def create_pact params = {} params.delete(:comment) - @pact = PactBroker::Pacts::Repository.new.create({version_id: @consumer_version.id, consumer_id: @consumer.id, provider_id: @provider.id, json_content: params[:json_content] || default_json_content}) + json_content = params[:json_content] || default_json_content + pact_version_sha = params[:pact_version_sha] || PactBroker.configuration.sha_generator.call(json_content) + @pact = PactBroker::Pacts::Repository.new.create( + version_id: @consumer_version.id, + consumer_id: @consumer.id, + provider_id: @provider.id, + json_content: json_content, + pact_version_sha: pact_version_sha + ) set_created_at_if_set params[:created_at], :pact_publications, {id: @pact.id} set_created_at_if_set params[:created_at], :pact_versions, {sha: @pact.pact_version_sha} @pact = PactBroker::Pacts::PactPublication.find(id: @pact.id).to_domain @@ -230,7 +250,11 @@ def republish_same_pact params = {} def revise_pact json_content = nil json_content = json_content ? json_content : {random: rand}.to_json - @pact = PactBroker::Pacts::Repository.new.update(@pact.id, json_content: json_content) + pact_version_sha = PactBroker.configuration.sha_generator.call(json_content) + @pact = PactBroker::Pacts::Repository.new.update(@pact.id, + json_content: json_content, + pact_version_sha: pact_version_sha + ) self end diff --git a/spec/lib/pact_broker/pacts/repository_spec.rb b/spec/lib/pact_broker/pacts/repository_spec.rb index 9c243ba7e..c68f67098 100644 --- a/spec/lib/pact_broker/pacts/repository_spec.rb +++ b/spec/lib/pact_broker/pacts/repository_spec.rb @@ -16,12 +16,23 @@ module Pacts let(:consumer) { Pacticipants::Repository.new.create name: 'Consumer' } let(:provider) { Pacticipants::Repository.new.create name: 'Provider' } let(:version) { PactBroker::Versions::Repository.new.create number: '1.2.3', pacticipant_id: consumer.id } + let(:pact_version_sha) { '123' } let(:json_content) { {some: 'json'}.to_json } - subject { Repository.new.create version_id: version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: json_content } + let(:params) do + { + version_id: version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: json_content, + pact_version_sha: pact_version_sha + } + end + + subject { Repository.new.create(params) } it "saves the pact" do - expect{subject}.to change{ PactPublication.count }.by(1) + expect{ subject }.to change{ PactPublication.count }.by(1) end it "sets the consumer_id" do @@ -43,18 +54,26 @@ module Pacts end context "when a pact already exists with exactly the same content" do - before do - PactBroker.configuration.base_equality_only_on_content_that_affects_verification_results = false - end - let(:another_version) { Versions::Repository.new.create number: '2.0.0', pacticipant_id: consumer.id } before do - Repository.new.create version_id: version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: json_content + Repository.new.create( + version_id: version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: json_content, + pact_version_sha: pact_version_sha + ) end subject do - Repository.new.create version_id: another_version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: json_content + Repository.new.create( + version_id: another_version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: json_content, + pact_version_sha: pact_version_sha + ) end it "does not the content" do @@ -77,13 +96,25 @@ module Pacts let(:sha_2) { '1' } before do - PactBroker.configuration.base_equality_only_on_content_that_affects_verification_results = true - allow(PactBroker.configuration.sha_generator).to receive(:call).and_return(sha_1, sha_2) - Repository.new.create version_id: version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: json_content + # PactBroker.configuration.base_equality_only_on_content_that_affects_verification_results = true + # allow(PactBroker.configuration.sha_generator).to receive(:call).and_return(sha_1, sha_2) + Repository.new.create( + version_id: version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: json_content, + pact_version_sha: sha_1 + ) end subject do - Repository.new.create version_id: another_version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: json_content + Repository.new.create( + version_id: another_version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: json_content, + pact_version_sha: sha_2 + ) end context "when the sha is the same" do @@ -105,11 +136,23 @@ module Pacts let(:another_version) { Versions::Repository.new.create number: '2.0.0', pacticipant_id: consumer.id } let(:another_provider) { Pacticipants::Repository.new.create name: 'Provider2' } before do - Repository.new.create version_id: version.id, consumer_id: consumer.id, provider_id: another_provider.id, json_content: json_content + Repository.new.create( + version_id: version.id, + consumer_id: consumer.id, + provider_id: another_provider.id, + json_content: json_content, + pact_version_sha: pact_version_sha + ) end subject do - Repository.new.create version_id: another_version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: json_content + Repository.new.create( + version_id: another_version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: json_content, + pact_version_sha: pact_version_sha + ) end it "does not reuse the same PactVersion" do @@ -121,11 +164,23 @@ module Pacts let(:another_version) { Versions::Repository.new.create number: '2.0.0', pacticipant_id: consumer.id } before do - Repository.new.create version_id: version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: {some_other: 'json_content'}.to_json + Repository.new.create( + version_id: version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: {some_other: 'json_content'}.to_json, + pact_version_sha: pact_version_sha + ) end subject do - Repository.new.create version_id: another_version.id, consumer_id: consumer.id, provider_id: provider.id, json_content: json_content + Repository.new.create( + version_id: another_version.id, + consumer_id: consumer.id, + provider_id: provider.id, + json_content: json_content, + pact_version_sha: "#{pact_version_sha}111" + ) end it "creates a new PactVersion" do @@ -137,27 +192,33 @@ module Pacts describe "update" do let(:existing_pact) do - TestDataBuilder.new.create_pact_with_hierarchy("A Consumer", "1.2.3", "A Provider", original_json_content).and_return(:pact) + td.create_consumer("A Consumer") + .create_provider("A Provider") + .create_consumer_version("1.2.3") + .create_pact(pact_version_sha: pact_version_sha, json_content: original_json_content) + .and_return(:pact) end let(:repository) { Repository.new } before do - ::DB::PACT_BROKER_DB[:pact_publications] + PactPublication + .dataset .where(id: existing_pact.id) - .update( - created_at: created_at) - ::DB::PACT_BROKER_DB[:pact_versions] - .update( - created_at: created_at) + .update(created_at: created_at) + PactVersion + .dataset + .update(created_at: created_at) end let(:created_at) { DateTime.new(2014, 3, 2) } let(:original_json_content) { {some: 'json'}.to_json } let(:json_content) { {some_other: 'json'}.to_json } + let(:pact_version_sha) { '123' } + let(:new_pact_version_sha) { '567' } - context "when the attributes have changed" do - subject { repository.update existing_pact.id, json_content: json_content } + context "when the pact_version_sha has changed" do + subject { repository.update(existing_pact.id, json_content: json_content, pact_version_sha: new_pact_version_sha) } it "creates a new PactVersion" do expect { subject }.to change{ PactBroker::Pacts::PactPublication.count }.by(1) @@ -203,9 +264,13 @@ module Pacts end end - context "when the content has not changed" do - - subject { repository.update existing_pact.id, json_content: original_json_content } + context "when the pact_version_sha has not changed" do + subject do + repository.update(existing_pact.id, + json_content: original_json_content, + pact_version_sha: pact_version_sha + ) + end it "does not create a new PactVersion" do expect { subject }.to_not change{ PactBroker::Pacts::PactPublication.count }