feat: use a separate table to track the successful verifications of p…

…act versions for each provider version tag (feature toggled with "new_wip_calculation")

This is due to performance issues with calculating it using a live query on the verifications table.

db/migrations/20211120_create_pact_version_provider_tag_successful_verifications.rb

@@ -0,0 +1,23 @@
+Sequel.migration do
+  up do
+    create_table(:pact_version_provider_tag_successful_verifications, charset: "utf8") do
+      primary_key :id
+      foreign_key :pact_version_id, :pact_versions, null: false, on_delete: :cascade, foreign_key_constraint_name: "pact_version_provider_tag_successful_verifications_pact_version_id_fk"
+      String :provider_version_tag_name, null: false
+      Boolean :wip, null: false
+      Integer :verification_id
+      DateTime :execution_date, null: false
+      index([:pact_version_id, :provider_version_tag_name, :wip], unique: true, name: "pact_version_provider_tag_verifications_pv_pvtn_wip_unique")
+      # The implication of the on_delete: :set_null for verification_id is
+      # that even if the verification result is deleted from the broker,
+      # the wip/pending status stays the same.
+      # We may or may not want this. Will have to wait and see.
+      # Have made the foreign key a separate declaration so it can more easily be remade.
+      foreign_key([:verification_id], :verifications, on_delete: :set_null, name: "pact_version_provider_tag_successful_verifications_verification_id_fk")
+    end
+  end
+
+  down do
+    drop_table(:pact_version_provider_tag_successful_verifications)
+  end
+end

db/migrations/20211121_migrate_pact_version_provider_tag_successful_verifications_data.rb

@@ -0,0 +1,11 @@
+require "pact_broker/db/data_migrations/migrate_pact_version_provider_tag_successful_verifications"
+
+Sequel.migration do
+  up do
+    PactBroker::DB::DataMigrations::MigratePactVersionProviderTagSuccessfulVerifications.call(self)
+  end
+
+  down do
+
+  end
+end

lib/pact_broker/db/data_migrations/migrate_pact_version_provider_tag_successful_verifications.rb

@@ -0,0 +1,38 @@
+require "pact_broker/db/data_migrations/helpers"
+
+module PactBroker
+  module DB
+    module DataMigrations
+      class MigratePactVersionProviderTagSuccessfulVerifications
+        extend Helpers
+
+        def self.call(connection)
+          successful_verifications_join = {
+            Sequel[:sv][:pact_version_id] => Sequel[:verifications][:pact_version_id],
+            Sequel[:sv][:provider_version_tag_name] => Sequel[:tags][:name],
+            Sequel[:sv][:wip] => Sequel[:verifications][:wip]
+          }
+
+          missing_verifications = connection
+                                    .select(
+                                      Sequel[:verifications][:pact_version_id],
+                                      Sequel[:tags][:name],
+                                      Sequel[:verifications][:wip],
+                                      Sequel[:verifications][:id],
+                                      Sequel[:verifications][:execution_date]
+                                    )
+                                    .order(Sequel[:verifications][:execution_date], Sequel[:verifications][:id])
+                                    .from(:verifications)
+                                    .join(:tags, { Sequel[:verifications][:provider_version_id] => Sequel[:tags][:version_id] })
+                                    .left_outer_join(:pact_version_provider_tag_successful_verifications, successful_verifications_join, { table_alias: :sv })
+                                    .where(Sequel[:sv][:pact_version_id] => nil)
+                                    .where(Sequel[:verifications][:success] => true)
+
+          connection[:pact_version_provider_tag_successful_verifications]
+            .insert_ignore
+            .insert([:pact_version_id, :provider_version_tag_name, :wip, :verification_id, :execution_date], missing_verifications)
+        end
+      end
+    end
+  end
+end

lib/pact_broker/db/migrate_data.rb

@@ -28,6 +28,7 @@ def self.call database_connection, _options = {}
         DataMigrations::SetExtraColumnsForTags.call(database_connection)
         DataMigrations::CreateBranches.call(database_connection)
         DataMigrations::MigrateIntegrations.call(database_connection)
+        DataMigrations::MigratePactVersionProviderTagSuccessfulVerifications.call(database_connection)
       end
     end
   end

lib/pact_broker/pacts/pact_publication_wip_dataset_module.rb

@@ -33,6 +33,8 @@ def join_branch_versions_excluding_branch(branch_name)
       end
 
       def successfully_verified_by_provider_tag_when_not_wip(provider_id, provider_tag)
