feat: include more columns in latest_verifications_for_consumer_version_tags to avoid having to do extra queries for pact_versions and provider_versions

Author: bethesque

db/migrations/20180523_create_latest_verifications_for_consumer_version_tags.rb

@@ -2,7 +2,11 @@
   up do
     # The latest verification id for each consumer version tag
     create_view(:latest_verifications_ids_for_consumer_version_tags,
-      "select pv.pacticipant_id as provider_id, lpp.consumer_id, t.name as consumer_version_tag_name, max(v.id) as latest_verification_id
+      "select
+        pv.pacticipant_id as provider_id,
+        lpp.consumer_id,
+        t.name as consumer_version_tag_name,
+        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
@@ -12,12 +16,31 @@
         on v.provider_version_id = pv.id
       group by pv.pacticipant_id, lpp.consumer_id, t.name")
 
-    # The latest verification for each consumer version tag
-    create_view(:latest_verifications_for_consumer_version_tags,
-      "select v.*, lv.provider_id, lv.consumer_id, lv.consumer_version_tag_name
-      from verifications v
-      join latest_verifications_ids_for_consumer_version_tags lv
-        on lv.latest_verification_id = v.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[:lv][:consumer_version_tag_name],
+        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_verifications_ids_for_consumer_version_tags,
+        {
+          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_version_tags, latest_verifications)
   end
 
   down do

lib/pact_broker/index/service.rb

@@ -19,7 +19,7 @@ def self.find_index_items options = {}
           .eager(:latest_triggered_webhooks)
           .eager(:webhooks)
           .order(:consumer_name, :provider_name)
-          .eager(:verification)
+          #.eager(verification: [:provider_version, :pact_version])
 
         if !options[:tags]
           rows = rows.where(consumer_version_tag_name: nil)
@@ -34,12 +34,13 @@ def self.find_index_items options = {}
         rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) }
 
         rows.sort.collect do | row |
+          # TODO simplify
           PactBroker::Domain::IndexItem.create(
             row.consumer,
             row.provider,
             row.pact,
             row.overall_latest?,
-            options[:tags] ? row.latest_verification : row.verification,
+            row.latest_verification,
             row.webhooks,
             row.latest_triggered_webhooks,
             options[:tags] ? row.consumer_head_tag_names : [],

lib/pact_broker/matrix/aggregated_row.rb

@@ -2,7 +2,7 @@
 
 # A collection of matrix rows with the same pact publication id
 # It's basically a normalised view of a denormalised view :(
-
+# A pact publication may be the overall latest, and/or the latest for a tag
 module PactBroker
   module Matrix
     class AggregatedRow
@@ -13,7 +13,6 @@ class AggregatedRow
       delegate [:pact, :pact_version, :pact_revision_number, :webhooks, :latest_triggered_webhooks, :'<=>'] => :first_row
       delegate [:verification_id, :verification] => :first_row
 
-
       def initialize matrix_rows
         @matrix_rows = matrix_rows
         @first_row = matrix_rows.first
@@ -23,20 +22,26 @@ def overall_latest?
         !!matrix_rows.find{ | row| row.consumer_version_tag_name.nil? }
       end
 
+      # If this comes back nil, it won't be "cached", but it's a reasonably
+      # quick query, so it's probably ok.
       def latest_verification
         @latest_verification ||= begin
           verification = matrix_rows.collect do | row|
-              row.verification || row.latest_verification_for_consumer_version_tag
-            end.compact.sort{ |v1, v2| v1.id <=> v2.id}.last
+              row.verification || latest_verification_for_consumer_version_tag(row)
+            end.compact.sort{ |v1, v2| v1.id <=> v2.id }.last
 
           if !verification && overall_latest?
-            PactBroker::Verifications::Repository.new.find_latest_verification_for(consumer_name, provider_name)
+            overall_latest_verification
           else
             verification
           end
         end
       end
 
+      # The list of tag names for which this pact publication is the most recent with that tag
+      # There could, however, be a later consumer version that does't have a pact (perhaps because it was deleted)
+      # that has the same tag.
+      # TODO show a warning when the data is "corrupted" as above.
       def consumer_head_tag_names
         matrix_rows.collect(&:consumer_version_tag_name).compact
       end
@@ -45,6 +50,18 @@ def consumer_head_tag_names
 
       attr_reader :matrix_rows
 
+      def latest_verification_for_consumer_version_tag row
+        row.latest_verification_for_consumer_version_tag if row.consumer_version_tag_name
+      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)
+      end
+
       def first_row
         @first_row
       end

