feat(matrix): speed up query for UI by reducing the number of joins and removing unncessary criteria (pact-foundation#332)

* Add a created_at column to the latest pacts/latest verifications tables
* Use the created_at columns newly created on latest_pact_publication_ids_for_consumer_versions and latest_verification_id_for_pact_version_and_provider_version to avoid having to join in the versions tables to order the matrix query rows
* speed up matrix UI page by not joining verifications table when no version is specified
* speed up matrix query by removing unncessary null check for provider versions when we are only querying by pacticipant names

Author: bethesque

db/migrations/20200318_add_created_at_to_latest_pact_publications.rb

@@ -0,0 +1,9 @@
+Sequel.migration do
+  change do
+    # TODO
+    # alter_table(:latest_pact_publication_ids_for_consumer_versions) do
+    #   set_column_not_null(:created_at)
+    # end
+    add_column(:latest_pact_publication_ids_for_consumer_versions, :created_at, DateTime)
+  end
+end

db/migrations/20200319_add_created_at_to_latest_verifications.rb

@@ -0,0 +1,9 @@
+Sequel.migration do
+  change do
+    # TODO
+    # alter_table(:latest_verification_id_for_pact_version_and_provider_version) do
+    #   set_column_not_null(:created_at)
+    # end
+    add_column(:latest_verification_id_for_pact_version_and_provider_version, :created_at, DateTime)
+  end
+end

lib/pact_broker/db/data_migrations/set_created_at_for_latest_pact_publications.rb

@@ -0,0 +1,25 @@
+require 'pact_broker/db/data_migrations/helpers'
+
+module PactBroker
+  module DB
+    module DataMigrations
+      class SetCreatedAtForLatestPactPublications
+        def self.call connection
+          # pact ordering goes by creation date of the consumer version
+          connection[:latest_pact_publication_ids_for_consumer_versions]
+          query = "UPDATE latest_pact_publication_ids_for_consumer_versions
+                  SET created_at = (SELECT created_at
+                    FROM versions
+                    WHERE id = latest_pact_publication_ids_for_consumer_versions.consumer_version_id)
+                  WHERE created_at IS NULL"
+          connection.run(query)
+        end
+
+        def self.columns_exist?(connection)
+          column_exists?(connection, :latest_pact_publication_ids_for_consumer_versions, :created_at) &&
+            column_exists?(connection, :pact_publications, :created_at)
+        end
+      end
+    end
+  end
+end

lib/pact_broker/db/data_migrations/set_created_at_for_latest_verifications.rb

@@ -0,0 +1,24 @@
+require 'pact_broker/db/data_migrations/helpers'
+
+module PactBroker
+  module DB
+    module DataMigrations
+      class SetCreatedAtForLatestVerifications
+        def self.call connection
+          connection[:latest_verification_id_for_pact_version_and_provider_version]
+          query = "UPDATE latest_verification_id_for_pact_version_and_provider_version
+                  SET created_at = (SELECT created_at
+                    FROM verifications
+                    WHERE id = latest_verification_id_for_pact_version_and_provider_version.verification_id)
+                  WHERE created_at is null"
+          connection.run(query)
+        end
+
+        def self.columns_exist?(connection)
+          column_exists?(connection, :latest_verification_id_for_pact_version_and_provider_version, :created_at) &&
+            column_exists?(connection, :verifications, :created_at)
+        end
+      end
+    end
+  end
+end

lib/pact_broker/db/migrate_data.rb

@@ -19,6 +19,8 @@ def self.call database_connection, options = {}
         DataMigrations::SetLatestVersionSequenceValue.call(database_connection)
         DataMigrations::SetWebhooksEnabled.call(database_connection)
         DataMigrations::DeleteDeprecatedWebhookExecutions.call(database_connection)
+        DataMigrations::SetCreatedAtForLatestPactPublications.call(database_connection)
+        DataMigrations::SetCreatedAtForLatestVerifications.call(database_connection)
       end
     end
   end

lib/pact_broker/matrix/every_row.rb

@@ -8,16 +8,22 @@ class EveryRow < PactBroker::Matrix::QuickRow
       P_V_JOIN = { Sequel[:p][:pact_version_id] => Sequel[:v][:pact_version_id] }
 
       PACT_COLUMNS = [
+        Sequel[:p][:consumer_id],
+        Sequel[:p][:provider_id],
+        Sequel[:p][:consumer_version_id],
         Sequel[:p][:id].as(:pact_publication_id),
         Sequel[:p][:pact_version_id],
-        Sequel[:p][:revision_number].as(:pact_revision_number)
+        Sequel[:p][:revision_number].as(:pact_revision_number),
+        Sequel[:p][:created_at].as(:consumer_version_created_at),
+        Sequel[:p][:id].as(:pact_order)
       ]
       VERIFICATION_COLUMNS = [
-        Sequel[:v][:id].as(:verification_id)
+        Sequel[:v][:id].as(:verification_id),
+        Sequel[:v][:provider_version_id],
+        Sequel[:v][:created_at].as(:provider_version_created_at)
       ]
 
-      ALL_COLUMNS = [LAST_ACTION_DATE] + CONSUMER_COLUMNS + CONSUMER_VERSION_COLUMNS + PACT_COLUMNS +
-                    PROVIDER_COLUMNS + PROVIDER_VERSION_COLUMNS + VERIFICATION_COLUMNS
+      ALL_COLUMNS = PACT_COLUMNS + VERIFICATION_COLUMNS
 
       SELECT_ALL_COLUMN_ARGS = [:select_all_columns] + ALL_COLUMNS
       dataset_module do
@@ -29,7 +35,7 @@ def join_verifications
 
         def verifications_for(query_ids)
           db[:verifications]
-            .select(:id, :pact_version_id, :provider_id, :provider_version_id)
+            .select(:id, :pact_version_id, :provider_id, :provider_version_id, :created_at)
             .where {
               Sequel.&(
                 QueryBuilder.consumer_in_pacticipant_ids(query_ids),

lib/pact_broker/matrix/parse_query.rb

@@ -26,11 +26,17 @@ def self.call query
         if params.key?('latestby') && params['latestby'] != ''
           options[:latestby] = params['latestby']
         end
+
+        if params.key?('days') && params['days'] != ''
+          options[:days] = params['days'].to_i
+        end
+
         if params.key?('limit') && params['limit'] != ''
           options[:limit] = params['limit']
         else
           options[:limit] = "100"
         end
+
         if params.key?('latest') && params['latest'] != ''
           options[:latest] = params['latest'] == 'true'
         end

lib/pact_broker/matrix/query_builder.rb

@@ -1,24 +1,36 @@
 module PactBroker
   module Matrix
     class QueryBuilder
-      def self.provider_or_provider_version_matches(query_ids, verifications_qualifier = nil)
-        Sequel.|(*provider_or_provider_version_criteria(query_ids, verifications_qualifier))
+      def self.provider_or_provider_version_matches(query_ids, provider_version_qualifier = nil, provider_qualifier = nil)
+        Sequel.|(*provider_or_provider_version_criteria(query_ids, provider_version_qualifier, provider_qualifier))
       end
 
-      def self.provider_or_provider_version_matches_or_pact_unverified(query_ids, verifications_qualifier = nil)
-        ors = provider_or_provider_version_criteria(query_ids, verifications_qualifier)
-
-        ors << {
-          qualify(:p, :provider_id) => query_ids.all_pacticipant_ids,
-          qualify(verifications_qualifier, :provider_version_id) => nil
+      def self.provider_matches(query_ids, qualifier)
+        {
+          qualify(qualifier, :provider_id) => query_ids.pacticipant_ids,
         }
+      end
+
+      def self.provider_or_provider_version_matches_or_pact_unverified(query_ids, provider_version_qualifier = nil, provider_qualifier = nil)
+        ors = provider_or_provider_version_criteria(query_ids, provider_version_qualifier, provider_qualifier)
+
+        # If we have specified any versions, then we need to add an
+        # "OR (provider matches these IDs and provider version is null)"
+        # so that we get a line with blank verification details.
+        if query_ids.pacticipant_version_ids.any?
+          ors << {
+            qualify(provider_qualifier, :provider_id) => query_ids.all_pacticipant_ids,
+            qualify(provider_version_qualifier, :provider_version_id) => nil
+          }
+        end
+
         Sequel.|(*ors)
       end
 
-      def self.provider_or_provider_version_criteria(query_ids, qualifier = nil)
+      def self.provider_or_provider_version_criteria(query_ids, provider_version_qualifier = nil, provider_qualifier = nil)
         ors = []
-        ors << { qualify(qualifier, :provider_version_id) => query_ids.pacticipant_version_ids } if query_ids.pacticipant_version_ids.any?
-        ors << { qualify(qualifier, :provider_id) => query_ids.pacticipant_ids } if query_ids.pacticipant_ids.any?
+        ors << { qualify(provider_version_qualifier, :provider_version_id) => query_ids.pacticipant_version_ids } if query_ids.pacticipant_version_ids.any?
+        ors << { qualify(provider_qualifier, :provider_id) => query_ids.pacticipant_ids } if query_ids.pacticipant_ids.any?
         ors
       end