diff --git a/db/migrations/43_add_provider_version_to_verification.rb b/db/migrations/43_add_provider_version_to_verification.rb new file mode 100644 index 000000000..11fd1ccdb --- /dev/null +++ b/db/migrations/43_add_provider_version_to_verification.rb @@ -0,0 +1,12 @@ +Sequel.migration do + change do + alter_table(:verifications) do + add_foreign_key(:provider_version_id, :versions, foreign_key_constraint_name: 'fk_verifications_versions') + end + end + + # TODO + # alter_table(:verifications) do + # set_column_not_null(:provider_version_id) + # end +end diff --git a/db/migrations/44_populate_verifications_provider_version_id.rb b/db/migrations/44_populate_verifications_provider_version_id.rb new file mode 100644 index 000000000..3b7807fc3 --- /dev/null +++ b/db/migrations/44_populate_verifications_provider_version_id.rb @@ -0,0 +1,25 @@ +Sequel.migration do + up do + from(:verifications) + .select(Sequel[:verifications][:id], :provider_version, :provider_id, Sequel[:verifications][:created_at]) + .join(:pact_versions, {id: :pact_version_id}) + .each do | line | + version = from(:versions) + .where(number: line[:provider_version], pacticipant_id: line[:provider_id]).single_record + version_id = if version + version[:id] + else + from(:versions).insert( + number: line[:provider_version], + pacticipant_id: line[:provider_id], + created_at: line[:created_at], + updated_at: line[:created_at] + ) + end + from(:verifications).where(id: line[:id]).update(provider_version_id: version_id) + end + end + + down do + end +end diff --git a/db/migrations/45_set_verification_provider_number_nullable.rb b/db/migrations/45_set_verification_provider_number_nullable.rb new file mode 100644 index 000000000..b8e0a075d --- /dev/null +++ b/db/migrations/45_set_verification_provider_number_nullable.rb @@ -0,0 +1,7 @@ +Sequel.migration do + change do + alter_table(:verifications) do + set_column_allow_null(:provider_version) + end + end +end diff --git a/db/migrations/46_recreate_latest_verifications.rb b/db/migrations/46_recreate_latest_verifications.rb new file mode 100644 index 000000000..6ce2292b3 --- /dev/null +++ b/db/migrations/46_recreate_latest_verifications.rb @@ -0,0 +1,18 @@ +Sequel.migration do + up do + create_or_replace_view(:latest_verification_numbers, + "SELECT pact_version_id, MAX(number) latest_number + FROM verifications + GROUP BY pact_version_id") + + # The most recent verification for each pact version + # provider_version is DEPRECATED, use provider_version_number + create_or_replace_view(:latest_verifications, + "SELECT v.id, v.number, v.success, s.number as provider_version, v.build_url, v.pact_version_id, v.execution_date, v.created_at, v.provider_version_id, s.number as provider_version_number + FROM verifications v + INNER JOIN latest_verification_numbers lv + ON v.pact_version_id = lv.pact_version_id AND v.number = lv.latest_number + INNER JOIN versions s on v.provider_version_id = s.id" + ) + end +end diff --git a/lib/pact_broker/api/decorators/verification_decorator.rb b/lib/pact_broker/api/decorators/verification_decorator.rb index 277545318..43e622d56 100644 --- a/lib/pact_broker/api/decorators/verification_decorator.rb +++ b/lib/pact_broker/api/decorators/verification_decorator.rb @@ -6,7 +6,7 @@ module Decorators class VerificationDecorator < BaseDecorator property :provider_name, as: :providerName, writeable: :false - property :provider_version, as: :providerApplicationVersion + property :provider_version_number, as: :providerApplicationVersion, writeable: false property :success property :execution_date, as: :verificationDate property :build_url, as: :buildUrl diff --git a/lib/pact_broker/domain/relationship.rb b/lib/pact_broker/domain/relationship.rb index f3d74ab91..3de8c7c67 100644 --- a/lib/pact_broker/domain/relationship.rb +++ b/lib/pact_broker/domain/relationship.rb @@ -74,8 +74,8 @@ def pact_changed_since_last_verification? latest_verification.pact_version_sha != latest_pact.pact_version_sha end - def latest_verification_provider_version - latest_verification.provider_version + def latest_verification_provider_version_number + latest_verification.provider_version.number end def pacticipants diff --git a/lib/pact_broker/domain/verification.rb b/lib/pact_broker/domain/verification.rb index 37f4c5dca..3fc7b65ad 100644 --- a/lib/pact_broker/domain/verification.rb +++ b/lib/pact_broker/domain/verification.rb @@ -8,6 +8,7 @@ class Verification < Sequel::Model set_primary_key :id associate(:many_to_one, :pact_version, class: "PactBroker::Pacts::PactVersion", key: :pact_version_id, primary_key: :id) + associate(:many_to_one, :provider_version, class: "PactBroker::Domain::Version", key: :provider_version_id, primary_key: :id) def before_create super @@ -78,10 +79,13 @@ def provider .limit(1).select(:provider_id)) end + def provider_version_number + provider_version.number + end + def latest_pact_publication pact_version.latest_pact_publication end - end Verification.plugin :timestamps diff --git a/lib/pact_broker/ui/view_models/relationship.rb b/lib/pact_broker/ui/view_models/relationship.rb index a8bb28480..ee93af442 100644 --- a/lib/pact_broker/ui/view_models/relationship.rb +++ b/lib/pact_broker/ui/view_models/relationship.rb @@ -99,11 +99,11 @@ def warning? def verification_tooltip case @relationship.verification_status when :success - "Successfully verified by #{provider_name} (v#{@relationship.latest_verification_provider_version})" + "Successfully verified by #{provider_name} (v#{@relationship.latest_verification_provider_version_number})" when :stale - "Pact has changed since last successful verification by #{provider_name} (v#{@relationship.latest_verification_provider_version})" + "Pact has changed since last successful verification by #{provider_name} (v#{@relationship.latest_verification_provider_version_number})" when :failed - "Verification by #{provider_name} (v#{@relationship.latest_verification_provider_version}) failed" + "Verification by #{provider_name} (v#{@relationship.latest_verification_provider_version_number}) failed" else nil end diff --git a/lib/pact_broker/verifications/repository.rb b/lib/pact_broker/verifications/repository.rb index 01b5eced7..ba9b6811f 100644 --- a/lib/pact_broker/verifications/repository.rb +++ b/lib/pact_broker/verifications/repository.rb @@ -7,9 +7,13 @@ module Verifications class Repository include PactBroker::Repositories::Helpers + include PactBroker::Repositories - def create verification, pact + def create verification, provider_version_number, pact + provider = pacticipant_repository.find_by_name(pact.provider_name) + version = version_repository.find_by_pacticipant_id_and_number_or_create(provider.id, provider_version_number) verification.pact_version_id = pact_version_id_for(pact) + verification.provider_version = version verification.save end diff --git a/lib/pact_broker/verifications/service.rb b/lib/pact_broker/verifications/service.rb index 07be0414d..c7d7de161 100644 --- a/lib/pact_broker/verifications/service.rb +++ b/lib/pact_broker/verifications/service.rb @@ -19,9 +19,10 @@ def next_number_for pact def create next_verification_number, params, pact PactBroker.logger.info "Creating verification #{next_verification_number} for pact_id=#{pact.id} from params #{params}" verification = PactBroker::Domain::Verification.new + provider_version_number = params.fetch('providerApplicationVersion') PactBroker::Api::Decorators::VerificationDecorator.new(verification).from_hash(params) verification.number = next_verification_number - verification_repository.create(verification, pact) + verification_repository.create(verification, provider_version_number, pact) end def errors params diff --git a/spec/lib/pact_broker/api/decorators/verification_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/verification_decorator_spec.rb index 01cc01c31..9497361a0 100644 --- a/spec/lib/pact_broker/api/decorators/verification_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/verification_decorator_spec.rb @@ -9,7 +9,7 @@ module Decorators instance_double('PactBroker::Domain::Verification', number: 1, success: true, - provider_version: "4.5.6", + provider_version_number: "4.5.6", provider_name: 'Provider', consumer_name: 'Consumer', build_url: 'http://build-url', diff --git a/spec/lib/pact_broker/api/decorators/verification_summary_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/verification_summary_decorator_spec.rb index 64c14204f..233f5e511 100644 --- a/spec/lib/pact_broker/api/decorators/verification_summary_decorator_spec.rb +++ b/spec/lib/pact_broker/api/decorators/verification_summary_decorator_spec.rb @@ -12,7 +12,7 @@ module Decorators let(:verification) do instance_double("PactBroker::Domain::Verification", success: true, number: 1, - provider_version: '4.5.6', + provider_version_number: '4.5.6', build_url: 'http://some-build', provider_name: 'Provider', consumer_name: 'Consumer', diff --git a/spec/lib/pact_broker/ui/view_models/relationship_spec.rb b/spec/lib/pact_broker/ui/view_models/relationship_spec.rb index e0fbc62d4..259de4860 100644 --- a/spec/lib/pact_broker/ui/view_models/relationship_spec.rb +++ b/spec/lib/pact_broker/ui/view_models/relationship_spec.rb @@ -25,7 +25,7 @@ module ViewDomain instance_double("PactBroker::Domain::Relationship", verification_status: verification_status, provider_name: "Foo", - latest_verification_provider_version: "4.5.6") + latest_verification_provider_version_number: "4.5.6") end let(:ever_verified) { true } let(:pact_changed) { false } diff --git a/spec/lib/pact_broker/verifications/repository_spec.rb b/spec/lib/pact_broker/verifications/repository_spec.rb index 3eff57ee7..d17a4becc 100644 --- a/spec/lib/pact_broker/verifications/repository_spec.rb +++ b/spec/lib/pact_broker/verifications/repository_spec.rb @@ -61,7 +61,7 @@ module Verifications subject { Repository.new.find "Consumer1", "Provider1", pact.pact_version_sha, 2} it "finds the latest verifications for the given consumer version" do - expect(subject.provider_version).to eq "3.7.4" + expect(subject.provider_version_number).to eq "3.7.4" expect(subject.consumer_name).to eq "Consumer1" expect(subject.provider_name).to eq "Provider1" expect(subject.pact_version_sha).to eq pact.pact_version_sha @@ -95,8 +95,8 @@ module Verifications subject { Repository.new.find_latest_verifications_for_consumer_version("Consumer1", "1.2.3")} it "finds the latest verifications for the given consumer version" do - expect(subject.first.provider_version).to eq "7.8.9" - expect(subject.last.provider_version).to eq "6.5.4" + expect(subject.first.provider_version_number).to eq "7.8.9" + expect(subject.last.provider_version_number).to eq "6.5.4" end end @@ -116,7 +116,7 @@ module Verifications subject { Repository.new.find_latest_verification_for("Consumer1", "Provider1")} it "finds the latest verifications for the given consumer version" do - expect(subject.provider_version).to eq "7.8.9" + expect(subject.provider_version_number).to eq "7.8.9" end end @@ -142,7 +142,7 @@ module Verifications subject { Repository.new.find_latest_verification_for("Consumer1", "Provider1")} it "finds the latest verifications for the given consumer version" do - expect(subject.provider_version).to eq "7.8.9" + expect(subject.provider_version_number).to eq "7.8.9" end end @@ -173,7 +173,7 @@ module Verifications subject { Repository.new.find_latest_verification_for("Consumer1", "Provider1", 'prod')} it "finds the latest verifications for the given consumer version with the specified tag" do - expect(subject.provider_version).to eq "5.4.3" + expect(subject.provider_version_number).to eq "5.4.3" end context "when no verification exists" do @@ -212,7 +212,7 @@ module Verifications subject { Repository.new.find_latest_verification_for("Consumer1", "Provider1", :untagged)} it "finds the latest verifications for the given consumer version with no tag" do - expect(subject.provider_version).to eq "5.4.3" + expect(subject.provider_version_number).to eq "5.4.3" end end end diff --git a/spec/lib/pact_broker/verifications/service_spec.rb b/spec/lib/pact_broker/verifications/service_spec.rb index a92944e38..7b42b9920 100644 --- a/spec/lib/pact_broker/verifications/service_spec.rb +++ b/spec/lib/pact_broker/verifications/service_spec.rb @@ -43,6 +43,12 @@ module Verifications expect(verification.pact_version_id).to_not be_nil expect(verification.pact_version).to_not be_nil end + + it "sets the provider version" do + verification = create_verification + expect(verification.provider_version).to_not be nil + expect(verification.provider_version_number).to eq '4.5.6' + end end describe "#errors" do diff --git a/spec/migrations/44_add_provider_version_to_verification_spec.rb b/spec/migrations/44_add_provider_version_to_verification_spec.rb new file mode 100644 index 000000000..817545638 --- /dev/null +++ b/spec/migrations/44_add_provider_version_to_verification_spec.rb @@ -0,0 +1,60 @@ +describe 'add provider version relationship to verification (migrate 42-44)', migration: true do + before do + PactBroker::Database.migrate(42) + end + + let(:now) { DateTime.new(2018, 2, 2) } + let!(:consumer) { create(:pacticipants, {name: 'Consumer', created_at: now, updated_at: now}) } + let!(:provider) { create(:pacticipants, {name: 'Provider', created_at: now, updated_at: now}) } + let!(:provider) { create(:pacticipants, {name: 'Provider', created_at: now, updated_at: now}) } + let!(:consumer_version) { create(:versions, {number: '1.2.3', order: 1, pacticipant_id: consumer[:id], created_at: now, updated_at: now}) } + let!(:pact_version) { create(:pact_versions, {content: {some: 'json'}.to_json, sha: '1234', consumer_id: consumer[:id], provider_id: provider[:id], created_at: now}) } + let!(:pact_publication) do + create(:pact_publications, { + consumer_version_id: consumer_version[:id], + provider_id: provider[:id], + revision_number: 1, + pact_version_id: pact_version[:id], + created_at: (now - 1) + }) + end + let!(:verification) do + create(:verifications, { + number: 1, + success: true, + provider_version: '1.2.3', + pact_version_id: pact_version[:id], + execution_date: now, + created_at: now + }) + end + + subject { PactBroker::Database.migrate(46) } + + it "creates a version object" do + expect { subject }.to change { PactBroker::Domain::Version.count }.by(1) + end + + it "sets the foreign key to the new version" do + subject + expect(PactBroker::Domain::Verification.first.provider_version.number).to eq '1.2.3' + end + + it "sets the created_at date of the new version to the created_at of the verification" do + subject + expect(PactBroker::Domain::Verification.first.provider_version.created_at).to eq now + end + + context "when the version already exists" do + let!(:provider_version) { create(:versions, {number: '1.2.3', order: 1, pacticipant_id: provider[:id], created_at: now, updated_at: now}) } + + it "does not create a version object" do + expect { subject }.to_not change { PactBroker::Domain::Version.count } + end + + it "sets the foreign key to the existing version" do + subject + expect(PactBroker::Domain::Verification.first.provider_version.number).to eq '1.2.3' + end + end +end diff --git a/spec/support/test_data_builder.rb b/spec/support/test_data_builder.rb index ee92e243f..49c621996 100644 --- a/spec/support/test_data_builder.rb +++ b/spec/support/test_data_builder.rb @@ -201,9 +201,12 @@ def create_deprecated_webhook_execution params = {} end def create_verification parameters = {} - default_parameters = {success: true, provider_version: '4.5.6', number: 1} - verification = PactBroker::Domain::Verification.new(default_parameters.merge(parameters)) - @verification = PactBroker::Verifications::Repository.new.create(verification, @pact) + provider_version_number = parameters[:provider_version] || '4.5.6' + default_parameters = {success: true, number: 1} + parameters = default_parameters.merge(parameters) + parameters.delete(:provider_version) + verification = PactBroker::Domain::Verification.new(parameters) + @verification = PactBroker::Verifications::Repository.new.create(verification, provider_version_number, @pact) self end