style: refactor version selector object from hash into array of hashes

lib/pact_broker/api/resources/matrix.rb

@@ -16,7 +16,7 @@ def allowed_methods
         end
 
         def malformed_request?
-          error_messages = matrix_service.validate_selectors(criteria)
+          error_messages = matrix_service.validate_selectors(selectors)
           if error_messages.any?
             set_json_validation_error_messages error_messages
             true
@@ -26,12 +26,12 @@ def malformed_request?
         end
 
         def to_json
-          lines = matrix_service.find(criteria)
+          lines = matrix_service.find(selectors)
           PactBroker::Api::Decorators::MatrixPactDecorator.new(lines).to_json(user_options: { base_url: base_url })
         end
 
-        def criteria
-          @criteria ||= PactBroker::Matrix::ParseQuery.call(request.uri.query)
+        def selectors
+          @selectors ||= PactBroker::Matrix::ParseQuery.call(request.uri.query)
         end
       end
     end

lib/pact_broker/matrix/parse_query.rb

@@ -5,9 +5,7 @@ module Matrix
     class ParseQuery
       def self.call query
         params = Rack::Utils.parse_nested_query(query)
-        (params['q'] || []).each_with_object({}) do | selector, hash |
-          hash[selector['pacticipant']] = selector['version']
-        end
+        (params['q'] || []).collect{ |i| { pacticipant_name: i['pacticipant'], pacticipant_version_number: i['version'] } }
       end
     end
   end

lib/pact_broker/matrix/repository.rb

@@ -6,29 +6,28 @@ class Repository
       include PactBroker::Repositories::Helpers
       include PactBroker::Repositories
 
-      def find criteria
-        find_all(criteria)
+      def find selectors
+        find_all(selectors)
           .group_by{|line| [line[:consumer_version_number], line[:provider_version_number]]}
           .values
           .collect{ | lines | lines.first[:provider_version_number].nil? ? lines : lines.last }
           .flatten
       end
 
       def find_for_consumer_and_provider pacticipant_1_name, pacticipant_2_name
-        find_all({pacticipant_1_name => nil, pacticipant_2_name => nil})
+        selectors = [{ pacticipant_name: pacticipant_1_name }, { pacticipant_name: pacticipant_2_name }]
+        find_all(selectors)
           .sort{|l1, l2| l2[:consumer_version_order] <=> l1[:consumer_version_order]}
       end
 
-      ##
-      # criteria Hash of pacticipant_name => version
-      # Ihe value is nil, it means all versions for that pacticipant are to be included
-      # Returns a list of matrix lines indicating the compatible versions
-      #
-      def find_compatible_pacticipant_versions criteria
-        find(criteria).select{ |line | line[:success] }
+      def find_compatible_pacticipant_versions selectors
+        find(selectors).select{ |line | line[:success] }
       end
 
-      def find_all criteria
+      ##
+      # If the version is nil, it means all versions for that pacticipant are to be included
+      #
+      def find_all selectors
         PactBroker::Pacts::LatestPactPublicationsByConsumerVersion
           .select_append(:consumer_version_number, :provider_name, :consumer_name, :provider_version_id, :provider_version_number, :success)
           .select_append(Sequel[:latest_pact_publications_by_consumer_versions][:created_at].as(:pact_created_at))
@@ -39,11 +38,11 @@ def find_all criteria
           .where{
             Sequel.&(
               Sequel.|(
-                *criteria.collect{|key, value| value ? Sequel.&(consumer_name: key, consumer_version_number: value) :  Sequel.&(consumer_name: key) }
+                *selectors.collect{ |s| s[:pacticipant_version_number] ? Sequel.&(consumer_name: s[:pacticipant_name], consumer_version_number: s[:pacticipant_version_number]) :  Sequel.&(consumer_name: s[:pacticipant_name]) }
               ),
               Sequel.|(
-                *(criteria.collect{|key, value| value ? Sequel.&(provider_name: key, provider_version_number: value) :  Sequel.&(provider_name: key) } +
-                  criteria.collect{|key, value| Sequel.&(provider_name: key, provider_version_number: nil) })
+                *(selectors.collect{ |s| s[:pacticipant_version_number] ? Sequel.&(provider_name: s[:pacticipant_name], provider_version_number: s[:pacticipant_version_number]) :  Sequel.&(provider_name: s[:pacticipant_name]) } +
+                  selectors.collect{ |s| Sequel.&(provider_name: s[:pacticipant_name], provider_version_number: nil) })
               )
             )
           }