+        return new_successfully_verified_by_provider_tag_when_not_wip(provider_id, provider_tag) if PactBroker.feature_enabled?(:new_wip_calculation)
+
         from_self(alias: :pp)
           .select(Sequel[:pp].*)
           .where(Sequel[:pp][:provider_id] => provider_id)
@@ -41,7 +43,23 @@ def successfully_verified_by_provider_tag_when_not_wip(provider_id, provider_tag
           .distinct
       end
 
+      def new_successfully_verified_by_provider_tag_when_not_wip(provider_id, provider_tag)
+        pact_version_provider_tag_verifications_join = {
+          Sequel[:sv][:pact_version_id] => Sequel[:pp][:pact_version_id],
+          Sequel[:sv][:provider_version_tag_name] => provider_tag,
+          Sequel[:sv][:wip] => false
+        }
+
+        from_self(alias: :pp)
+          .select(Sequel[:pp].*)
+          .join(:pact_version_provider_tag_successful_verifications, pact_version_provider_tag_verifications_join, { table_alias: :sv })
+          .distinct
+
+      end
+
       def successfully_verified_by_provider_another_tag_before_this_tag_first_created(provider_id, provider_tag)
+        return new_successfully_verified_by_provider_another_tag_before_this_tag_first_created(provider_id, provider_tag) if PactBroker.feature_enabled?(:new_wip_calculation)
+
         first_tag_with_name = PactBroker::Domain::Tag.where(pacticipant_id: provider_id, name: provider_tag).order(:created_at).first
         from_self(alias: :pp)
           .select(Sequel[:pp].*)
@@ -54,6 +72,32 @@ def successfully_verified_by_provider_another_tag_before_this_tag_first_created(
           .distinct
       end
 
+      def new_successfully_verified_by_provider_another_tag_before_this_tag_first_created(provider_id, provider_tag)
+        first_tag_with_name = PactBroker::Domain::Tag.where(pacticipant_id: provider_id, name: provider_tag).order(:created_at).first
+
+        pact_version_provider_tag_verifications_join = {
+          Sequel[:sv][:pact_version_id] => Sequel[:pp][:pact_version_id],
+          Sequel[:sv][:wip] => false
+        }
+
+        created_at_criteria = if first_tag_with_name
+                                Sequel.lit("sv.execution_date < ?", first_tag_with_name.created_at)
+                              else
+                                Sequel.lit("1 = 1")
+                              end
+
+        from_self(alias: :pp)
+          .select(Sequel[:pp].*)
+          .where(Sequel[:pp][:provider_id] => provider_id)
+          .join(:pact_version_provider_tag_successful_verifications, pact_version_provider_tag_verifications_join, { table_alias: :sv }) do
+            Sequel.&(
+              Sequel.lit("sv.provider_version_tag_name NOT IN (?)", provider_tag),
+              created_at_criteria
+            )
+          end
+          .distinct
+      end
+
       protected
 
       def verified_before_date(date)

lib/pact_broker/test/test_data_builder.rb

@@ -31,6 +31,8 @@
 require "pact_broker/deployments/released_version_service"
 require "pact_broker/versions/branch_version_repository"
 require "pact_broker/integrations/repository"
+require "pact_broker/contracts/service"
+
 require "ostruct"
 
 module PactBroker
@@ -219,6 +221,27 @@ def create_label label_name
         self
       end
 
+      def publish_pact(consumer_name:, provider_name:, consumer_version_number: , tags: nil, branch: nil, build_url: nil, json_content: nil)
+        json_content = json_content || random_json_content(consumer_name, provider_name)
+        contracts = [
+          PactBroker::Contracts::ContractToPublish.from_hash(consumer_name: consumer_name, provider_name: provider_name, decoded_content: json_content, content_type: "application/json", specification: "pact")
+        ]
+        contracts_to_publish = PactBroker::Contracts::ContractsToPublish.from_hash(
+          pacticipant_name: consumer_name,
+          pacticipant_version_number: consumer_version_number,
+          tags: tags,
+          branch: branch,
+          build_url: build_url,
+          contracts: contracts
+        )
+        PactBroker::Contracts::Service.publish(contracts_to_publish, base_url: "http://example.org")
+        @consumer = find_pacticipant(consumer_name)
+        @consumer_version = find_version(consumer_name, consumer_version_number)
+        @provider = find_pacticipant(provider_name)
+        @pact = PactBroker::Pacts::PactPublication.last.to_domain
+        self
+      end
+
       def create_pact params = {}
         params.delete(:comment)
         json_content = params[:json_content] || default_json_content
@@ -363,6 +386,7 @@ def create_webhook_execution params = {}
       end
 
       def create_verification parameters = {}
+        # This should use the verification service. what a mess
         parameters.delete(:comment)
         branch = parameters.delete(:branch)
         tag_names = [parameters.delete(:tag_names), parameters.delete(:tag_name)].flatten.compact
@@ -373,20 +397,23 @@ def create_verification parameters = {}
         parameters.delete(:provider_version)
         verification = PactBroker::Domain::Verification.new(parameters)
         pact_version = PactBroker::Pacts::Repository.new.find_pact_version(@consumer, @provider, pact.pact_version_sha)
-        @verification = PactBroker::Verifications::Repository.new.create(verification, provider_version_number, pact_version)
-        @provider_version = PactBroker::Domain::Version.where(pacticipant_id: @provider.id, number: provider_version_number).single_record
+        @provider_version = version_repository.find_by_pacticipant_id_and_number_or_create(provider.id, provider_version_number)
         PactBroker::Versions::BranchVersionRepository.new.add_branch(@provider_version, branch) if branch
 
-        set_created_at_if_set(parameters[:created_at], :verifications, id: @verification.id)
-        set_created_at_if_set(parameters[:created_at], :versions, id: @provider_version.id)
-        set_created_at_if_set(parameters[:created_at], :latest_verification_id_for_pact_version_and_provider_version, pact_version_id: pact_version_id, provider_version_id: @provider_version.id)
-
         if tag_names.any?
           tag_names.each do | tag_name |
-            PactBroker::Domain::Tag.new(name: tag_name, version: @provider_version).insert_ignore
+            PactBroker::Domain::Tag.new(name: tag_name, version: @provider_version, version_order: @provider_version.order).insert_ignore
             set_created_at_if_set(parameters[:created_at], :tags, version_id: @provider_version.id, name: tag_name)
           end
         end
+
+        @verification = PactBroker::Verifications::Repository.new.create(verification, provider_version_number, pact_version)
+
+        set_created_at_if_set(parameters[:created_at], :verifications, id: @verification.id)
+        set_created_at_if_set(parameters[:created_at], :versions, id: @provider_version.id)
+        set_created_at_if_set(parameters[:created_at], :latest_verification_id_for_pact_version_and_provider_version, pact_version_id: pact_version_id, provider_version_id: @provider_version.id)
+        set_created_at_if_set(parameters[:created_at], :pact_version_provider_tag_successful_verifications, { verification_id: @verification.id }, :execution_date)
+
         self
       end
 
@@ -594,10 +621,10 @@ def prepare_json_content(json_content)
         PactBroker::Pacts::Content.from_json(json_content).with_ids(false).to_json
       end
 
-      def set_created_at_if_set created_at, table_name, selector
+      def set_created_at_if_set created_at, table_name, selector, date_column_name = :created_at
         date_to_set = created_at || @now
         if date_to_set
-          Sequel::Model.db[table_name].where(selector).update(created_at: date_to_set)
+          Sequel::Model.db[table_name].where(selector).update(date_column_name => date_to_set)
           if Sequel::Model.db.schema(table_name).any?{ |col| col.first == :updated_at }
             Sequel::Model.db[table_name].where(selector.keys.first => selector.values.first).update(updated_at: date_to_set)
           end

lib/pact_broker/verifications/pact_version_provider_tag_successful_verification.rb

@@ -0,0 +1,11 @@
+require "pact_broker/domain/verification"
+
+# Represents a non WIP, successful verification for a provider version with a tag.
+
+module PactBroker
+  module Verifications
+    class PactVersionProviderTagSuccessfulVerification < Sequel::Model
+      plugin :insert_ignore, identifying_columns: [:pact_version_id, :provider_version_tag_name, :wip]
+    end
+  end
+end

lib/pact_broker/verifications/repository.rb

@@ -2,6 +2,7 @@
 require "pact_broker/domain/verification"
 require "pact_broker/verifications/sequence"
 require "pact_broker/verifications/latest_verification_id_for_pact_version_and_provider_version"
+require "pact_broker/verifications/pact_version_provider_tag_successful_verification"
 
 module PactBroker
   module Verifications
@@ -27,6 +28,7 @@ def create verification, provider_version_number, pact_version
         verification.tag_names = version.tag_names # TODO pass this in from contracts service
         verification.save
         update_latest_verification_id(verification)
+        update_pact_version_provider_tag_verifications(verification, version.tag_names)
         verification
       end
 
@@ -46,6 +48,20 @@ def update_latest_verification_id verification
         LatestVerificationIdForPactVersionAndProviderVersion.new(params).upsert
       end
 
+      def update_pact_version_provider_tag_verifications(verification, tag_names)
+        if verification.success
+          tag_names&.each do | tag_name |
+            PactVersionProviderTagSuccessfulVerification.new(
+              pact_version_id: verification.pact_version_id,
+              provider_version_tag_name: tag_name,
+              wip: verification.wip,
+              verification_id: verification.id,
+              execution_date: verification.execution_date
+            ).insert_ignore
+          end
+        end
+      end
+
       def find consumer_name, provider_name, pact_version_sha, verification_number
         PactBroker::Domain::Verification
           .select_all_qualified

spec/lib/pact_broker/db/data_migrations/migrate_pact_version_provider_tag_successful_verifications_spec.rb

@@ -0,0 +1,51 @@
+require 'pact_broker/db/data_migrations/migrate_pact_version_provider_tag_successful_verifications'
+
+module PactBroker
+  module DB
+    module DataMigrations
+      describe MigratePactVersionProviderTagSuccessfulVerifications do
+        describe ".call" do
+          before do
+            td.set_now(Date.new(2020, 2, 1))
+              .publish_pact(consumer_name: "Foo", provider_name: "Bar", consumer_version_number: "1")
+              .set_now(Date.new(2021, 2, 1))
+              .create_verification(provider_version: "2", tag_names: ["feat/x"], wip: true, success: false)
+              .publish_pact(consumer_name: "Foo", provider_name: "Bar", consumer_version_number: "2")
+              .set_now(Date.new(2022, 2, 1))
+              .create_verification(provider_version: "3", tag_names: ["feat/x"], wip: true, success: true)
+              .set_now(Date.new(2023, 2, 1))
+              .create_verification(provider_version: "4", tag_names: ["feat/x"], wip: true, success: true, number: 2)
+              .set_now(Date.new(2024, 2, 1))
+              .create_verification(provider_version: "5", tag_names: ["feat/x"], wip: false, success: true, number: 3)
+              .set_now(Date.new(2025, 2, 1))
+              .create_verification(provider_version: "6", tag_names: ["feat/x"], wip: false, success: true, number: 4)
+
+              Sequel::Model.db[:pact_version_provider_tag_successful_verifications].delete
+          end
+
+          subject do
+            MigratePactVersionProviderTagSuccessfulVerifications.call(Sequel::Model.db)
+            MigratePactVersionProviderTagSuccessfulVerifications.call(Sequel::Model.db)
+          end
+
+          let(:first_verification) { Sequel::Model.db[:verifications].where(provider_version_id: Sequel::Model.db[:versions].select(:id).where(number: "3")).first }
+          let(:last_verification) { Sequel::Model.db[:verifications].where(provider_version_id: Sequel::Model.db[:versions].select(:id).where(number: "5")).first }
+
+          it "uses the date of the first verification for each wip status (true/false) for the row" do
+            expect { subject }.to change { Sequel::Model.db[:pact_version_provider_tag_successful_verifications].count }.by(2)
+
+            first_row = Sequel::Model.db[:pact_version_provider_tag_successful_verifications].order(:id).first
+
+            expect(first_row[:execution_date].to_s).to include "2022"
+            expect(first_row[:verification_id]).to eq first_verification[:id]
+
+            last_row = Sequel::Model.db[:pact_version_provider_tag_successful_verifications].order(:id).last
+
+            expect(last_row[:execution_date].to_s).to include "2024"
+            expect(last_row[:verification_id]).to eq last_verification[:id]
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/pact_broker/verifications/repository_spec.rb

@@ -11,7 +11,8 @@ module Verifications
         let(:verification) do
           PactBroker::Domain::Verification.new(
             success: true,
-            consumer_version_selector_hashes: [{ foo: "bar" }]
+            consumer_version_selector_hashes: [{ foo: "bar" }],
+            wip: false
           )
         end
 
@@ -42,6 +43,14 @@ module Verifications
           it "saves the consumer_version_selector_hashes" do
             expect(subject.reload.consumer_version_selector_hashes).to eq [{ foo: "bar" }]
           end
+
+          it "creates a PactVersionProviderTagSuccessfulVerification for each tag" do
+            expect { subject }.to change { PactVersionProviderTagSuccessfulVerification.count }.by(2)
+            expect(PactVersionProviderTagSuccessfulVerification.first).to have_attributes(
+              wip: false,
+              provider_version_tag_name: "foo"
+            )
+          end
         end
       end