refactor: use dynamic query rather than view for matrix queries that …

…return *all* rows

lib/pact_broker/matrix/every_row.rb

@@ -0,0 +1,43 @@
+require 'pact_broker/matrix/quick_row'
+
+module PactBroker
+  module Matrix
+    class EveryRow < PactBroker::Matrix::QuickRow
+      set_dataset(Sequel.as(:pact_publications, :p))
+
+      P_V_JOIN = { Sequel[:p][:pact_version_id] => Sequel[:v][:pact_version_id] }
+
+      PACT_COLUMNS = [
+        Sequel[:p][:id].as(:pact_publication_id),
+        Sequel[:p][:pact_version_id],
+        Sequel[:p][:revision_number].as(:pact_revision_number)
+      ]
+      VERIFICATION_COLUMNS = [
+        Sequel[:v][:id].as(:verification_id)
+      ]
+
+      ALL_COLUMNS = CONSUMER_COLUMNS + CONSUMER_VERSION_COLUMNS + PACT_COLUMNS +
+                    PROVIDER_COLUMNS + PROVIDER_VERSION_COLUMNS + VERIFICATION_COLUMNS
+
+      SELECT_ALL_COLUMN_ARGS = [:select_all_columns] + ALL_COLUMNS
+      dataset_module do
+        select *SELECT_ALL_COLUMN_ARGS
+
+        def join_verifications
+          left_outer_join(:verifications, P_V_JOIN, { table_alias: :v } )
+        end
+
+        def verifications_for(query_ids)
+          db[:verifications]
+            .select(:id, :pact_version_id, :provider_id, :provider_version_id)
+            .where {
+              Sequel.&(
+                QueryBuilder.consumer_in_pacticipant_ids(query_ids),
+                QueryBuilder.provider_or_provider_version_matches(query_ids)
+              )
+            }
+        end
+      end
+    end
+  end
+end

lib/pact_broker/matrix/query_builder.rb

@@ -9,7 +9,7 @@ def self.provider_or_provider_version_matches_or_pact_unverified(query_ids, qual
         ors = provider_or_provider_version_criteria(query_ids, qualifier)
 
         ors << {
-          qualify(:lp, :provider_id) => query_ids.all_pacticipant_ids,
+          qualify(:p, :provider_id) => query_ids.all_pacticipant_ids,
           qualify(qualifier, :provider_version_id) => nil
         }
         Sequel.|(*ors)
@@ -53,13 +53,13 @@ def self.either_consumer_or_provider_was_specified_in_query(query_ids, qualifier
       def self.consumer_or_consumer_version_or_provider_or_provider_or_provider_version_match(query_ids)
         ors = if query_ids.pacticipant_version_id
           [
-            { Sequel[:lp][:consumer_version_id] => query_ids.pacticipant_version_id },
-            { Sequel[:lv][:provider_version_id] => query_ids.pacticipant_version_id }
+            { Sequel[:p][:consumer_version_id] => query_ids.pacticipant_version_id },
+            { Sequel[:v][:provider_version_id] => query_ids.pacticipant_version_id }
           ]
         else
           [
-            { Sequel[:lp][:consumer_id] => query_ids.pacticipant_id },
-            { Sequel[:lp][:provider_id] => query_ids.pacticipant_id }
+            { Sequel[:p][:consumer_id] => query_ids.pacticipant_id },
+            { Sequel[:p][:provider_id] => query_ids.pacticipant_id }
           ]
         end
 

lib/pact_broker/matrix/quick_row.rb

@@ -20,52 +20,52 @@
 
 module PactBroker
   module Matrix
-    class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consumer_versions, :lp))
+    class QuickRow < Sequel::Model(Sequel.as(:latest_pact_publication_ids_for_consumer_versions, :p))
 
       # Tables
       LV = :latest_verification_id_for_pact_version_and_provider_version
       LP = :latest_pact_publication_ids_for_consumer_versions
 
       # Joins
