feat: load latest verification for consumer/provider via relationship rather than repository

Author: bethesque

db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb

@@ -1,7 +1,7 @@
 Sequel.migration do
   up do
     # The latest verification id for each consumer version tag
-    create_view(:latest_verifications_ids_for_consumer_version_tags,
+    create_view(:latest_verification_ids_for_consumer_version_tags,
       "select
         pv.pacticipant_id as provider_id,
         lpp.consumer_id,
@@ -27,7 +27,7 @@
         Sequel[:prv][:order].as(:provider_version_order),
         )
       .select_append{ verifications.* }
-      .join(:latest_verifications_ids_for_consumer_version_tags,
+      .join(:latest_verification_ids_for_consumer_version_tags,
         {
           Sequel[:verifications][:id] => Sequel[:lv][:latest_verification_id],
         }, { table_alias: :lv })
@@ -45,6 +45,6 @@
 
   down do
     drop_view(:latest_verifications_for_consumer_version_tags)
-    drop_view(:latest_verifications_ids_for_consumer_version_tags)
+    drop_view(:latest_verification_ids_for_consumer_version_tags)
   end
 end

db/migrations/20180524_create_latest_verifications_for_consumer_and_provider.rb

@@ -0,0 +1,46 @@
+Sequel.migration do
+  up do
+    # The latest verification id for each consumer version tag
+    create_view(:latest_verification_ids_for_consumer_and_provider,
+      "select
+        pv.pacticipant_id as provider_id,
+        lpp.consumer_id,
+        max(v.id) as latest_verification_id
+      from verifications v
+      join latest_pact_publications_by_consumer_versions lpp
+        on v.pact_version_id = lpp.pact_version_id
+      join versions pv
+        on v.provider_version_id = pv.id
+      group by pv.pacticipant_id, lpp.consumer_id")
+
+    # The most recent verification for each consumer/consumer version tag/provider
+    latest_verifications = from(:verifications)
+      .select(
+        Sequel[:lv][:consumer_id],
+        Sequel[:lv][:provider_id],
+        Sequel[:pv][:sha].as(:pact_version_sha),
+        Sequel[:prv][:number].as(:provider_version_number),
+        Sequel[:prv][:order].as(:provider_version_order),
+        )
+      .select_append{ verifications.* }
+      .join(:latest_verification_ids_for_consumer_and_provider,
+        {
+          Sequel[:verifications][:id] => Sequel[:lv][:latest_verification_id],
+        }, { table_alias: :lv })
+      .join(:versions,
+        {
+          Sequel[:verifications][:provider_version_id] => Sequel[:prv][:id]
+        }, { table_alias: :prv })
+      .join(:pact_versions,
+        {
+          Sequel[:verifications][:pact_version_id] => Sequel[:pv][:id]
+        }, { table_alias: :pv })
+
+    create_or_replace_view(:latest_verifications_for_consumer_and_provider, latest_verifications)
+  end
+
+  down do
+    drop_view(:latest_verifications_for_consumer_and_provider)
+    drop_view(:latest_verification_ids_for_consumer_and_provider)
+  end
+end

lib/pact_broker/index/service.rb

@@ -18,8 +18,6 @@ def self.find_index_items options = {}
           .select_all_qualified
           .eager(:latest_triggered_webhooks)
           .eager(:webhooks)
-          .order(:consumer_name, :provider_name)
-          #.eager(verification: [:provider_version, :pact_version])
 
         if !options[:tags]
           rows = rows.where(consumer_version_tag_name: nil)

lib/pact_broker/matrix/aggregated_row.rb

@@ -55,11 +55,8 @@ def latest_verification_for_consumer_version_tag row
       end
 
       def overall_latest_verification
-        # This causes the
-        # SELECT "latest_verifications".* FROM "latest_verifications"
-        # query in the logs for the tagged index.
-        # Given it only happens rarely, it's ok to not lazy load it.
-        PactBroker::Verifications::Repository.new.find_latest_verification_for(consumer_name, provider_name)
+        # not eager loaded because it shouldn't be called that often
+        first_row.latest_verification_for_consumer_and_provider
       end
 
       def first_row

lib/pact_broker/matrix/row.rb

@@ -3,6 +3,7 @@
 require 'pact_broker/tags/tag_with_latest_flag'
 require 'pact_broker/logging'
 require 'pact_broker/verifications/latest_verification_for_consumer_version_tag'
+require 'pact_broker/verifications/latest_verification_for_consumer_and_provider'
 
 module PactBroker
   module Matrix
@@ -17,6 +18,10 @@ class Row < Sequel::Model(:materialized_matrix)
       associate(:one_to_many, :consumer_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :consumer_version_id, key: :version_id)
       associate(:one_to_many, :provider_version_tags, :class => "PactBroker::Tags::TagWithLatestFlag", primary_key: :provider_version_id, key: :version_id)
 
+      many_to_one :latest_verification_for_consumer_and_provider,
+        :class => "PactBroker::Verifications::LatestVerificationForConsumerAndProvider",
+        primary_key: [:provider_id, :consumer_id], key: [:provider_id, :consumer_id]
+
       dataset_module do
         include PactBroker::Repositories::Helpers
         include PactBroker::Logging

lib/pact_broker/verifications/latest_verification_for_consumer_and_provider.rb

@@ -0,0 +1,26 @@
+require 'pact_broker/domain/verification'
+
+module PactBroker
+  module Verifications
+
+    include PactBroker::Repositories::Helpers
+
+    class LatestVerificationForConsumerAndProvider < PactBroker::Domain::Verification
+      set_dataset(:latest_verifications_for_consumer_and_provider)
+
+      # Don't need to load the pact_version as we do in the superclass,
+      # as pact_version_sha is included in the view for convenience
+      def pact_version_sha
+        values[:pact_version_sha]
+      end
+
+      def provider_version_number
+        values[:provider_version_number]
+      end
+
+      def provider_version_order
+        values[:provider_version_order]
+      end
+    end
+  end
+end

lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb

@@ -5,9 +5,9 @@ module Verifications
     class LatestVerificationForConsumerVersionTag < PactBroker::Domain::Verification
       set_dataset(:latest_verifications_for_consumer_version_tags)
 
+      # Don't need to load the pact_version as we do in the superclass,
+      # as pact_version_sha is included in the view for convenience
       def pact_version_sha
-        # Don't need to load the pact_version as we do in the superclass,
-        # as pact_version_sha is included in the view for convenience
         values[:pact_version_sha]
       end
 

spec/lib/pact_broker/matrix/aggregated_row_spec.rb

@@ -45,22 +45,21 @@ module Matrix
         end
 
         context "when there is no verification for any of the rows or any of the pacts with the same tag" do
-          before do
-            allow_any_instance_of(PactBroker::Verifications::Repository).to receive(:find_latest_verification_for).and_return(overall_latest_verification)
-          end
-
-          let(:overall_latest_verification) { instance_double('PactBroker::Domain::Verification', id: 5) }
           let(:verification_1) { nil }
           let(:verification_2) { nil }
           let(:tag_verification_1) { nil }
           let(:tag_verification_2) { nil }
 
           context "when one of the rows is the overall latest" do
             let(:consumer_version_tag_name_1) { nil }
+            let(:overall_latest_verification) { instance_double('PactBroker::Domain::Verification', id: 1) }
+            before do
+              allow(row_1).to receive(:latest_verification_for_consumer_and_provider).and_return(overall_latest_verification)
+            end
 
             it "looks up the overall latest verification" do
-             expect_any_instance_of(PactBroker::Verifications::Repository).to receive(:find_latest_verification_for).with("Foo", "Bar")
-             subject
+              expect(row_1).to receive(:latest_verification_for_consumer_and_provider)
+              subject
             end
 
             it "returns the overall latest verification" do

spec/lib/pact_broker/matrix/row_spec.rb

@@ -5,6 +5,27 @@ module Matrix
     describe Row do
       let(:td) { TestDataBuilder.new }
 
+      describe "latest_verification_for_consumer_and_provider" do
+        before do
+          td.create_pact_with_hierarchy("Foo", "1", "Bar")
+            .create_verification(provider_version: "9")
+            .create_consumer_version("2")
+            .create_consumer_version_tag("prod")
+            .create_pact
+            .create_verification(provider_version: "10")
+            .create_consumer("Wiffle")
+            .create_consumer_version("4")
+            .create_pact
+            .create_verification(provider_version: "11")
+        end
+
+        subject { Row.where(consumer_name: "Foo", provider_name: "Bar").all.collect(&:latest_verification_for_consumer_and_provider) }
+
+        it "returns the latest verification for the consumer and provider" do
+          expect(subject.collect(&:provider_version_number)).to eq ["10", "10"]
+        end
+      end
+
       describe "refresh", migration: true do
         let(:td) { TestDataBuilder.new(auto_refresh_matrix: false) }