lib/pact_broker/matrix/service.rb

@@ -23,30 +23,26 @@ def find_compatible_pacticipant_versions criteria
       def validate_selectors selectors
         error_messages = []
 
-        selectors.each do | pacticipant_name, version |
-          if pacticipant_name.nil? && version.nil?
+        selectors.each do | selector |
+          if selector[:pacticipant_name].nil? && selector[:pacticipant_version_number].nil?
             error_messages << "Please specify the pacticipant name and version"
-          elsif pacticipant_name.nil?
+          elsif selector[:pacticipant_name].nil?
             error_messages << "Please specify the pacticipant name"
-          elsif version.nil?
-            error_messages << "Please specify the version for #{pacticipant_name}"
+          elsif selector[:pacticipant_version_number].nil?
+            error_messages << "Please specify the version for #{selector[:pacticipant_name]}"
           end
         end
 
-        if selectors.values.any?(&:nil?)
-          error_messages << "Please specify the pacticipant version"
-        end
-
-        selectors.keys.compact.each do | pacticipant_name |
+        selectors.collect{ |selector| selector[:pacticipant_name] }.compact.each do | pacticipant_name |
           unless pacticipant_service.find_pacticipant_by_name(pacticipant_name)
             error_messages << "Pacticipant '#{pacticipant_name}' not found"
           end
         end
 
         if error_messages.empty?
-          selectors.each do | pacticipant_name, version_number |
-            version = version_service.find_by_pacticipant_name_and_number(pacticipant_name: pacticipant_name, pacticipant_version_number: version_number)
-            error_messages << "No pact or verification found for #{pacticipant_name} version #{version_number}" if version.nil?
+          selectors.each do | selector |
+            version = version_service.find_by_pacticipant_name_and_number(pacticipant_name: selector[:pacticipant_name], pacticipant_version_number: selector[:pacticipant_version_number])
+            error_messages << "No pact or verification found for #{selector[:pacticipant_name]} version #{selector[:pacticipant_version_number]}" if version.nil?
           end
         end
 

spec/lib/pact_broker/api/resources/matrix_spec.rb

@@ -7,18 +7,21 @@ module Resources
       describe Matrix do
         before do
           allow(PactBroker::Matrix::Service).to receive(:validate_selectors).and_return(error_messages)
+          allow(PactBroker::Matrix::Service).to receive(:find).and_return([])
+          allow(PactBroker::Matrix::ParseQuery).to receive(:call).and_return(selectors)
         end
 
         let(:td) { TestDataBuilder.new }
         let(:path) { "/matrix" }
         let(:json_response_body) { JSON.parse(subject.body, symbolize_names: true) }
         let(:params) { {q: [{pacticipant: 'Foo', version: '1'}, {pacticipant: 'Bar', version: '2'}]} }
         let(:error_messages) { [] }
+        let(:selectors) { double('selectors') }
 
         subject { get path, params, {'Content-Type' => 'application/hal+json'}; last_response }
 
         it "validates the selectors" do
-          expect(PactBroker::Matrix::Service).to receive(:validate_selectors).with('Foo' => '1', 'Bar' => '2')
+          expect(PactBroker::Matrix::Service).to receive(:validate_selectors).with(selectors)
           subject
         end
 

spec/lib/pact_broker/matrix/parse_query_spec.rb

@@ -9,30 +9,30 @@ module Matrix
         subject { ParseQuery.call(query) }
 
         it "extracts the pacticipant names and respective versions" do
