feat: check for potential duplicate pacticipants in publish contracts…

… endpoint (#558)

lib/pact_broker/api/resources/pacticipant_resource_methods.rb

@@ -1,7 +1,6 @@
 module PactBroker
   module Api
     module Resources
-
       module PacticipantResourceMethods
 
         def potential_duplicate_pacticipants? pacticipant_names

lib/pact_broker/api/resources/publish_contracts.rb

@@ -100,7 +100,7 @@ def set_conflict_response
         end
 
         def conflict_notices
-          @conflict_notices ||= contract_service.conflict_notices(parsed_contracts)
+          @conflict_notices ||= contract_service.conflict_notices(parsed_contracts, base_url: base_url)
         end
 
         def base64_decode(content)

lib/pact_broker/contracts/contract_to_publish.rb

@@ -14,6 +14,10 @@ def pact?
       def merge?
         on_conflict == "merge"
       end
+
+      def pacticipant_names
+        [consumer_name, provider_name]
+      end
     end
   end
 end

lib/pact_broker/contracts/contracts_to_publish.rb

@@ -6,6 +6,10 @@ def self.from_hash(pacticipant_name: nil, pacticipant_version_number: nil, tags:
         new(pacticipant_name, pacticipant_version_number, tags, branch, build_url, contracts)
       end
       # rubocop: enable Metrics/ParameterLists
+
+      def pacticipant_names
+        contracts.flat_map(&:pacticipant_names).uniq
+      end
     end
   end
 end

lib/pact_broker/contracts/service.rb

@@ -43,19 +43,26 @@ def publish(parsed_contracts, base_url: )
         )
       end
 
-      def conflict_notices(parsed_contracts)
+      def conflict_notices(parsed_contracts, base_url: )
         notices = []
+        add_pact_conflict_notices(notices, parsed_contracts)
+        add_pacticipant_conflict_notices(notices, parsed_contracts, base_url)
+        notices
+      end
+
+      def add_pact_conflict_notices(notices, parsed_contracts)
         parsed_contracts.contracts.collect do | contract_to_publish |
           pact_params = create_pact_params(parsed_contracts, contract_to_publish)
           existing_pact = pact_service.find_pact(pact_params)
           if existing_pact && pact_service.disallowed_modification?(existing_pact, contract_to_publish.decoded_content)
-            add_conflict_notice(notices, parsed_contracts, contract_to_publish, existing_pact.json_content, contract_to_publish.decoded_content)
+            add_pact_conflict_notice(notices, parsed_contracts, contract_to_publish, existing_pact.json_content, contract_to_publish.decoded_content)
           end
         end
-        notices
       end
 
-      def add_conflict_notice(notices, parsed_contracts, contract_to_publish, existing_json_content, new_json_content)
+      private :add_pact_conflict_notices
+
+      def add_pact_conflict_notice(notices, parsed_contracts, contract_to_publish, existing_json_content, new_json_content)
         message_params = {
           consumer_name: contract_to_publish.provider_name,
           consumer_version_number: parsed_contracts.pacticipant_version_number,
@@ -65,7 +72,18 @@ def add_conflict_notice(notices, parsed_contracts, contract_to_publish, existing
         notices << Notice.info(PactBroker::Pacts::CreateFormattedDiff.call(new_json_content, existing_json_content))
       end
 
-      private :add_conflict_notice
+      private :add_pact_conflict_notice
+
+      def add_pacticipant_conflict_notices(notices, parsed_contracts, base_url)
+        if PactBroker.configuration.check_for_potential_duplicate_pacticipant_names
+          duplicate_pacticipant_messages = pacticipant_service.messages_for_potential_duplicate_pacticipants(parsed_contracts.pacticipant_names, base_url)
+          duplicate_pacticipant_messages.each do | message_text |
+            notices << Notice.error(message_text)
+          end
+        end
+      end
+
+      private :add_pacticipant_conflict_notices
 
       def create_version(parsed_contracts)
         version_params = {

lib/pact_broker/locale/en.yml

@@ -96,12 +96,14 @@ en:
         invalid_limit: The limit must be 1 or greater.
       duplicate_pacticipant: |
         This is the first time a pact has been published for "%{new_name}".
-        The name "%{new_name}" is very similar to the following existing consumers/providers:
+        The name "%{new_name}" is very similar to the following existing pacticipants:
         %{existing_names}
         If you meant to specify one of the above names, please correct the pact configuration, and re-publish the pact.
-        If the pact is intended to be for a new consumer or provider, please manually create "%{new_name}" using the following command, and then re-publish the pact:
-        $ curl -v -XPOST -H "Content-Type: application/json" -d "{\"name\": \"%{new_name}\"}" %{create_pacticipant_url}
-        If the pact broker requires basic authentication, add '-u <username:password>' to the command.
+        If the pact is intended to be for a new consumer or provider, please manually create "%{new_name}" using the following command from the Pact Broker CLI, and then re-publish the pact:
+
+        pact-broker create-or-update-pacticipant --name "%{new_name}" --broker-base-url %{base_url}
+
+        For information on the Pact Broker CLI see https://docs.pact.io/go/pbc-cli
         To disable this check, set `check_for_potential_duplicate_pacticipant_names` to false in the configuration.
       new_line_in_url_path: URL path cannot contain a new line character.
       tab_in_url_path: URL path cannot contain a tab character.

lib/pact_broker/messages.rb

@@ -23,15 +23,6 @@ def validation_message key, options = {}
       message("errors.validation." + key, options)
     end
 
-    def potential_duplicate_pacticipant_message new_name, potential_duplicate_pacticipants, base_url
-      existing_names = potential_duplicate_pacticipants.
-        collect{ | p | "* #{p.name}"  }.join("\n")
-      message("errors.duplicate_pacticipant",
-        new_name: new_name,
-        existing_names: existing_names,
-        create_pacticipant_url: pacticipants_url(base_url))
-    end
-
     def pluralize(word, count)
       if count == 1
         word
@@ -43,11 +34,5 @@ def pluralize(word, count)
         end
       end
     end
-
-    private
-
-    def pacticipants_url base_url
-      PactBroker::Api::PactBrokerUrls.pacticipants_url base_url
-    end
   end
 end

lib/pact_broker/pacticipants/service.rb

@@ -10,13 +10,14 @@ class Service
       extend PactBroker::Repositories
       extend PactBroker::Services
       include PactBroker::Logging
+      extend PactBroker::Messages
 
       def self.messages_for_potential_duplicate_pacticipants(pacticipant_names, base_url)
         messages = []
         pacticipant_names.each do | name |
           potential_duplicate_pacticipants = find_potential_duplicate_pacticipants(name)
           if potential_duplicate_pacticipants.any?
-            messages << Messages.potential_duplicate_pacticipant_message(name, potential_duplicate_pacticipants, base_url)
+            messages << potential_duplicate_pacticipant_message(name, potential_duplicate_pacticipants, base_url)
           end
         end
         messages
@@ -98,6 +99,15 @@ def self.maybe_set_main_branch(pacticipant, potential_main_branch)
           pacticipant
         end
       end
+
+      private_class_method def self.potential_duplicate_pacticipant_message(new_name, potential_duplicate_pacticipants, base_url)
+        existing_names = potential_duplicate_pacticipants.
+          collect{ | p | "* #{p.name}"  }.join("\n")
+        message("errors.duplicate_pacticipant",
+          new_name: new_name,
+          existing_names: existing_names,
+          base_url: base_url)
+      end
     end
   end
 end

spec/lib/pact_broker/contracts/service_spec.rb

@@ -126,6 +126,15 @@ module Contracts
       end
 
       describe "#conflict_errors" do
+        before do
+          allow(Service).to receive(:pacticipant_service).and_return(pacticipant_service)
+          allow(pacticipant_service).to receive(:messages_for_potential_duplicate_pacticipants).and_return(duplicate_pacticipant_messages)
+          allow(PactBroker.configuration).to receive(:check_for_potential_duplicate_pacticipant_names).and_return(true)
+        end
+
+        let(:pacticipant_service) { class_double("PactBroker::Pacticipants::Service").as_stubbed_const }
+        let(:duplicate_pacticipant_messages) { [] }
+
         let(:contracts_to_publish) do
           ContractsToPublish.from_hash(
             pacticipant_name: "Foo",
@@ -152,7 +161,7 @@ module Contracts
         let(:contract_hash) { { consumer: { name: "Foo" }, provider: { name: "Bar" }, interactions: [{a: "b"}] } }
         let(:decoded_contract) { contract_hash.to_json }
 
-        subject { Service.conflict_notices(contracts_to_publish) }
+        subject { Service.conflict_notices(contracts_to_publish, base_url: "base_url") }
 
         context "when a pact already exists" do
           before do
@@ -187,6 +196,19 @@ module Contracts
         context "when no pacts exist" do
           it { is_expected.to be_empty }
         end
+
+        it "checks if there are potential duplicate pacticipants" do
+          expect(pacticipant_service).to receive(:messages_for_potential_duplicate_pacticipants).with(["Foo", "Bar"], "base_url")
+          subject
+        end
+
+        context "when there are potential duplicate pacticipants" do
+          let(:duplicate_pacticipant_messages) { ["some message" ] }
+
+          it "returns the messages as error notices" do
+            expect(subject).to contain_exactly(have_attributes(type: "error", text: "some message"))
+          end
+        end
       end
     end
   end

spec/lib/pact_broker/pacticipants/service_spec.rb

@@ -1,4 +1,3 @@
-require "spec_helper"
 require "pact_broker/pacticipants/service"
 require "pact_broker/domain/tag"
 require "pact_broker/domain/pact"
@@ -15,15 +14,14 @@ module Pacticipants
       subject { Service }
 
       describe ".messages_for_potential_duplicate_pacticipants" do
-
         let(:base_url) { "http://example.org" }
         let(:fred_duplicates) { [double("Frederich pacticipant")] }
         let(:mary_dulicates) { [double("Marta pacticipant")] }
 
         before do
           allow(Service).to receive(:find_potential_duplicate_pacticipants).with("Fred").and_return(fred_duplicates)
           allow(Service).to receive(:find_potential_duplicate_pacticipants).with("Mary").and_return(mary_dulicates)
-          allow(Messages).to receive(:potential_duplicate_pacticipant_message).and_return("message1", "message2")
+          allow(Service).to receive(:potential_duplicate_pacticipant_message).and_return("message1", "message2")
         end
 
         subject { Service.messages_for_potential_duplicate_pacticipants ["Fred", "Mary"], base_url }
@@ -35,10 +33,9 @@ module Pacticipants
         end
 
         context "when there are potential duplicates" do
-
           it "creates a message for each dupliate" do
-            expect(Messages).to receive(:potential_duplicate_pacticipant_message).with("Fred", fred_duplicates, base_url)
-            expect(Messages).to receive(:potential_duplicate_pacticipant_message).with("Mary", mary_dulicates, base_url)
+            expect(Service).to receive(:potential_duplicate_pacticipant_message).with("Fred", fred_duplicates, base_url)
+            expect(Service).to receive(:potential_duplicate_pacticipant_message).with("Mary", mary_dulicates, base_url)
             subject
           end