From 6ab3fda66be97af439fa99f5758cf778528513b1 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Fri, 6 Oct 2017 09:27:32 +1100 Subject: [PATCH] feat: add configuration option for check_for_potential_duplicate_pacticipant_names This allows the checking for potential duplicate names (eg. a service being called "Foo" in one pact and "Foo Service" or "foo" or "Foos" in another) to be turned off when publishing a pact. Turning this check off will likely result in a mess of data, and result in the end of the world as we know it. Just for @uglyog --- .../resources/pacticipant_resource_methods.rb | 17 ++++++----- lib/pact_broker/configuration.rb | 10 ++++++- lib/pact_broker/locale/en.yml | 4 +-- ...d_potential_duplicate_pacticipant_names.rb | 2 -- spec/features/create_pacticipant_spec.rb | 30 +++++++++++++++++++ spec/features/publish_pact_spec.rb | 26 ++++++++++++++++ spec/lib/pact_broker/messages_spec.rb | 6 +--- 7 files changed, 78 insertions(+), 17 deletions(-) create mode 100644 spec/features/create_pacticipant_spec.rb diff --git a/lib/pact_broker/api/resources/pacticipant_resource_methods.rb b/lib/pact_broker/api/resources/pacticipant_resource_methods.rb index 1e14405db..29dab2b88 100644 --- a/lib/pact_broker/api/resources/pacticipant_resource_methods.rb +++ b/lib/pact_broker/api/resources/pacticipant_resource_methods.rb @@ -5,15 +5,18 @@ module Resources module PacticipantResourceMethods def potential_duplicate_pacticipants? pacticipant_names - messages = pacticipant_service.messages_for_potential_duplicate_pacticipants pacticipant_names, base_url - if messages.any? - response.body = messages.join("\n") - response.headers['Content-Type'] = 'text/plain' + if PactBroker.configuration.check_for_potential_duplicate_pacticipant_names + messages = pacticipant_service.messages_for_potential_duplicate_pacticipants pacticipant_names, base_url + if messages.any? + response.body = messages.join("\n") + response.headers['Content-Type'] = 'text/plain' + end + messages.any? + else + false end - messages.any? end - end end end -end \ No newline at end of file +end diff --git a/lib/pact_broker/configuration.rb b/lib/pact_broker/configuration.rb index ec358d775..17c3b90d8 100644 --- a/lib/pact_broker/configuration.rb +++ b/lib/pact_broker/configuration.rb @@ -11,11 +11,18 @@ def self.reset_configuration class Configuration - SAVABLE_SETTING_NAMES = [:order_versions_by_date, :use_case_sensitive_resource_names, :enable_public_badge_access, :shields_io_base_url] + SAVABLE_SETTING_NAMES = [ + :order_versions_by_date, + :use_case_sensitive_resource_names, + :enable_public_badge_access, + :shields_io_base_url, + :check_for_potential_duplicate_pacticipant_names + ] attr_accessor :log_dir, :database_connection, :auto_migrate_db, :use_hal_browser, :html_pact_renderer attr_accessor :validate_database_connection_config, :enable_diagnostic_endpoints, :version_parser attr_accessor :use_case_sensitive_resource_names, :order_versions_by_date + attr_accessor :check_for_potential_duplicate_pacticipant_names attr_accessor :semver_formats attr_accessor :enable_public_badge_access, :shields_io_base_url attr_accessor :webhook_retry_schedule @@ -50,6 +57,7 @@ def self.default_configuration config.order_versions_by_date = false config.semver_formats = ["%M.%m.%p%s%d","%M.%m", "%M"] config.webhook_retry_schedule = [10, 60, 120, 300, 600, 1200] #10 sec, 1 min, 2 min, 5 min, 10 min, 20 min => 38 minutes + config.check_for_potential_duplicate_pacticipant_names = true config end diff --git a/lib/pact_broker/locale/en.yml b/lib/pact_broker/locale/en.yml index 0aefc31c0..78f9e728a 100644 --- a/lib/pact_broker/locale/en.yml +++ b/lib/pact_broker/locale/en.yml @@ -28,8 +28,8 @@ en: 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 authentication, include the '-u' flag with the proper credentials: - $ curl -v -XPOST -u : -H "Content-Type: application/json" -d "{\"name\": \"%{new_name}\"}" %{create_pacticipant_url} + If the pact broker requires basic authentication, add '-u ' to the command. + To disable this check, set `check_for_potential_duplicate_pacticipant_names` to false in the configuration. "400": title: 400 Malformed Request message: The request was malformed and could not be processed. diff --git a/lib/pact_broker/pacticipants/find_potential_duplicate_pacticipant_names.rb b/lib/pact_broker/pacticipants/find_potential_duplicate_pacticipant_names.rb index 31b00883f..bbdc26a9e 100644 --- a/lib/pact_broker/pacticipants/find_potential_duplicate_pacticipant_names.rb +++ b/lib/pact_broker/pacticipants/find_potential_duplicate_pacticipant_names.rb @@ -38,8 +38,6 @@ def self.split(string) .downcase .split("_") end - end - end end \ No newline at end of file diff --git a/spec/features/create_pacticipant_spec.rb b/spec/features/create_pacticipant_spec.rb new file mode 100644 index 000000000..0ea4d67e1 --- /dev/null +++ b/spec/features/create_pacticipant_spec.rb @@ -0,0 +1,30 @@ +require 'support/test_data_builder' + +describe "Creating a pacticipant" do + let(:path) { "/pacticipants" } + let(:headers) { {'CONTENT_TYPE' => 'application/json'} } + let(:response_body) { JSON.parse(last_response.body, symbolize_names: true)} + let(:pacticipant_hash) { {name: 'Foo Thing'}} + + subject { post path, pacticipant_hash.to_json, headers; last_response } + + it "returns a 201 response" do + subject + expect(last_response.status).to be 201 + end + + it "returns the Location header" do + subject + expect(last_response.headers['Location']).to eq 'http://example.org/pacticpants/Foo%20Thing' + end + + it "returns a JSON Content Type" do + subject + expect(last_response.headers['Content-Type']).to eq 'application/hal+json;charset=utf-8' + end + + it "returns the newly created webhook" do + subject + expect(response_body).to include pacticipant_hash + end +end diff --git a/spec/features/publish_pact_spec.rb b/spec/features/publish_pact_spec.rb index 301066a17..71be0bb42 100644 --- a/spec/features/publish_pact_spec.rb +++ b/spec/features/publish_pact_spec.rb @@ -46,4 +46,30 @@ expect(subject).to be_a_json_error_response "does not match" end end + + context "when the pacticipant name is an almost duplicate of an existing pacticipant name" do + before do + TestDataBuilder.new.create_pacticipant("A Provider Service") + end + + context "when duplicate checking is on" do + before do + PactBroker.configuration.check_for_potential_duplicate_pacticipant_names = true + end + + it "returns a 409" do + expect(subject.status).to eq 409 + end + end + + context "when duplicate checking is off" do + before do + PactBroker.configuration.check_for_potential_duplicate_pacticipant_names = false + end + + it "returns a 201" do + expect(subject.status).to eq 201 + end + end + end end diff --git a/spec/lib/pact_broker/messages_spec.rb b/spec/lib/pact_broker/messages_spec.rb index b6ddccaac..a8ef663cd 100644 --- a/spec/lib/pact_broker/messages_spec.rb +++ b/spec/lib/pact_broker/messages_spec.rb @@ -3,8 +3,6 @@ module PactBroker module Messages - - describe "#potential_duplicate_pacticipant_message" do let(:new_name) { 'Contracts' } let(:fred) { double('Contracts Service', name: 'Contracts Service') } @@ -19,14 +17,12 @@ module Messages 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 "Contracts" using the following command, and then re-publish the pact: $ curl -v -XPOST -H "Content-Type: application/json" -d "{\\\"name\\\": \\\"Contracts\\\"}" http://example.org/pacticipants -If the pact broker requires authentication, include the '-u' flag with the proper credentials: -$ curl -v -XPOST -u : -H "Content-Type: application/json" -d "{\\\"name\\\": \\\"Contracts\\\"}" http://example.org/pacticipants EOS } subject { Messages.potential_duplicate_pacticipant_message new_name, potential_duplicate_pacticipants, 'http://example.org' } it "returns a message" do - expect(subject).to eq expected_message + expect(subject).to start_with expected_message end end end