feat: shorten length of metadata in pact URLs by using the consumer v…

…ersion id instead of number

lib/pact_broker/pacts/all_pact_publications.rb

@@ -17,6 +17,7 @@ class AllPactPublications < Sequel::Model(:all_pact_publications)
       associate(:one_to_many, :tags, :class => "PactBroker::Domain::Tag", :reciprocal => :version, :key => :version_id, :primary_key => :consumer_version_id)
       associate(:many_to_one, :pact_version, :key => :pact_version_id, :primary_key => :id)
       associate(:many_to_one, :latest_verification, :class => "PactBroker::Verifications::LatestVerificationForPactVersion", key: :pact_version_id, primary_key: :pact_version_id)
+      associate(:many_to_one, :consumer_version, :class => "PactBroker::Domain::Version", :key => :consumer_version_id, :primary_key => :id)
 
       dataset_module do
         include PactBroker::Repositories::Helpers
@@ -98,7 +99,8 @@ def to_domain_without_tags
         consumer.id = consumer_id
         provider = Domain::Pacticipant.new(name: provider_name)
         provider.id = provider_id
-        consumer_version = OpenStruct.new(
+        consumer_version_struct = OpenStruct.new(
+          id: consumer_version_id,
           number: consumer_version_number,
           order: consumer_version_order,
           pacticipant: consumer,
@@ -107,7 +109,7 @@ def to_domain_without_tags
         Domain::Pact.new(
           id: id,
           consumer: consumer,
-          consumer_version: consumer_version,
+          consumer_version: consumer_version_struct,
           provider: provider,
           consumer_version_number: consumer_version_number,
           revision_number: revision_number,

lib/pact_broker/pacts/metadata.rb

@@ -1,11 +1,14 @@
+require "pact_broker/domain/version"
+
 module PactBroker
   module Pacts
     module Metadata
       extend self
 
       MAPPINGS = [
         [:consumer_version_tags, "cvt"],
-        [:consumer_version_number, "cvn"],
+        [:consumer_version_number, "cvn"], # for old urls
+        [:consumer_version_id, "cv"],
         [:wip, "w"],
         [:consumer_version_selectors, "s"],
         [:tag, "t"],
@@ -21,11 +24,11 @@ def build_metadata_for_latest_pact(pact, selection_parameters)
         if selection_parameters[:tag]
           {
             "cvt" => [selection_parameters[:tag]],
-            "cvn" => pact.consumer_version_number
+            "cv" => pact.consumer_version.id
           }
         else
           {
-            "cvn" => pact.consumer_version_number
+            "cv" => pact.consumer_version.id
           }
         end
       end
@@ -56,7 +59,7 @@ def build_metadata_for_pact_for_verification(verifiable_pact)
               {
                 "t" => selector.tag,
                 "l" => selector.latest,
-                "cvn" => selector.consumer_version.number
+                "cv" => selector.consumer_version.id
               }.compact
             end
           }
@@ -78,7 +81,11 @@ def parse_object(object)
       def parse_hash(hash)
         hash.each_with_object({}) do | (key, value), new_hash |
           long_key = MAPPINGS.find{ |mapping| mapping.last == key }&.first
-          new_hash[long_key || key] = parse_object(value)
+          if long_key == :consumer_version_id
+            new_hash[:consumer_version_number] = PactBroker::Domain::Version.find(id: value.to_i)&.number
+          else
+            new_hash[long_key || key] = parse_object(value)
+          end
         end
       end
     end

lib/pact_broker/pacts/pact_publication.rb

@@ -112,12 +112,23 @@ def to_domain_lightweight
           )
       end
 
+      # Think we really could just use the version here.
       def to_version_domain
-        OpenStruct.new(number: consumer_version.number, pacticipant: consumer, tags: consumer_version.tags, order: consumer_version.order, branch: consumer_version.branch)
+        OpenStruct.new(
+          id: consumer_version.id,
+          number: consumer_version.number,
+          pacticipant: consumer,
+          tags: consumer_version.tags,
+          order: consumer_version.order,
+          branch: consumer_version.branch,
+          current_deployed_versions: consumer_version.current_deployed_versions,
+          current_supported_released_versions: consumer_version.current_supported_released_versions
+        )
       end
 
       def to_version_domain_lightweight
         OpenStruct.new(
+          id: consumer_version.id,
           number: consumer_version.number,
           pacticipant: consumer,
           order: consumer_version.order,

spec/features/execute_webhook_spec.rb

@@ -1,4 +1,3 @@
-require "support/test_data_builder"
 require "webmock/rspec"
 require "rack/pact_broker/database_transaction"
 
@@ -8,7 +7,6 @@
       .create_webhook(method: "POST", body: "${pactbroker.pactUrl}")
   end
 
-  let(:td) { TestDataBuilder.new }
   let(:path) { "/webhooks/#{td.webhook.uuid}/execute" }
   let(:response_body) { JSON.parse(last_response.body, symbolize_names: true)}
 
@@ -77,7 +75,7 @@
       PactBroker::Database.truncate
     end
 
-    subject { post path; last_response }
+    subject { post(path) }
 
     it "returns a 200 response" do
       expect(subject.status).to be 200

spec/features/get_provider_pacts_for_verification_spec.rb

@@ -11,8 +11,7 @@
 
   context "when the provider exists" do
     before do
-      TestDataBuilder.new
-        .create_provider("Provider")
+      td.create_provider("Provider")
         .create_consumer("Consumer")
         .create_consumer_version("0.0.1")
         .create_pact(json_content: { some: "content" }.to_json)

spec/features/publish_verification_results_and_version_spec.rb

@@ -0,0 +1,70 @@
+require 'pact_broker/domain/verification'
+
+describe "Publishing a pact verification and provider version" do
+  let(:path) { "/pacts/provider/Provider/consumer/Consumer/pact-version/#{pact.pact_version_sha}/verification-results" }
+  let(:verification_content) { load_fixture('verification.json') }
+  let(:parsed_response_body) { JSON.parse(subject.body) }
+  let(:pact) { td.pact }
+  let(:rack_env) do
+    {
+      'CONTENT_TYPE' => 'application/json',
+      'HTTP_ACCEPT' => 'application/hal+json',
+      'pactbroker.database_connector' => lambda { |&block| block.call }
+    }
+  end
+
+  subject { post path, verification_content, rack_env; last_response  }
+
+  before do
+    td.create_provider("Provider")
+      .create_consumer("Consumer")
+      .create_consumer_version("1.0.0")
+      .create_pact
+      .create_consumer_version("1.2.3")
+      .create_pact
+      .revise_pact
+  end
+
+  it "Responds with a 201 Created" do
+    expect(subject.status).to be 201
+  end
+
+  it "saves new verification" do
+    expect { subject }.to change { PactBroker::Domain::Verification.count }.by(1)
+  end
+
+  it "saves the verification against the correct pact" do
+    subject
+    expect(PactBroker::Domain::Verification.order(:id).last.pact_version_sha).to eq pact.pact_version_sha
+  end
+
+  it "saves the test results" do
+    subject
+    expect(PactBroker::Domain::Verification.order(:id).last.test_results).to eq('some' => 'results')
+  end
+
+  it "returns a link to itself that can be followed" do
+    get_verification_link = parsed_response_body['_links']['self']['href']
+    get get_verification_link, nil, { 'HTTP_ACCEPT' => 'application/hal+json' }
+    expect(last_response.status).to be 200
+    expect(JSON.parse(subject.body)).to include JSON.parse(verification_content)
+  end
+
+  context "with a webhook configured", job: true do
+    before do
+      td.create_webhook(
+        method: 'POST',
+        url: 'http://example.org',
+        events: [{ name: PactBroker::Webhooks::WebhookEvent::VERIFICATION_PUBLISHED }]
+      )
+    end
+    let!(:request) do
+      stub_request(:post, 'http://example.org').to_return(:status => 200)
+    end
+
+    it "executes the webhook" do
+      subject
+      expect(request).to have_been_made
+    end
+  end
+end

spec/fixtures/approvals/get_provider_pacts_for_verification.approved.json

@@ -22,7 +22,7 @@
     "status": 200,
     "headers": {
       "Content-Type": "application/hal+json;charset=utf-8",
-      "Content-Length": "815"
+      "Content-Length": "808"
     },
     "body": {
       "_embedded": {
@@ -39,7 +39,7 @@
             },
             "_links": {
               "self": {
-                "href": "http://example.org/pacts/provider/Provider/consumer/Consumer%202/pact-version/0f22f551a422b027066db7635cad8bd8a59ac869/metadata/c1tdW3RdPXByb2Qmc1tdW2xdPXRydWUmc1tdW2N2bl09NC41LjY",
+                "href": "http://example.org/pacts/provider/Provider/consumer/Consumer%202/pact-version/0f22f551a422b027066db7635cad8bd8a59ac869/metadata/c1tdW3RdPXByb2Qmc1tdW2xdPXRydWUmc1tdW2N2XT0y",
                 "name": "Pact between Consumer 2 (4.5.6) and Provider"
               }
             }

spec/lib/pact_broker/pacts/metadata_spec.rb

@@ -7,7 +7,7 @@ module Metadata
         let(:selectors) do
           Selectors.new([ResolvedSelector.new({ latest: true, consumer: "consumer", tag: "tag" }, consumer_version)])
         end
-        let(:consumer_version) { double("version", number: "2") }
+        let(:consumer_version) { double("version", number: "2", id: 1) }
         let(:verifiable_pact) { double("PactBroker::Pacts::VerifiablePact", wip: wip, selectors: selectors) }
         let(:wip) { false }
 
@@ -19,7 +19,7 @@ module Metadata
               {
                 "l" => true,
                 "t" => "tag",
-                "cvn" => "2"
+                "cv" => 1
               }
             ]
           })
@@ -34,38 +34,104 @@ module Metadata
       end
 
       describe "parse_metadata" do
-        let(:incoming_metadata) do
-          {
-            "cvn" => "2",
-            "cvt" => ["tag"],
-            "w" => true,
-            "s" => [
-              {
-                "l" => true,
-                "t" => "tag",
-                "cvn" => "2"
-              }
-            ]
-          }
-        end
+        context "with a consumer version id" do
+          before do
+            allow(PactBroker::Domain::Version).to receive(:find).with(id: 1).and_return(consumer_version)
+          end
+
+          let(:consumer_version) { double("version", number: "2", id: 1) }
+
+          let(:incoming_metadata) do
+            {
+              "cv" => 1,
+              "cvt" => ["tag"],
+              "w" => true,
+              "s" => [
+                {
+                  "l" => true,
+                  "t" => "tag",
+                  "cv" => 1
+                }
+              ]
+            }
+          end
 
-        let(:parsed_metadata) do
-          {
-            :consumer_version_number => "2",
-            :consumer_version_tags => ["tag"],
-            :wip => true,
-            :consumer_version_selectors => [
+          let(:parsed_metadata) do
+            {
+              :consumer_version_number => "2",
+              :consumer_version_tags => ["tag"],
+              :wip => true,
+              :consumer_version_selectors => [
+                {
+                  :latest => true,
+                  :tag => "tag",
+                  :consumer_version_number => "2"
+                }
+              ]
+            }
+          end
+
+          it "expands the key names" do
+            expect(Metadata.parse_metadata(incoming_metadata)).to eq parsed_metadata
+          end
+
+          context "when the version can't be found" do
+            let(:consumer_version) { nil }
+
+            let(:parsed_metadata) do
               {
-                :latest => true,
-                :tag => "tag",
-                :consumer_version_number => "2"
+                :consumer_version_number => nil,
+                :consumer_version_tags => ["tag"],
+                :wip => true,
+                :consumer_version_selectors => [
+                  {
+                    :latest => true,
+                    :tag => "tag",
+                    :consumer_version_number => nil
+                  }
+                ]
               }
-            ]
-          }
+            end
+
+            it "sets the consumer version number to nil" do
+              expect(Metadata.parse_metadata(incoming_metadata)).to eq parsed_metadata
+            end
+          end
         end
 
-        it "expands the key names" do
-          expect(Metadata.parse_metadata(incoming_metadata)).to eq parsed_metadata
+        context "with a consumer version number (support the old format for previously created URLs)" do
+          let(:incoming_metadata) do
+            {
+              "cvn" => "2",
+              "cvt" => ["tag"],
+              "w" => true,
+              "s" => [
+                {
+                  "l" => true,
+                  "t" => "tag"
+                }
+              ]
+            }
+          end
+
+          let(:parsed_metadata) do
+            {
+              :consumer_version_number => "2",
+              :consumer_version_tags => ["tag"],
+              :wip => true,
+              :consumer_version_selectors => [
+                {
+                  :latest => true,
+                  :tag => "tag"
+                }
+              ]
+            }
+          end
+
+          it "expands the key names" do
+            expect(Metadata.parse_metadata(incoming_metadata)).to eq parsed_metadata
+          end
+
         end
       end
     end

spec/migrations/sanity_spec.rb

@@ -0,0 +1,12 @@
+require 'pact_broker/db'
+
+RSpec.describe "the Pact Broker migrations" do
+  it "doesn't have any migrations with the same number" do
+    duplicates = Dir.glob(PactBroker::DB::MIGRATIONS_DIR + "/*")
+      .collect { |path| path.split("/").last }
+      .select  { | filename| filename =~ /^\d\d\d\d/ }
+      .group_by{ | filename | filename.split("_").first.to_i }
+      .select  { | number, filenames | filenames.size > 1 }
+    expect(duplicates).to eq({})
+  end
+end