feat(webhooks): support upsert of webhook via a PUT to /webhooks/{uuid}

Author: bethesque

lib/pact_broker/api/resources/index.rb

@@ -94,6 +94,11 @@ def links
               title: 'Webhooks',
               templated: false
             },
+            'pb:webhook' => {
+              href: base_url + '/webhooks/{uuid}',
+              title: 'Webhook',
+              templated: true
+            },
             'pb:integrations' => {
               href: base_url + '/integrations',
               title: 'Integrations',

lib/pact_broker/api/resources/webhook.rb

@@ -28,7 +28,7 @@ def resource_exists?
 
         def malformed_request?
           if request.put?
-            return invalid_json? || webhook_validation_errors?(new_webhook)
+            return invalid_json? || webhook_validation_errors?(parsed_webhook, uuid)
           end
           false
         end
@@ -38,7 +38,9 @@ def from_json
             @webhook = webhook_service.update_by_uuid uuid, params_with_string_keys
             response.body = to_json
           else
-            404
+            @webhook = webhook_service.create(uuid, parsed_webhook, consumer, provider)
+            response.body = to_json
+            201
           end
         end
 
@@ -57,8 +59,8 @@ def webhook
           @webhook ||= webhook_service.find_by_uuid uuid
         end
 
-        def new_webhook
-          @new_webhook ||= Decorators::WebhookDecorator.new(PactBroker::Domain::Webhook.new).from_json(request_body)
+        def parsed_webhook
+          @parsed_webhook ||= Decorators::WebhookDecorator.new(PactBroker::Domain::Webhook.new).from_json(request_body)
         end
 
         def uuid

lib/pact_broker/api/resources/webhook_resource_methods.rb

@@ -2,8 +2,8 @@ module PactBroker
   module Api
     module Resources
       module WebhookResourceMethods
-        def webhook_validation_errors? webhook
-          errors = webhook_service.errors(webhook)
+        def webhook_validation_errors?(webhook, uuid = nil)
+          errors = webhook_service.errors(webhook, uuid)
           if !errors.empty?
             set_json_validation_error_messages(errors.messages)
             true

lib/pact_broker/locale/en.yml

@@ -43,6 +43,7 @@ en:
         consumer_version_number_invalid: "Consumer version number '%{consumer_version_number}' cannot be parsed to a version number. The expected format (unless this configuration has been overridden) is a semantic version. eg. 1.3.0 or 2.0.4.rc1"
         pacticipant_name_mismatch: "in pact ('%{name_in_pact}') does not match %{pacticipant} name in path ('%{name}')."
         connection_encoding_not_utf8: "The Sequel connection encoding (%{encoding}) is strongly recommended to be \"utf8\". If you need to set it to %{encoding} for some particular reason, then disable this check by setting config.validate_database_connection_config = false"
+        invalid_webhook_uuid: The UUID can only contain the characters A-Z, a-z, 0-9, _ and -, and must be 16 or more characters.
       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:

lib/pact_broker/webhooks/service.rb

@@ -14,6 +14,7 @@
 require 'pact_broker/webhooks/execution_configuration'
 require 'pact_broker/messages'
 require 'pact_broker/webhooks/pact_and_verification_parameters'
+require 'reform/contract/errors'
 
 module PactBroker
   module Webhooks
@@ -28,14 +29,24 @@ class Service
       include Logging
       extend PactBroker::Messages
 
+      # Not actually a UUID. Ah well.
+      def self.valid_uuid_format?(uuid)
+        !!(uuid =~ /^[A-Za-z0-9_\-]{16,}$/)
+      end
+
       def self.next_uuid
         SecureRandom.urlsafe_base64
       end
 
-      def self.errors webhook
+      def self.errors webhook, uuid = nil
         contract = PactBroker::Api::Contracts::WebhookContract.new(webhook)
         contract.validate(webhook.attributes)
-        contract.errors
+        errors = contract.errors
+
+        if uuid && !valid_uuid_format?(uuid)
+          errors.add("uuid", message("errors.validation.invalid_webhook_uuid"))
+        end
+        errors
       end
 
       def self.create uuid, webhook, consumer, provider

spec/features/create_webhook_spec.rb

@@ -127,4 +127,12 @@
       expect(PactBroker::Webhooks::Webhook.first.provider).to be nil
     end
   end
+
+  context "with a UUID" do
+    let(:path) { "/webhooks/1234123412341234" }
+
+    subject { put(path, webhook_json, headers) }
+
+    its(:status) { is_expected.to be 201 }
+  end
 end

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

@@ -2,11 +2,8 @@
 require 'pact_broker/api/resources/webhook'
 
 module PactBroker::Api
-
   module Resources
-
     describe Webhook do
-
       before do
         allow(PactBroker::Webhooks::Service).to receive(:find_by_uuid).and_return(webhook)
       end
@@ -23,15 +20,14 @@ module Resources
         end
 
         context "when the webhook exists" do
+          before do
+            allow(Decorators::WebhookDecorator).to receive(:new).and_return(decorator)
+          end
 
           let(:webhook) { double("webhook") }
           let(:decorator) { double(Decorators::WebhookDecorator, to_json: json)}
           let(:json) { {some: 'json'}.to_json }
 
-          before do
-            allow(Decorators::WebhookDecorator).to receive(:new).and_return(decorator)
-          end
-
           it "finds the webhook by UUID" do
             expect(PactBroker::Webhooks::Service).to receive(:find_by_uuid).with('some-uuid')
             subject
@@ -56,14 +52,61 @@ module Resources
       end
 
       describe "PUT" do
+        before do
+          allow(Decorators::WebhookDecorator).to receive(:new).and_return(decorator)
+          allow(PactBroker::Webhooks::Service).to receive(:create).and_return(created_webhook)
+          allow_any_instance_of(Webhook).to receive(:consumer).and_return(consumer)
+          allow_any_instance_of(Webhook).to receive(:provider).and_return(provider)
+          allow_any_instance_of(Webhook).to receive(:webhook_validation_errors?).and_return(false)
+        end
+
+        let(:consumer) { double('consumer') }
+        let(:provider) { double('provider') }
+        let(:webhook) { double("webhook") }
+        let(:decorator) { double(Decorators::WebhookDecorator, from_json: parsed_webhook, to_json: json)}
+        let(:json) { {some: 'json'}.to_json }
+
+        let(:parsed_webhook) { double('parsed_webhook') }
+        let(:created_webhook) { double('created_webhook') }
+        let(:webhook) { nil }
+        let(:webhook_json) { load_fixture('webhook_valid.json') }
+        let(:uuid) { 'some-uuid' }
+
+        subject { put("/webhooks/#{uuid}", webhook_json, 'CONTENT_TYPE' => 'application/json') }
+
+        it "validates the UUID" do
+          expect_any_instance_of(Webhook).to receive(:webhook_validation_errors?).with(parsed_webhook, uuid)
+          subject
+        end
+
         context "when the webhook does not exist" do
-          let(:webhook) { nil }
-          let(:webhook_json) { load_fixture('webhook_valid.json') }
+          it "creates the webhook" do
+            expect(PactBroker::Webhooks::Service).to receive(:create).with(uuid, parsed_webhook, consumer, provider)
+            subject
+          end
 
-          subject { put '/webhooks/some-uuid', webhook_json, {'CONTENT_TYPE' => 'application/json'}; last_response }
+          its(:status) { is_expected.to eq 201 }
 
-          it "returns a 404" do
-            expect(subject).to be_a_404_response
+          it "returns the JSON respresentation of the webhook" do
+            expect(subject.body).to eq json
+          end
+        end
+
+        context "when the webhook does exist" do
+          before do
+            allow(PactBroker::Webhooks::Service).to receive(:update_by_uuid).and_return(created_webhook)
+          end
+          let(:webhook) { double('existing webhook') }
+
+          its(:status) { is_expected.to eq 200 }
+
+          it "updates the webhook" do
+            expect(PactBroker::Webhooks::Service).to receive(:update_by_uuid).with(uuid, JSON.parse(webhook_json))
+            subject
+          end
+
+          it "returns the JSON respresentation of the webhook" do
+            expect(subject.body).to eq json
           end
         end
       end

spec/lib/pact_broker/webhooks/service_spec.rb

@@ -14,9 +14,38 @@ module Webhooks
         allow(Service).to receive(:logger).and_return(logger)
       end
 
-      let(:td) { TestDataBuilder.new }
       let(:logger) { double('logger').as_null_object }
 
+      describe "validate - integration test" do
+        let(:invalid_webhook) { PactBroker::Domain::Webhook.new }
+
+        context "with no uuid" do
+          subject { Service.errors(invalid_webhook) }
+
+          it "does not contain an error for the uuid" do
+            expect(subject.messages).to_not have_key('uuid')
+          end
+        end
+
+        context "with a uuid" do
+          subject { Service.errors(invalid_webhook, '') }
+
+          it "merges the uuid errors with the webhook errors" do
+            expect(subject.messages['uuid'].first).to include "can only contain"
+          end
+        end
+      end
+
+      describe ".valid_uuid_format?" do
+        it 'does something' do
+          expect(Service.valid_uuid_format?("_-bcdefghigHIJKLMNOP")).to be true
+          expect(Service.valid_uuid_format?("HIJKLMNOP")).to be false
+          expect(Service.valid_uuid_format?("abcdefghigHIJKLMNOP\\")).to be false
+          expect(Service.valid_uuid_format?("abcdefghigHIJKLMNOP#")).to be false
+          expect(Service.valid_uuid_format?("abcdefghigHIJKLMNOP$")).to be false
+        end
+      end
+
       describe ".delete_by_uuid" do
         before do
           td.create_pact_with_hierarchy