feat: refactor can-i-deploy to use HAL Client instead of HTTParty

lib/pact_broker/client/can_i_deploy.rb

@@ -4,6 +4,7 @@
 require 'pact_broker/client/matrix/formatter'
 require 'term/ansicolor'
 require 'pact_broker/client/colorize_notices'
+require "pact_broker/client/matrix/query"
 
 module PactBroker
   module Client
@@ -18,12 +19,11 @@ def initialize success, message = nil
         end
       end
 
-      def self.call(pact_broker_base_url, version_selectors, matrix_options, options, pact_broker_client_options={})
-        new(pact_broker_base_url, version_selectors, matrix_options, options, pact_broker_client_options).call
+      def self.call(version_selectors, matrix_options, options, pact_broker_client_options={})
+        new(version_selectors, matrix_options, options, pact_broker_client_options).call
       end
 
-      def initialize(pact_broker_base_url, version_selectors, matrix_options, options, pact_broker_client_options)
-        @pact_broker_base_url = pact_broker_base_url
+      def initialize(version_selectors, matrix_options, options, pact_broker_client_options)
         @version_selectors = version_selectors
         @matrix_options = matrix_options
         @options = options
@@ -32,15 +32,13 @@ def initialize(pact_broker_base_url, version_selectors, matrix_options, options,
 
       def call
         create_result(fetch_matrix_with_retries)
-      rescue PactBroker::Client::Error => e
-        Result.new(dry_run_or_false, for_dry_run(Term::ANSIColor.red(e.message)))
       rescue StandardError => e
         Result.new(dry_run_or_false, for_dry_run(Term::ANSIColor.red("Error retrieving matrix. #{e.class} - #{e.message}") + "\n#{e.backtrace.join("\n")}"))
       end
 
       private
 
-      attr_reader :pact_broker_base_url, :version_selectors, :matrix_options, :options, :pact_broker_client_options
+      attr_reader :version_selectors, :matrix_options, :options, :pact_broker_client_options
 
       def create_result(matrix)
         if matrix.deployable?
@@ -106,7 +104,7 @@ def format
       end
 
       def fetch_matrix
-        Retry.while_error { pact_broker_client.matrix.get(version_selectors, matrix_options) }
+        Retry.while_error { PactBroker::Client::Matrix::Query.call({ selectors: version_selectors, matrix_options: matrix_options }, options, pact_broker_client_options) }
       end
 
       def fetch_matrix_with_retries
@@ -124,10 +122,6 @@ def fetch_matrix_with_retries
         matrix
       end
 
-      def pact_broker_client
-        @pact_broker_client ||= PactBroker::Client::PactBrokerClient.new(base_url: pact_broker_base_url, client_options: pact_broker_client_options)
-      end
-
       def retry_while_unknown?
         options[:retry_while_unknown] > 0
       end

lib/pact_broker/client/cli/matrix_commands.rb

@@ -33,7 +33,7 @@ def can_i_deploy(*ignored_but_necessary)
               validate_can_i_deploy_selectors(selectors)
               dry_run = options.dry_run || ENV["PACT_BROKER_CAN_I_DEPLOY_DRY_RUN"] == "true"
               can_i_deploy_options = { output: options.output, retry_while_unknown: options.retry_while_unknown, retry_interval: options.retry_interval, dry_run: dry_run, verbose: options.verbose }
-              result = CanIDeploy.call(options.broker_base_url, selectors, { to_tag: options.to, to_environment: options.to_environment, limit: options.limit, ignore_selectors: ignore_selectors }, can_i_deploy_options, pact_broker_client_options)
+              result = CanIDeploy.call(selectors, { to_tag: options.to, to_environment: options.to_environment, limit: options.limit, ignore_selectors: ignore_selectors }, can_i_deploy_options, pact_broker_client_options)
               $stdout.puts result.message
               $stdout.flush
               exit(can_i_deploy_exit_status) unless result.success

lib/pact_broker/client/hal/http_client.rb

@@ -19,7 +19,7 @@ def initialize options
         end
 
         def get href, params = {}, headers = {}
-          query = params.collect{ |(key, value)| "#{CGI::escape(key.to_s)}=#{CGI::escape(value.to_s)}" }.join("&")
+          query = build_nested_query(params)
           uri = URI(href)
           uri.query = query
           perform_request(create_request(uri, 'Get', nil, headers), uri)
@@ -130,6 +130,29 @@ def disable_ssl_verification?
           ENV['PACT_DISABLE_SSL_VERIFICATION'] == 'true' || ENV['PACT_BROKER_DISABLE_SSL_VERIFICATION'] == 'true'
         end
 
+        # From Rack lib/rack/utils.rb
+        def build_nested_query(value, prefix = nil)
+          case value
+          when Array
+            value.map { |v|
+              build_nested_query(v, "#{prefix}[]")
+            }.join("&")
+          when Hash
+            value.map { |k, v|
+              build_nested_query(v, prefix ? "#{prefix}[#{escape(k)}]" : escape(k))
+            }.delete_if(&:empty?).join('&')
+          when nil
+            prefix
+          else
+            raise ArgumentError, "value must be a Hash" if prefix.nil?
+            "#{prefix}=#{escape(value)}"
+          end
+        end
+
+        def escape(s)
+          URI.encode_www_form_component(s)
+        end
+
         class Response < SimpleDelegator
           def body
             bod = raw_body

lib/pact_broker/client/matrix/query.rb

@@ -0,0 +1,77 @@
+require 'pact_broker/client/base_command'
+require "pact_broker/client/matrix/resource"
+require "pact_broker/client/matrix"
+
+module PactBroker
+  module Client
+    class Matrix < BaseClient
+      class Query < PactBroker::Client::BaseCommand
+
+        def call
+          matrix_entity = create_entry_point("#{pact_broker_client_options[:pact_broker_base_url]}/matrix", pact_broker_client_options).get!(query_params)
+          Matrix::Resource.new(JSON.parse(matrix_entity.response.raw_body, symbolize_names: true))
+        end
+
+        private
+
+        attr_reader :action, :response_entity
+
+
+        def selectors
+          params[:selectors]
+        end
+
+        def matrix_options
+          @matrix_options ||= params[:matrix_options] ||{}
+        end
+
+        def query_params
+          latestby = selectors.size == 1 ? 'cvp' : 'cvpv'
+          query = {
+            q: convert_selector_hashes_to_params(selectors),
+            latestby: latestby
+          }.merge(query_options)
+        end
+
+        def query_options
+          opts = {}
+          if matrix_options.key?(:success)
+            opts[:success] = [*matrix_options[:success]]
+          end
+          opts[:limit] = matrix_options[:limit] if matrix_options[:limit]
+          opts[:environment] = matrix_options[:to_environment] if matrix_options[:to_environment]
+          if matrix_options[:to_tag]
+            opts[:latest] = 'true'
+            opts[:tag] = matrix_options[:to_tag]
+          elsif selectors.size == 1 && !matrix_options[:to_environment]
+            opts[:latest] = 'true'
+          end
+          if matrix_options[:ignore_selectors] && matrix_options[:ignore_selectors].any?
+            opts[:ignore] = convert_selector_hashes_to_params(matrix_options[:ignore_selectors])
+          end
+          opts
+        end
+
+        def convert_selector_hashes_to_params(selectors)
+          selectors.collect do |selector|
+            { pacticipant: selector[:pacticipant] }.tap do | hash |
+              hash[:version] = selector[:version] if selector[:version]
+              hash[:latest] = 'true' if selector[:latest]
+              hash[:tag] = selector[:tag] if selector[:tag]
+              hash[:branch] = selector[:branch] if selector[:branch]
+            end
+          end
+        end
+
+
+        def result_message
+          if json_output?
+            response_entity.response.raw_body
+          else
+            green("Pacticipant \"#{params[:name]}\" #{action} in #{pact_broker_name}")
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/pact_broker/client/can_i_deploy_spec.rb

@@ -7,7 +7,7 @@ module Client
       let(:pact_broker_base_url) { 'http://example.org' }
       let(:version_selectors) { [{ pacticipant: "Foo", version: "1" }] }
       let(:matrix_options) { { } }
-      let(:pact_broker_client_options) { { foo: 'bar' } }
+      let(:pact_broker_client_options) { { pact_broker_base_url: pact_broker_base_url, foo: 'bar' } }
       let(:dry_run) { false }
       let(:matrix_client) { instance_double('PactBroker::Client::Matrix') }
       let(:matrix) do
@@ -31,15 +31,14 @@ module Client
 
 
       before do
-        allow_any_instance_of(PactBroker::Client::PactBrokerClient).to receive(:matrix).and_return(matrix_client)
-        allow(matrix_client).to receive(:get).and_return(matrix)
+        allow(PactBroker::Client::Matrix::Query).to receive(:call).and_return(matrix)
         allow(Matrix::Formatter).to receive(:call).and_return('text matrix')
       end
 
-      subject { CanIDeploy.call(pact_broker_base_url, version_selectors, matrix_options, options, pact_broker_client_options) }
+      subject { CanIDeploy.call(version_selectors, matrix_options, options, pact_broker_client_options) }
 
       it "retrieves the matrix from the pact broker" do
-        expect(matrix_client).to receive(:get).with(version_selectors, matrix_options)
+        expect(PactBroker::Client::Matrix::Query).to receive(:call).with({ selectors: version_selectors, matrix_options: matrix_options }, options, pact_broker_client_options)
         subject
       end
 
@@ -193,7 +192,7 @@ module Client
 
       context "when a PactBroker::Client::Error is raised" do
         before do
-          allow(matrix_client).to receive(:get).and_raise(PactBroker::Client::Error.new('error text'))
+          allow(PactBroker::Client::Matrix::Query).to receive(:call).and_raise(PactBroker::Client::Error.new('error text'))
         end
 
         it "returns a failure response" do
@@ -222,7 +221,7 @@ module Client
         before do
           allow(Retry).to receive(:while_error) { |&block| block.call }
           allow($stderr).to receive(:puts)
-          allow(matrix_client).to receive(:get).and_raise(StandardError.new('error text'))
+          allow(PactBroker::Client::Matrix::Query).to receive(:call).and_raise(StandardError.new('error text'))
         end
 
         it "returns a failure response" do

spec/lib/pact_broker/client/cli/broker_can_i_deploy_spec.rb

@@ -38,7 +38,7 @@ module CLI
         end
 
         it "invokes the CanIDeploy service" do
-          expect(CanIDeploy).to receive(:call).with('http://pact-broker', version_selectors, { to_tag: nil, to_environment: nil, limit: 1000, ignore_selectors: []}, {output: 'table', retry_while_unknown: 1, retry_interval: 2, dry_run: false, verbose: "verbose"}, { pact_broker_base_url: 'http://pact-broker', verbose: 'verbose' })
+          expect(CanIDeploy).to receive(:call).with(version_selectors, { to_tag: nil, to_environment: nil, limit: 1000, ignore_selectors: []}, {output: 'table', retry_while_unknown: 1, retry_interval: 2, dry_run: false, verbose: "verbose"}, { pact_broker_base_url: 'http://pact-broker', verbose: 'verbose' })
           invoke_can_i_deploy
         end
 
@@ -56,7 +56,7 @@ module CLI
           end
 
           it "passes the value as the matrix options" do
-            expect(CanIDeploy).to receive(:call).with(anything, anything, {to_tag: 'prod', to_environment: nil, limit: 1000, ignore_selectors: []}, anything, anything)
+            expect(CanIDeploy).to receive(:call).with(anything, {to_tag: 'prod', to_environment: nil, limit: 1000, ignore_selectors: []}, anything, anything)
             invoke_can_i_deploy
           end
         end
@@ -67,7 +67,7 @@ module CLI
           end
 
           it "passes the value as the matrix options" do
-            expect(CanIDeploy).to receive(:call).with(anything, anything, {to_tag: nil, to_environment: 'prod', limit: 1000, ignore_selectors: []}, anything, anything)
+            expect(CanIDeploy).to receive(:call).with(anything, {to_tag: nil, to_environment: 'prod', limit: 1000, ignore_selectors: []}, anything, anything)
             invoke_can_i_deploy
           end
         end
@@ -79,7 +79,7 @@ module CLI
           end
 
           it "invokes the CanIDeploy service with the basic auth credentials" do
-            expect(CanIDeploy).to receive(:call).with(anything, anything, anything, anything, { pact_broker_base_url: 'http://pact-broker', basic_auth: {username: "foo", password: "bar"}, verbose: 'verbose'})
+            expect(CanIDeploy).to receive(:call).with(anything, anything, anything, { pact_broker_base_url: 'http://pact-broker', basic_auth: {username: "foo", password: "bar"}, verbose: 'verbose'})
             invoke_can_i_deploy
           end
         end
@@ -90,7 +90,7 @@ module CLI
           end
 
           it "invokes the CanIDeploy service with the basic auth credentials" do
-            expect(CanIDeploy).to receive(:call).with(anything, anything, anything, anything, {pact_broker_base_url: 'http://pact-broker', token: "some token", verbose: 'verbose'})
+            expect(CanIDeploy).to receive(:call).with(anything, anything, anything, {pact_broker_base_url: 'http://pact-broker', token: "some token", verbose: 'verbose'})
             invoke_can_i_deploy
           end
         end
@@ -102,7 +102,7 @@ module CLI
           end
 
           it "invokes the CanIDeploy service with dry_run set to true" do
-            expect(CanIDeploy).to receive(:call).with(anything, anything, anything, hash_including(dry_run: true), anything)
+            expect(CanIDeploy).to receive(:call).with(anything, anything, hash_including(dry_run: true), anything)
             invoke_can_i_deploy
           end
         end

spec/service_providers/pact_broker_client_matrix_ignore_spec.rb

@@ -88,7 +88,7 @@ module PactBroker::Client
           )
       end
 
