feat(index): optimise query, feature toggled

Author: bethesque

DEVELOPER_DOCUMENTATION.md

@@ -106,8 +106,19 @@ pact publication resource will be created, but it will reuse the existing pact v
     -> versions
   -> latest_tagged_pact_consumer_version_orders
   -> latest_pact_publications_by_consumer_versions
+
+= head_pact_publications
+  -> latest_pact_publications
+    -> latest_pact_publication_ids_for_consumer_versions
+  -> latest_tagged_pact_publications
+    -> latest_pact_publications_by_consumer_versions (optimised for pp_ids)
+    -> latest_tagged_pact_consumer_version_orders (optimised for pp_ids)
+
 ```
 
+
+
+
 ### Useful to know stuff
 
 * The supported database types are Postgres (recommended), MySQL (sigh) and Sqlite (just for testing, not recommended for production). Check the travis.yml file for the supported database versions.

lib/pact_broker/domain/index_item.rb

@@ -60,6 +60,10 @@ def consumer_version_number
         @latest_pact.consumer_version_number
       end
 
+      def consumer_version_order
+        consumer_version.order
+      end
+
       def consumer_version
         @latest_pact.consumer_version
       end

lib/pact_broker/index/service.rb

@@ -3,16 +3,24 @@
 require 'pact_broker/domain/index_item'
 require 'pact_broker/matrix/head_row'
 require 'pact_broker/matrix/aggregated_row'
+require 'pact_broker/repositories/helpers'
 
 module PactBroker
-
   module Index
     class Service
-
       extend PactBroker::Repositories
       extend PactBroker::Services
       extend PactBroker::Logging
 
+      COLS = [:id, :consumer_name, :provider_name, :consumer_version_order]
+      LATEST_PPS = Sequel::Model.db[:latest_pact_publications].select(*COLS)
+      LATEST_TAGGED_PPS = Sequel::Model.db[:latest_tagged_pact_publications].select(*COLS)
+      HEAD_PP_ORDER_COLUMNS = [
+          Sequel.asc(Sequel.function(:lower, :consumer_name)),
+          Sequel.desc(:consumer_version_order),
+          Sequel.asc(Sequel.function(:lower, :provider_name))
+        ].freeze
+
       # This method provides data for both the OSS server side rendered index (with and without tags)
       # and the Pactflow UI. It really needs to be broken into to separate methods, as it's getting too messy
       # supporting both
@@ -46,10 +54,7 @@ def self.find_index_items_original options = {}
         end
         rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) }
 
-
-
         rows.sort.collect do | row |
-          # TODO simplify. Do we really need 3 layers of abstraction?
           PactBroker::Domain::IndexItem.create(
             row.consumer,
             row.provider,
@@ -65,31 +70,14 @@ def self.find_index_items_original options = {}
       end
 
       def self.find_index_items_optimised options = {}
-        pact_publication_ids = nil
-        latest_verifications_for_cv_tags = nil
-
-        if !options[:tags]
-          # server side rendered index page without tags
-          pact_publication_ids = latest_pact_publications.select(:id)
-        else
-          # server side rendered index page with tags=true or tags[]=a&tags=[]b
-          if options[:tags].is_a?(Array)
-            # TODO test for this
-            pact_publication_ids = head_pact_publications_ids_for_tags(options[:tags])
-            latest_verifications_for_cv_tags = PactBroker::Verifications::LatestVerificationForConsumerVersionTag
-                                                    .eager(:provider_version)
-                                                    .where(consumer_version_tag_name: options[:tags]).all
-          else
-            pact_publication_ids = head_pact_publications_ids
-            latest_verifications_for_cv_tags = PactBroker::Verifications::LatestVerificationForConsumerVersionTag.eager(:provider_version).all
-          end
-        end
-
+        latest_verifications_for_cv_tags = latest_verifications_for_consumer_version_tags(options)
         latest_pact_publication_ids = latest_pact_publications.select(:id).all.collect{ |h| h[:id] }
 
         # We only need to know if a webhook exists for an integration, not what its properties are
         webhooks = PactBroker::Webhooks::Webhook.select(:consumer_id, :provider_id).distinct.all
 
+        pact_publication_ids = head_pact_publication_ids(options)
+
         pact_publications = PactBroker::Pacts::PactPublication
           .where(id: pact_publication_ids)
           .select_all_qualified
@@ -102,7 +90,6 @@ def self.find_index_items_optimised options = {}
           .eager(:head_pact_tags)
 
         pact_publications.all.collect do | pact_publication |
-
           is_overall_latest_for_integration = latest_pact_publication_ids.include?(pact_publication.id)
           latest_verification = latest_verification_for_pseudo_branch(pact_publication, is_overall_latest_for_integration, latest_verifications_for_cv_tags, options[:tags])
           webhook = webhooks.find{ |webhook| webhook.is_for?(pact_publication.integration) }
@@ -148,47 +135,83 @@ def self.consumer_version_tags(pact_publication, tags_option)
       end
 
       def self.find_index_items_for_api(consumer_name: nil, provider_name: nil, **ignored)
-        rows = PactBroker::Matrix::HeadRow
-          .eager(:consumer_version_tags)
-          .eager(:provider_version_tags)
+        latest_pact_publication_ids = latest_pact_publications.select(:id).all.collect{ |h| h[:id] }
+        pact_publication_ids = head_pact_publication_ids(consumer_name: consumer_name, provider_name: provider_name, tags: true)
+
+        pact_publications = PactBroker::Pacts::PactPublication
+          .where(id: pact_publication_ids)
           .select_all_qualified
+          .eager(:consumer)
+          .eager(:provider)
+          .eager(:pact_version)
+          .eager(:consumer_version)
+          .eager(latest_verification: { provider_version: :tags_with_latest_flag })
+          .eager(:head_pact_tags)
 
-        rows = rows.consumer(consumer_name) if consumer_name
-        rows = rows.provider(provider_name) if provider_name
 
-        rows = rows.all.group_by(&:pact_publication_id).values.collect{ | rows| Matrix::AggregatedRow.new(rows) }
+        pact_publications.all.collect do | pact_publication |
+
+          is_overall_latest_for_integration = latest_pact_publication_ids.include?(pact_publication.id)
 
-        rows.sort.collect do | row |
-          # TODO separate this model from IndexItem
-          # webhook status not currently displayed in Pactflow UI, so don't query for it.
           PactBroker::Domain::IndexItem.create(
-            row.consumer,
-            row.provider,
-            row.pact,
-            row.overall_latest?,
-            row.latest_verification_for_pact_version,
+            pact_publication.consumer,
+            pact_publication.provider,
+            pact_publication.to_domain_lightweight,
+            is_overall_latest_for_integration,
+            pact_publication.latest_verification,
             [],
             [],
-            row.consumer_head_tag_names,
-            row.provider_version_tags.select(&:latest?)
+            pact_publication.head_pact_tags.collect(&:name),
+            pact_publication.latest_verification ? pact_publication.latest_verification.provider_version.tags_with_latest_flag.select(&:latest?) : []
           )
-        end
+        end.sort
       end
 
       def self.latest_pact_publications
         db[:latest_pact_publications]
       end
 
-      def self.head_pact_publications_ids
-        db[:head_pact_tags].select(Sequel[:pact_publication_id].as(:id)).union(db[:latest_pact_publications].select(:id)).limit(500)
+      def self.db
+        PactBroker::Pacts::PactPublication.db
       end
 
-      def self.head_pact_publications_ids_for_tags(tag_names)
-        db[:head_pact_tags].select(Sequel[:pact_publication_id].as(:id)).where(name: tag_names).union(db[:latest_pact_publications].select(:id)).limit(500)
+      def self.head_pact_publication_ids(options = {})
+        query = if options[:tags].is_a?(Array)
+          LATEST_PPS.union(LATEST_TAGGED_PPS.where(tag_name: options[:tags]))
+        elsif options[:tags]
+          LATEST_PPS.union(LATEST_TAGGED_PPS)
+        else
+          LATEST_PPS
+        end
+
+        if options[:consumer_name]
+          query = query.where(PactBroker::Repositories::Helpers.name_like(:consumer_name, options[:consumer_name]))
+        end
+
+        if options[:provider_name]
+          query = query.where(PactBroker::Repositories::Helpers.name_like(:provider_name, options[:provider_name]))
+        end
+
+        query.order(*HEAD_PP_ORDER_COLUMNS)
+          .limit(options[:limit] || 50)
+          .offset(options[:offset] || 0)
+          .select(:id)
       end
 
-      def self.db
-        PactBroker::Pacts::PactPublication.db
+      def self.latest_verifications_for_consumer_version_tags(options)
+        # server side rendered index page with tags[]=a&tags=[]b
+        if options[:tags].is_a?(Array)
+          PactBroker::Verifications::LatestVerificationForConsumerVersionTag
+            .eager(:provider_version)
+            .where(consumer_version_tag_name: options[:tags])
+            .all
+        elsif options[:tags] # server side rendered index page with tags=true
+          PactBroker::Verifications::LatestVerificationForConsumerVersionTag
+            .eager(:provider_version)
+            .all
+        else
+          nil # should not be used
+        end
       end
     end
   end

lib/pact_broker/ui/controllers/index.rb

@@ -14,7 +14,7 @@ class Index < Base
           if params[:tags]
             tags = params[:tags] == 'true' ? true : [*params[:tags]].compact
           end
-          options = { tags: tags }
+          options = { tags: tags, limit: params[:limit]&.to_i, offset: params[:offset]&.to_i }
           options[:optimised] = true if params[:optimised] == 'true'
           view_model = ViewDomain::IndexItems.new(index_service.find_index_items(options))
           page = tags ? :'index/show-with-tags' : :'index/show'

lib/pact_broker/ui/view_models/index_item.rb

@@ -27,6 +27,10 @@ def consumer_version_number
           PactBroker::Versions::AbbreviateNumber.call(@relationship.consumer_version_number)
         end
 
+        def consumer_version_order
+          @relationship.consumer_version_order
+        end
+
         def provider_version_number
           PactBroker::Versions::AbbreviateNumber.call(@relationship.provider_version_number)
         end
@@ -164,7 +168,9 @@ def verification_tooltip
         def <=> other
           comp = consumer_name.downcase <=> other.consumer_name.downcase
           return comp unless comp == 0
-          provider_name.downcase <=> other.provider_name.downcase
+          comp = provider_name.downcase <=> other.provider_name.downcase
+          return comp unless comp == 0
+          other.consumer_version_order <=> consumer_version_order
         end
 
         def short_version_number version_number

lib/pact_broker/ui/views/index/show-with-tags.haml

@@ -38,7 +38,7 @@
             %td.consumer
               %a{:href => index_item.consumer_group_url }
                 = escape_html(index_item.consumer_name)
-            %td.consumer-version-number
+            %td.consumer-version-number{"data-text": index_item.consumer_version_order}
               %div.clippable
                 = escape_html(index_item.consumer_version_number)
                 - if index_item.consumer_version_number

spec/lib/pact_broker/index/service_spec.rb

@@ -4,7 +4,6 @@
 require 'pact_broker/domain/pact'
 
 module PactBroker
-
   module Index
     describe Service do
       let(:td) { TestDataBuilder.new }

spec/lib/pact_broker/ui/controllers/index_spec.rb

@@ -1,7 +1,6 @@
-require 'spec_helper'
-require 'pact_broker/ui/controllers/index'
-
 require 'rack/test'
+require 'pact_broker/ui/controllers/index'
+require 'pact_broker/index/service'
 
 module PactBroker
   module UI
@@ -37,7 +36,7 @@ module Controllers
               end
 
               it "passes tags: true to the IndexService" do
-                expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: true)
+                expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: true, limit: nil, offset: nil)
                 get "/", { tags: 'true' }
               end
             end
@@ -48,7 +47,7 @@ module Controllers
               end
 
               it "passes tags: ['prod'] to the IndexService" do
-                expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"])
+                expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"], limit: nil, offset: nil)
                 get "/", { tags: ["prod"] }
               end
             end
@@ -59,7 +58,7 @@ module Controllers
               end
 
               it "passes tags: ['prod'] to the IndexService" do
-                expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"])
+                expect(PactBroker::Index::Service).to receive(:find_index_items).with(tags: ["prod"], limit: nil, offset: nil)
                 get "/", { tags: "prod" }
               end
             end

spec/lib/pact_broker/ui/view_models/index_items_spec.rb

@@ -6,10 +6,10 @@ module UI
     module ViewDomain
       describe IndexItems do
 
-        let(:relationship_model_4) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "X") }
-        let(:relationship_model_2) { double("PactBroker::Domain::IndexItem", consumer_name: "a", provider_name: "y") }
-        let(:relationship_model_3) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "Z") }
-        let(:relationship_model_1) { double("PactBroker::Domain::IndexItem", consumer_name: "C", provider_name: "A") }
+        let(:relationship_model_4) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "X", consumer_version_order: 1) }
+        let(:relationship_model_2) { double("PactBroker::Domain::IndexItem", consumer_name: "a", provider_name: "y", consumer_version_order: 2) }
+        let(:relationship_model_3) { double("PactBroker::Domain::IndexItem", consumer_name: "A", provider_name: "Z", consumer_version_order: 3) }
+        let(:relationship_model_1) { double("PactBroker::Domain::IndexItem", consumer_name: "C", provider_name: "A", consumer_version_order: 4) }
 
         subject { IndexItems.new([relationship_model_1, relationship_model_3, relationship_model_4, relationship_model_2]) }