lib/pact_broker/matrix/head_row.rb

@@ -21,6 +21,7 @@ class HeadRow < Row
         tag_to_row = eo_opts[:rows].each_with_object({}) { | row, map | map[[row.provider_id, row.consumer_id, row.consumer_version_tag_name]] = row }
         eo_opts[:rows].each{|row| row.associations[:latest_verification_for_consumer_version_tag] = nil}
 
+        # Need the all then the each to ensure the eager loading works
         PactBroker::Verifications::LatestVerificationForConsumerVersionTag.each do | verification |
           key = [verification.provider_id, verification.consumer_id, verification.consumer_version_tag_name]
           if tag_to_row[key]

lib/pact_broker/matrix/row.rb

@@ -16,7 +16,6 @@ class Row < Sequel::Model(:materialized_matrix)
       associate(:one_to_many, :webhooks, :class => "PactBroker::Webhooks::Webhook", primary_key: [:consumer_id, :provider_id], key: [:consumer_id, :provider_id])
       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)
-      associate(:many_to_one, :verification, :class => "PactBroker::Domain::Verification", primary_key: :id, key: :verification_id)
 
       dataset_module do
         include PactBroker::Repositories::Helpers
@@ -114,20 +113,6 @@ def [] key
         end
       end
 
-      # tags for which this pact publication is the latest of that tag
-      # this is set in the code rather than the database
-      def consumer_head_tag_names
-        @consumer_head_tag_names ||= []
-      end
-
-      def consumer_head_tag_names= consumer_head_tag_names
-        @consumer_head_tag_names = consumer_head_tag_names
-      end
-
-      # def latest_triggered_webhooks
-      #   @latest_triggered_webhooks ||= []
-      # end
-
       def summary
         "#{consumer_name}#{consumer_version_number} #{provider_name}#{provider_version_number || '?'} (r#{pact_revision_number}n#{verification_number || '?'})"
       end
@@ -141,17 +126,18 @@ def provider
       end
 
       def consumer_version
-        @consumer_version ||= OpenStruct.new(number: consumer_version_number, id: consumer_version_id, pacticipant: consumer)
+        @consumer_version ||= OpenStruct.new(number: consumer_version_number, order: consumer_version_order, id: consumer_version_id, pacticipant: consumer)
       end
 
       def provider_version
         if provider_version_number
-          @provider_version ||= OpenStruct.new(number: provider_version_number, id: provider_version_id, pacticipant: provider)
+          @provider_version ||= OpenStruct.new(number: provider_version_number, order: provider_version_order, id: provider_version_id, pacticipant: provider)
         end
       end
 
       def pact
-        @pact ||= OpenStruct.new(consumer: consumer,
+        @pact ||= OpenStruct.new(
+          consumer: consumer,
           provider: provider,
           consumer_version: consumer_version,
           consumer_version_number: consumer_version_number,
@@ -161,7 +147,7 @@ def pact
         )
       end
 
-      def latest_verification
+      def verification
         if verification_executed_at
           @latest_verification ||= OpenStruct.new(
             id: verification_id,
@@ -170,10 +156,12 @@ def latest_verification
             execution_date: verification_executed_at,
             created_at: verification_executed_at,
             provider_version_number: provider_version_number,
+            provider_version_order: provider_version_order,
             build_url: verification_build_url,
             provider_version: provider_version,
             consumer_name: consumer_name,
-            provider_name: provider_name
+            provider_name: provider_name,
+            pact_version_sha: pact_version_sha
           )
         end
       end
@@ -190,7 +178,6 @@ def <=> other
         ]
 
         comparisons.find{|c| c != 0 } || 0
-
       end
 
       def compare_name_asc name1, name2

lib/pact_broker/verifications/latest_verification_for_consumer_version_tag.rb

@@ -4,6 +4,20 @@ module PactBroker
   module Verifications
     class LatestVerificationForConsumerVersionTag < PactBroker::Domain::Verification
       set_dataset(:latest_verifications_for_consumer_version_tags)
+
+      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
+
+      def provider_version_number
+        values[:provider_version_number]
+      end
+
+      def provider_version_order
+        values[:provider_version_order]
+      end
     end
   end
-end
+end