-      subject { PactBroker::Client::CanIDeploy.call(broker_base_url, selectors, matrix_options, options, {})}
+      subject { PactBroker::Client::CanIDeploy.call(selectors, matrix_options, options, { pact_broker_base_url: broker_base_url })}
 
       it 'returns the CLI output' do
         Approvals.verify(subject.message, :name => "can_i_deploy_ignore", format: :txt)

spec/service_providers/pact_broker_client_matrix_spec.rb

@@ -1,5 +1,6 @@
 require_relative 'pact_helper'
 require 'pact_broker/client'
+require "pact_broker/client/matrix/query"
 
 module PactBroker::Client
   describe Matrix, :pact => true do
@@ -10,6 +11,9 @@ module PactBroker::Client
       let(:matrix_response_body) { Pact.like(matrix) }
       let(:matrix) { JSON.parse(File.read('spec/support/matrix.json')) }
       let(:selectors) { [{ pacticipant: "Foo", version: "1.2.3" }, { pacticipant: "Bar", version: "4.5.6" }] }
+      let(:options) { {} }
+
+      subject { PactBroker::Client::Matrix::Query.call({ selectors: selectors, options: options }, {}, { pact_broker_base_url: pact_broker.mock_service_base_url }) }
 
       context "when results are found" do
         before do