-      LP_LV_JOIN = { Sequel[:lp][:pact_version_id] => Sequel[:lv][:pact_version_id] }
-      CONSUMER_VERSION_JOIN = { Sequel[:lp][:consumer_version_id] => Sequel[:cv][:id] }
-      PROVIDER_VERSION_JOIN = { Sequel[:lv][:provider_version_id] => Sequel[:pv][:id] }
+      LP_LV_JOIN = { Sequel[:p][:pact_version_id] => Sequel[:v][:pact_version_id] }
+      CONSUMER_VERSION_JOIN = { Sequel[:p][:consumer_version_id] => Sequel[:cv][:id] }
+      PROVIDER_VERSION_JOIN = { Sequel[:v][:provider_version_id] => Sequel[:pv][:id] }
 
       # Not sure why we're eager loading some things and including others in the base query :shrug:
 
       # Columns
       CONSUMER_COLUMNS = [
-        Sequel[:lp][:consumer_id],
+        Sequel[:p][:consumer_id],
         Sequel[:consumers][:name].as(:consumer_name)
       ]
       PROVIDER_COLUMNS = [
-        Sequel[:lp][:provider_id],
+        Sequel[:p][:provider_id],
         Sequel[:providers][:name].as(:provider_name)
       ]
       CONSUMER_VERSION_COLUMNS = [
-        Sequel[:lp][:consumer_version_id],
+        Sequel[:p][:consumer_version_id],
         Sequel[:cv][:number].as(:consumer_version_number),
         Sequel[:cv][:order].as(:consumer_version_order)
       ]
       PROVIDER_VERSION_COLUMNS = [
-        Sequel[:lv][:provider_version_id],
+        Sequel[:v][:provider_version_id],
         Sequel[:pv][:number].as(:provider_version_number),
         Sequel[:pv][:order].as(:provider_version_order)
       ]
       PACT_COLUMNS = [
-        Sequel[:lp][:pact_publication_id],
-        Sequel[:lp][:pact_version_id]
+        Sequel[:p][:pact_publication_id],
+        Sequel[:p][:pact_version_id]
       ]
       VERIFICATION_COLUMNS = [
-        Sequel[:lv][:verification_id]
+        Sequel[:v][:verification_id]
       ]
       ALL_COLUMNS = CONSUMER_COLUMNS + CONSUMER_VERSION_COLUMNS + PACT_COLUMNS +
                     PROVIDER_COLUMNS + PROVIDER_VERSION_COLUMNS + VERIFICATION_COLUMNS
 
 
       # cachable select arguments
       SELECT_ALL_COLUMN_ARGS = [:select_all_columns] + ALL_COLUMNS
-      SELECT_PACTICIPANT_IDS_ARGS = [:select_pacticipant_ids, Sequel[:lp][:consumer_id], Sequel[:lp][:provider_id]]
+      SELECT_PACTICIPANT_IDS_ARGS = [:select_pacticipant_ids, Sequel[:p][:consumer_id], Sequel[:p][:provider_id]]
 
       associate(:many_to_one, :pact_publication, :class => "PactBroker::Pacts::PactPublication", :key => :pact_publication_id, :primary_key => :id)
       associate(:many_to_one, :provider, :class => "PactBroker::Domain::Pacticipant", :key => :provider_id, :primary_key => :id)
@@ -157,15 +157,15 @@ def matching_multiple_selectors(selectors)
             .join_pacticipants_and_pacticipant_versions
             .where {
               Sequel.&(
-                QueryBuilder.consumer_or_consumer_version_matches(query_ids, :lp),
-                QueryBuilder.provider_or_provider_version_matches_or_pact_unverified(query_ids, :lv),
-                QueryBuilder.either_consumer_or_provider_was_specified_in_query(query_ids, :lp)
+                QueryBuilder.consumer_or_consumer_version_matches(query_ids, :p),
+                QueryBuilder.provider_or_provider_version_matches_or_pact_unverified(query_ids, :v),
+                QueryBuilder.either_consumer_or_provider_was_specified_in_query(query_ids, :p)
               )
             }
         end
 
         def join_verifications_for(query_ids)
-          left_outer_join(verifications_for(query_ids), LP_LV_JOIN, { table_alias: :lv } )
+          left_outer_join(verifications_for(query_ids), LP_LV_JOIN, { table_alias: :v } )
         end
 
         def verifications_for(query_ids)
@@ -186,15 +186,15 @@ def join_pacticipants_and_pacticipant_versions
             .join_provider_versions
         end
 