-          expect(subject).to eq "Foo" => "1.2.3", "Bar" => "9.9.9"
+          expect(subject).to eq([{ pacticipant_name: "Foo", pacticipant_version_number: "1.2.3" }, { pacticipant_name: "Bar", pacticipant_version_number: "9.9.9" }])
         end
 
         context "with spaces" do
           let(:query) { "q[][pacticipant]=Name%20With%20Spaces&q[][version]=1%202" }
 
           it "works" do
-            expect(subject).to eq "Name With Spaces" => "1 2"
+            expect(subject).to eq [{pacticipant_name: "Name With Spaces", pacticipant_version_number: "1 2"}]
           end
         end
 
         context "with no q" do
           let(:query) { "foo" }
 
           it "returns an empty hash" do
-            expect(subject).to eq({})
+            expect(subject).to eq([])
           end
         end
 
         context "with an incorrect param names" do
           let(:query) { "q[][wrong]=Foo&q[][blah]=1.2.3" }
 
           it "returns nil keys or values" do
-            expect(subject).to eq nil => nil
+            expect(subject).to eq [{ pacticipant_name: nil, pacticipant_version_number: nil }]
           end
         end
       end

spec/lib/pact_broker/matrix/repository_spec.rb

@@ -5,6 +5,12 @@ module Matrix
     describe Repository do
       let(:td) { TestDataBuilder.new}
 
+      def build_selectors(hash)
+        hash.collect do | key, value |
+          { pacticipant_name: key, pacticipant_version_number: value }
+        end
+      end
+
       describe "find" do
         context "when the provider version resource exists but there is no verification for that version" do
           before do
@@ -19,7 +25,7 @@ module Matrix
               .create_pact
           end
 
-          subject { Repository.new.find "A" => "1.2.3", "B" => "2.0.0", "C" => "3.0.0" }
+          subject { Repository.new.find build_selectors("A" => "1.2.3", "B" => "2.0.0", "C" => "3.0.0") }
 
           it "returns a row for each pact" do
             expect(subject.size).to eq 2
@@ -38,7 +44,7 @@ module Matrix
           end
 
           context "when only 2 version selectors are specified" do
-            subject { Repository.new.find "A" => "1.2.3", "B" => "2.0.0" }
+            subject { Repository.new.find build_selectors("A" => "1.2.3", "B" => "2.0.0") }
 
             it "only returns 1 row" do
               expect(subject.size).to eq 1
@@ -96,7 +102,9 @@ module Matrix
               .create_verification(provider_version: '3', number: 2)
           end
 
-          subject { Repository.new.find_compatible_pacticipant_versions("A" => "1", "B" => "2", "C" => "2") }
+          let(:selectors){ build_selectors("A" => "1", "B" => "2", "C" => "2") }
+
+          subject { Repository.new.find_compatible_pacticipant_versions(selectors) }
 
           it "returns matrix lines for each compatible version pair (A/1-B/2, B/2-C/2)" do
             expect(subject.first[:consumer_name]).to eq "A"
@@ -118,7 +126,8 @@ module Matrix
           end
 
           context "when one or more pacticipants does not have a version specified" do
-            subject { Repository.new.find_compatible_pacticipant_versions("A" => "1", "B" => "2", "C" => nil) }
+            let(:selectors){ build_selectors("A" => "1", "B" => "2", "C" => nil) }
+            subject { Repository.new.find_compatible_pacticipant_versions(selectors) }
 
             it "returns all the rows for that pacticipant" do
               expect(subject[1]).to include(provider_name: "C", provider_version_number: "2")
@@ -128,7 +137,9 @@ module Matrix
           end
 
           context "none of the pacticipants have a version specified" do
-            subject { Repository.new.find_compatible_pacticipant_versions("A" => nil, "B" => nil, "C" => nil) }
+            let(:selectors){ build_selectors("A" => nil, "B" => nil, "C" => nil) }
+
+            subject { Repository.new.find_compatible_pacticipant_versions(selectors) }
 
             it "returns all the rows" do
               expect(subject.size).to eq 4