@@ -29,7 +33,7 @@ module PactBroker::Client
         end
 
         it 'returns the pact matrix' do
-          matrix = pact_broker_client.matrix.get(selectors)
+          matrix = subject
           expect(matrix[:matrix].size).to eq 1
         end
       end
@@ -54,7 +58,7 @@ module PactBroker::Client
         let(:selectors) { [{ pacticipant: "Foo Thing", version: "1.2.3" }, { pacticipant: "Bar", version: "4.5.6" }] }
 
         it 'incorrectly escapes the spaces but it still seems to work' do
-          matrix = pact_broker_client.matrix.get(selectors)
+          matrix = subject
           expect(matrix[:matrix].size).to eq 1
         end
       end
@@ -79,7 +83,7 @@ module PactBroker::Client
         let(:selectors) { [{ pacticipant: "Foo", version: "1.2.3" }] }
 
         it 'returns the row with the lastest verification for version 1.2.3' do
-          matrix = pact_broker_client.matrix.get(selectors)
+          matrix = subject
           expect(matrix[:matrix].size).to eq 1
         end
       end
@@ -108,7 +112,7 @@ module PactBroker::Client
         let(:selectors) { [{ pacticipant: "Foo", version: "1.2.3" }, { pacticipant: "Bar", version: "9.9.9" }] }
 
         it 'does not raise an error' do
-          pact_broker_client.matrix.get(selectors)
+          subject
         end
       end
 
@@ -134,8 +138,8 @@ module PactBroker::Client
 
         it 'raises an error' do
           expect {
-            pact_broker_client.matrix.get(selectors)
-          }.to raise_error PactBroker::Client::Error, "an error message"
+            subject
+          }.to raise_error PactBroker::Client::Hal::ErrorResponseReturned, /an error message/
         end
       end
 
@@ -161,7 +165,7 @@ module PactBroker::Client
         let(:selectors) { [{ pacticipant: "Foo" }, { pacticipant: "Bar" }] }
 
         it "returns multiple rows" do
-          matrix = pact_broker_client.matrix.get(selectors)
+          matrix = subject
           expect(matrix[:matrix].size).to eq 2
         end
       end