-        def join_consumers qualifier = :lp, table_alias = :consumers
+        def join_consumers qualifier = :p, table_alias = :consumers
           join(
             :pacticipants,
             { Sequel[qualifier][:consumer_id] => Sequel[table_alias][:id] },
             { table_alias: table_alias }
           )
         end
 
-        def join_providers qualifier = :lp, table_alias = :providers
+        def join_providers qualifier = :p, table_alias = :providers
           join(
             :pacticipants,
             { Sequel[qualifier][:provider_id] => Sequel[table_alias][:id] },
@@ -211,7 +211,7 @@ def join_provider_versions
         end
 
         def join_verifications
-          left_outer_join(LV, LP_LV_JOIN, { table_alias: :lv } )
+          left_outer_join(LV, LP_LV_JOIN, { table_alias: :v } )
         end
       end # end dataset_module
 
@@ -321,6 +321,10 @@ def provider_version_order
         return_or_raise_if_not_set(:provider_version_order)
       end
 
+      def has_verification?
+        !!verification_id
+      end
+
       # This model needs the verifications and pacticipants joined to it
       # before it can be used, as it's not a "real" model.
       def return_or_raise_if_not_set(key)

lib/pact_broker/matrix/repository.rb

@@ -1,6 +1,7 @@
 require 'pact_broker/repositories/helpers'
 require 'pact_broker/matrix/row'
 require 'pact_broker/matrix/quick_row'
+require 'pact_broker/matrix/every_row'
 require 'pact_broker/matrix/head_row'
 require 'pact_broker/error'
 require 'pact_broker/matrix/query_results'
@@ -118,7 +119,8 @@ def apply_latestby options, selectors, lines
       end
 
       def query_matrix selectors, options
-        query = options[:latestby] ? QuickRow.select_all_columns.eager_all_the_things : Row.select_all
+        query = options[:latestby] ? QuickRow : EveryRow
+        query = query.select_all_columns.eager_all_the_things
         query = query.matching_selectors(selectors)
         query = query.limit(options[:limit]) if options[:limit]
         query

spec/lib/pact_broker/matrix/every_row_spec.rb

@@ -0,0 +1,132 @@
+require 'pact_broker/matrix/every_row'
+require 'pact_broker/matrix/resolved_selector'
+
+module PactBroker
+  module Matrix
+    describe EveryRow do
+      let(:foo) { PactBroker::Domain::Pacticipant.where(name: "Foo").single_record }
+      let(:bar) { PactBroker::Domain::Pacticipant.where(name: "Bar").single_record }
+      let(:wiffle) { PactBroker::Domain::Pacticipant.where(name: "Wiffle").single_record }
+
+      describe "matching_selectors" do
+        before do
+          td.create_pact_with_verification("Foo", "1", "Bar", "2")
+            .create_consumer_version("2")
+            .create_pact
+            .create_provider("Wiffle")
+            .create_pact
+            .create_verification(provider_version: "5")
+        end
+
+        let(:selector_1) do
+          PactBroker::Matrix::ResolvedSelector.for_pacticipant(foo, :specified)
+        end
+
+        let(:selector_2) do
+          PactBroker::Matrix::ResolvedSelector.for_pacticipant(bar, :specified)
+        end
+
+        let(:selectors) { [selector_1, selector_2] }
+
+        subject { EveryRow.select_all_columns.matching_selectors(selectors).all }
+
+        let(:un_verified_row) { subject.find{ |r| r.provider_id == bar.id && !r.has_verification? } }
+        let(:verified_row) { subject.find{ |r| r.provider_id == bar.id && r.has_verification? } }
+
+        it "includes the verified and unverified rows" do
+          expect(subject.size).to eq 2
+          expect(un_verified_row).to_not be nil
+          expect(verified_row).to_not be nil
+        end
+      end
+
+      describe "eager_all_the_things" do
+        before do
+          td.create_pact_with_verification("Foo", "1", "Bar", "2")
+        end
+
+        subject do
+          EveryRow
+            .select_all_columns
+            .join_verifications
+            .join_pacticipants_and_pacticipant_versions
+            .eager_all_the_things
+            .all
+        end
+
+        it "can eager load all the things" do
+          expect(subject.first.provider_version).to_not be nil
+          expect(subject.first.provider_version_id).to_not be nil
+          expect(subject.first.consumer_version).to_not be nil
+          expect(subject.first.consumer_version_id).to_not be nil
+          expect(subject.first.provider_version_id).to_not be nil
+          expect(subject.first.consumer_version_id).to_not be nil
+          expect(subject.first.pact_publication_id).to_not be nil
+          expect(subject.first.pact_version_id).to_not be nil
+          expect(subject.first.verification_id).to_not be nil
+          expect(subject.first.provider).to_not be nil
+          expect(subject.first.consumer).to_not be nil
+          expect(subject.first.consumer_version).to_not be nil
+          expect(subject.first.provider_version).to_not be nil
+          expect(subject.first.pact_version).to_not be nil
+          expect(subject.first.verification).to_not be nil
+          expect(subject.first.pact_revision_number).to_not be nil
+          expect(subject.first.verification_number).to_not be nil
+        end
+      end
+
+
+      describe "join_verifications" do
+        before do
+          td.create_pact_with_verification("Foo", "1", "Bar", "2")
+            .create_provider("Wiffle")
+            .create_pact
+            .create_verification(provider_version: "5")
+        end
+
+        subject do
+          EveryRow
+            .select_all_columns
+            .join_verifications
+            .join_pacticipants_and_pacticipant_versions
+            .all
+        end
+
+        it "joins all the verifications" do
+          expect(subject.size).to eq 2
+          expect(subject.all?(&:has_verification?)).to be true
+        end
+      end
+
+      describe "join_verifications_for" do
+        before do
+          td.create_pact_with_verification("Foo", "1", "Bar", "2")
+            .create_provider("Wiffle")
+            .create_pact
+            .create_verification(provider_version: "5")
+        end
+
+        let(:query_ids) do
+          double('query_ids',
+            all_pacticipant_ids: [foo.id, bar.id],
+            pacticipant_version_ids: [],
+            pacticipant_ids: [foo.id, bar.id]
+          )
+        end
+
+        subject do
+          EveryRow
+            .select_all_columns
+            .join_verifications_for(query_ids)
+            .join_pacticipants_and_pacticipant_versions
+            .all
+        end
+
+        it "pre-filters the verifications before joining them" do
+          expect(subject.size).to eq 2
+          expect(subject.find{ |r| r.provider_id == wiffle.id && !r.has_verification? }).to_not be nil
+        end
+      end
+    end
+  end
+end

spec/lib/pact_broker/matrix/integration_spec.rb

@@ -107,8 +107,12 @@ module Matrix
             expect(subject.integrations.size).to eq 2
           end
 
-          it "returns 1 row" do
-            expect(subject.rows.size).to eq 1
+          it "returns 1 row with a verification" do
+            expect(subject.rows.select(&:has_verification?).size).to eq 1
+          end
+
+          it "returns 1 row without a verification" do
+            expect(subject.rows.reject(&:has_verification?).size).to eq 1
           end
 
           it "does not allow the consumer to be deployed" do

spec/lib/pact_broker/matrix/repository_dependency_spec.rb

@@ -48,9 +48,9 @@ def shorten_rows rows
         subject { shorten_rows(rows) }
         let(:results) { Repository.new.find(selectors, options) }
 
-        it "returns an empty array" do
-          expect(results).to eq []
-          expect(results.resolved_selectors.find{ |s | s[:pacticipant_name] == "Bar"}[:pacticipant_version_id]).to eq -1
+        it "returns an array with one row that does not have a verification" do
+          expect(results.first).to_not have_verification
+          expect(results.resolved_selectors.find{ |s | s[:pacticipant_name] == "Bar"}.pacticipant_version_id).to eq -1
         end
       end
     end

spec/lib/pact_broker/matrix/repository_spec.rb

@@ -606,7 +606,7 @@ def shorten_rows rows
           end
 
           it "returns the tag information" do
-            expect(subject.first[:provider_version_tags]).to include_hash_matching name: 'prod', latest: 1
+            expect(subject.first.provider_version_tags).to include_hash_matching name: 'prod', latest: 1
           end
         end
 
@@ -647,8 +647,9 @@ def shorten_rows rows
             ]
           end
 
-          it "returns no data" do
-            expect(subject.size).to eq 0
+          it "returns a row with no verification" do
+            expect(subject.size).to eq 1
+            expect(subject.first).to_not have_verification
           end
         end
       end