@@ -142,7 +153,10 @@ module Matrix
               .create_verification(provider_version: "1")
               .create_verification(provider_version: "1", number: 2)
           end
-          subject { Repository.new.find_compatible_pacticipant_versions("X" => "1", "Y" => "1") }
+
+          let(:selectors){ build_selectors("X" => "1", "Y" => "1") }
+
+          subject { Repository.new.find_compatible_pacticipant_versions(selectors) }
 
           it "returns the last line" do
             expect(subject.size).to eq 1
@@ -157,7 +171,9 @@ module Matrix
               .create_verification(provider_version: "1", number: 2, success: false)
           end
 
-          subject { Repository.new.find_compatible_pacticipant_versions("X" => "1", "Y" => "1") }
+          let(:selectors){ build_selectors("X" => "1", "Y" => "1") }
+
+          subject { Repository.new.find_compatible_pacticipant_versions(selectors) }
 
           it "does not return a matrix line" do
             expect(subject.size).to eq 0
@@ -171,7 +187,9 @@ module Matrix
               .revise_pact
           end
 
-          subject { Repository.new.find_compatible_pacticipant_versions("X" => "1", "Y" => "1") }
+          let(:selectors){ build_selectors("X" => "1", "Y" => "1") }
+
+          subject { Repository.new.find_compatible_pacticipant_versions(selectors) }
 
           it "does not return a matrix line" do
             expect(subject.size).to eq 0
@@ -185,7 +203,9 @@ module Matrix
               .create_verification(provider_version: '1', success: false)
           end
 
-          subject { Repository.new.find_compatible_pacticipant_versions("D" => "1", "E" => "1") }
+          let(:selectors){ build_selectors("D" => "1", "E" => "1") }
+
+          subject { Repository.new.find_compatible_pacticipant_versions(selectors) }
 
           it "does not return the matrix line" do
             expect(subject.count).to eq 0

spec/lib/pact_broker/matrix/service_spec.rb

@@ -16,7 +16,7 @@ module Matrix
               .create_version("1")
           end
 
-          let(:selectors) { {"Foo" => "1"} }
+          let(:selectors) { [{ pacticipant_name: "Foo", pacticipant_version_number: "1" }] }
 
           it "returns error messages" do
             expect(subject.first).to eq "Please provide 2 or more version selectors."
@@ -32,39 +32,39 @@ module Matrix
 
           end
 
-          let(:selectors) { {"Foo" => "1", "Bar" => "1"} }
+          let(:selectors) { [{ pacticipant_name: "Foo", pacticipant_version_number: "1" }, { pacticipant_name: "Bar", pacticipant_version_number: "1" }] }
 
           it "returns error messages" do
             expect(subject).to eq ["No pact or verification found for Bar version 1"]
           end
         end
 
         context "when the pacticipant does not exist" do
-          let(:selectors) { {"Foo" => "1"} }
+          let(:selectors) { [{ pacticipant_name: "Foo", pacticipant_version_number: "1" }] }
 
           it "returns error messages" do
             expect(subject.first).to eq "Pacticipant 'Foo' not found"
           end
         end
 
         context "when the pacticipant name is not specified" do
-          let(:selectors) { {nil => "1"} }
+          let(:selectors) { [{ pacticipant_name: nil, pacticipant_version_number: "1" }] }
 
           it "returns error messages" do
             expect(subject.first).to eq "Please specify the pacticipant name"
           end
         end
 
         context "when the pacticipant version is not specified" do
-          let(:selectors) { {'Foo' => nil} }
+          let(:selectors) { [{ pacticipant_name: "Foo", pacticipant_version_number: nil }] }
 
           it "returns error messages" do
             expect(subject.first).to eq "Please specify the version for Foo"
           end
         end
 
         context "when the pacticipant name and version are not specified" do
-          let(:selectors) { {nil => nil} }
+          let(:selectors) { [{ pacticipant_name: nil, pacticipant_version_number: nil }] }
 
           it "returns error messages" do
             expect(subject.first).to eq "Please specify the pacticipant name and version"