Skip to content

Commit

Permalink
feat: apply allow_dangerous_contract_modification setting when publis…
Browse files Browse the repository at this point in the history
…hing using the /contracts/publish endpoint
  • Loading branch information
bethesque committed Oct 13, 2021
1 parent 183a77b commit 956227f
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 17 deletions.
2 changes: 1 addition & 1 deletion lib/pact_broker/api/resources/pact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def pact

def disallowed_modification?
if request.really_put? && pact_service.disallowed_modification?(pact, pact_params.json_content)
message_params = { consumer_name: pact_params.consumer_name, consumer_version_number: pact_params.consumer_version_number }
message_params = { consumer_name: pact_params.consumer_name, consumer_version_number: pact_params.consumer_version_number, provider_name: pact_params.provider_name }
set_json_error_message(message("errors.validation.pact_content_modification_not_allowed", message_params))
true
else
Expand Down
31 changes: 27 additions & 4 deletions lib/pact_broker/api/resources/publish_contracts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ def malformed_request?
end

def process_post
handle_webhook_events(consumer_version_branch: parsed_contracts.branch, build_url: parsed_contracts.build_url) do
results = contract_service.publish(parsed_contracts, base_url: base_url)
response.body = decorator_class(:publish_contracts_results_decorator).new(results).to_json(decorator_options)
if conflict_notices.any?
set_conflict_response
409
else
publish_contracts
true
end
true
end

def policy_name
Expand Down Expand Up @@ -75,6 +77,27 @@ def decode_and_parse_content(contract)
contract["decodedParsedContent"] = PactBroker::Pacts::Parse.call(contract["decodedContent"]) rescue nil
end
end

def publish_contracts
handle_webhook_events(consumer_version_branch: parsed_contracts.branch, build_url: parsed_contracts.build_url) do
results = contract_service.publish(parsed_contracts, base_url: base_url)
response.body = decorator_class(:publish_contracts_results_decorator).new(results).to_json(decorator_options)
end
end

def set_conflict_response
response.body = {
notices: conflict_notices.collect(&:to_h),
errors: {
contracts: conflict_notices.select(&:error?).collect(&:text)
}
}.to_json
response.headers["Content-Type"] = "application/json;charset=utf-8"
end

def conflict_notices
@conflict_notices ||= contract_service.conflict_notices(parsed_contracts)
end
end
end
end
Expand Down
9 changes: 2 additions & 7 deletions lib/pact_broker/contracts/contracts_publication_results.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
module PactBroker
module Contracts
ContractsPublicationResults = Struct.new(:pacticipant, :version, :tags, :contracts, :notices) do
ContractsPublicationResults = Struct.new(:pacticipant, :version, :tags, :contracts, :notices, keyword_init: true) do
def self.from_hash(params)
new(params[:pacticipant],
params[:version],
params[:tags],
params[:contracts],
params[:notices]
)
new(params)
end
end
end
Expand Down
8 changes: 8 additions & 0 deletions lib/pact_broker/contracts/notice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ def self.prompt(text)
def self.success(text)
Notice.new("success", text)
end

def self.error(text)
Notice.new("error", text)
end

def error?
type == "error"
end
end
end
end
25 changes: 25 additions & 0 deletions lib/pact_broker/contracts/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require "pact_broker/contracts/notice"
require "pact_broker/events/subscriber"
require "pact_broker/api/pact_broker_urls"
require "pact_broker/pacts/create_formatted_diff"

module PactBroker
module Contracts
Expand Down Expand Up @@ -42,6 +43,30 @@ def publish(parsed_contracts, base_url: )
)
end

def conflict_notices(parsed_contracts)
notices = []
parsed_contracts.contracts.collect do | contract_to_publish, i |
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)
end
end
notices
end

def add_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,
provider_name: contract_to_publish.provider_name
}
notices << Notice.error(message("errors.validation.pact_content_modification_not_allowed", message_params))
notices << Notice.info(PactBroker::Pacts::CreateFormattedDiff.call(new_json_content, existing_json_content))
end

private :add_conflict_notice

def create_version(parsed_contracts)
version_params = {
build_url: parsed_contracts.build_url,
Expand Down
34 changes: 32 additions & 2 deletions lib/pact_broker/doc/views/index/publish-contracts.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ This is the preferred endpoint with which to publish contracts (previously, cont

The previous tag and pact endpoints are still supported, however, future features that build on this endpoint may not be able to be backported into those endpoints.

This endpoint is designed to be used by a command line tool, and hence, the response notices are designed for output to the user in a terminal.

## Parameters

* `pacticipantName`: the name of the application. Required.
Expand All @@ -31,7 +33,7 @@ The previous tag and pact endpoints are still supported, however, future feature
### Success

* `notices`
* `level`: one of `debug`, `info`, `warning`,`prompt`,`success`
* `level`: one of `debug`, `info`, `warning`,`prompt`,`success`, `error`, `danger`
* `text`: the text of the notice. This is designed to be displayed in the output of a CLI.

The `_links` section will contain links to all the resources created by the publication. The relations are:
Expand All @@ -43,14 +45,42 @@ The `_links` section will contain links to all the resources created by the publ

### Errors

Any validation errors will be returned in the standard Pact Broker format:
### Schema validation errors

Any validation errors will be returned in the standard Pact Broker format with a 400 status:

{
"errors": {
"<fieldName>": ["message 1", "message 2"]
}
}

### Contract conflict errors

If there is a conflict with an existing published pact and `allow_dangerous_contract_modification` is set to false, a 409 will be returned with an array of notices, which will contain a diff between the existing pact content and the content that was attempted to be published. For consistency with the existing error responses, the errors hash will also contain the error messages, but there will be no diff included. For CLI usage, when there are notices and errors, just the notices should be displayed to the user.

{
"notices":
[
{
"text": "Cannot change the content of the pact for Foo version 183a77b0 and provider Bar, as race conditions will cause unreliable results for can-i-deploy. Each pact must be published with a unique consumer version number. For more information see https://docs.pact.io/go/versioning",
"type": "error"
},
{
"text": "<the diff, will include new lines>",
"type": "info"
}
],
"errors":
{
"content":
[
"Cannot change the content of the pact for Foo version 183a77b0 and provider Bar, as race conditions will cause unreliable results for can-i-deploy. Each pact must be published with a unique consumer version number. For more information see https://docs.pact.io/go/versioning"
]
}
}


## Example

POST http://broker/contracts/publish
Expand Down
2 changes: 1 addition & 1 deletion lib/pact_broker/locale/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ en:
invalid_http_method: "Invalid HTTP method '%{method}'"
invalid_url: "Invalid URL '%{url}'. Expected format: http://example.org"
pact_missing_pacticipant_name: "was not found at expected path $.%{pacticipant}.name in the submitted pact file."
pact_content_modification_not_allowed: "Cannot change the content of the pact for %{consumer_name} version %{consumer_version_number}, as race conditions will cause unreliable results for can-i-deploy. Each pact must be published with a unique consumer version number. For more information see https://docs.pact.io/go/versioning"
pact_content_modification_not_allowed: "Cannot change the content of the pact for %{consumer_name} version %{consumer_version_number} and provider %{provider_name}, as race conditions will cause unreliable results for can-i-deploy. Each pact must be published with a unique consumer version number. For more information see https://docs.pact.io/go/versioning"
consumer_version_number_missing: "Please specify the consumer version number by setting the X-Pact-Consumer-Version header."
consumer_version_number_header_invalid: "X-Pact-Consumer-Version '%{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"
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"
Expand Down
18 changes: 18 additions & 0 deletions lib/pact_broker/pacts/content.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
require "pact_broker/pacts/parse"
require "pact_broker/pacts/sort_content"
require "pact_broker/pacts/generate_interaction_sha"
require "pact_broker/hash_refinements"

module PactBroker
module Pacts
class Content
include GenerateInteractionSha
using PactBroker::HashRefinements

def initialize pact_hash
@pact_hash = pact_hash
Expand Down Expand Up @@ -75,6 +77,18 @@ def with_ids(overwrite_existing_id = true)
Content.from_hash(new_pact_hash)
end

def without_ids
new_pact_hash = pact_hash.dup
if interactions && interactions.is_a?(Array)
new_pact_hash["interactions"] = remove_ids(interactions)
end

if messages && messages.is_a?(Array)
new_pact_hash["messages"] = remove_ids(messages)
end
Content.from_hash(new_pact_hash)
end

def interaction_ids
messages_or_interaction_or_empty_array.collect do | interaction |
interaction["_id"]
Expand Down Expand Up @@ -138,6 +152,10 @@ def add_ids(interactions, overwrite_existing_id)
end
end

def remove_ids(interactions_or_messages)
interactions_or_messages.collect{ | h | h.without("_id") }
end

def merge_verification_results(interactions, tests)
interactions.collect(&:dup).collect do | interaction |
interaction["tests"] = tests.select do | test |
Expand Down
11 changes: 9 additions & 2 deletions lib/pact_broker/pacts/create_formatted_diff.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
require "pact/matchers"
require "pact_broker/json"
require "pact/matchers/unix_diff_formatter"
require "pact_broker/pacts/sort_content"
require "pact_broker/pacts/content"

module PactBroker
module Pacts
class CreateFormattedDiff

extend Pact::Matchers

def self.call pact_json_content, previous_pact_json_content
def self.call pact_json_content, previous_pact_json_content, raw: false
pact_hash = JSON.load(pact_json_content, nil, PactBroker::PACT_PARSING_OPTIONS)
previous_pact_hash = JSON.load(previous_pact_json_content, nil, PactBroker::PACT_PARSING_OPTIONS)

if !raw
pact_hash = SortContent.call(PactBroker::Pacts::Content.from_hash(pact_hash).without_ids.to_hash)
previous_pact_hash = SortContent.call(PactBroker::Pacts::Content.from_hash(previous_pact_hash).without_ids.to_hash)
end

difference = diff(previous_pact_hash, pact_hash)
Pact::Matchers::UnixDiffFormatter.call(difference, colour: false, include_explanation: false)
end
Expand Down
1 change: 1 addition & 0 deletions lib/pact_broker/pacts/sort_content.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module Pacts
class SortContent
extend OrderHashKeys

# TODO handle interactions and messages at the same time
def self.call pact_hash
key = verifiable_content_key_for(pact_hash)

Expand Down
19 changes: 19 additions & 0 deletions spec/features/publish_pact_all_in_one_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,23 @@

it { is_expected.to be_a_json_error_response("missing") }
end

context "with a conflicting pact" do
before do
allow(PactBroker.configuration).to receive(:allow_dangerous_contract_modification).and_return(false)
td.create_pact_with_hierarchy("Foo", "1", "Bar")
end

its(:status) { is_expected.to eq 409 }

it "returns notices" do
expect(JSON.parse(subject.body)["notices"]).to be_a(Array)
end

it "returns errors" do
contract_errors = JSON.parse(subject.body)["errors"]["contracts"]
expect(contract_errors).to be_a(Array)
expect(contract_errors.size).to eq 1
end
end
end
64 changes: 64 additions & 0 deletions spec/lib/pact_broker/contracts/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,70 @@ module Contracts
end
end
end

describe "#conflict_errors" do
let(:contracts_to_publish) do
ContractsToPublish.from_hash(
pacticipant_name: "Foo",
pacticipant_version_number: "1",
tags: ["a", "b"],
branch: branch,
contracts: contracts
)
end

let(:on_conflict) { "overwrite" }
let(:branch) { "main" }
let(:contracts) { [contract_1] }
let(:contract_1) do
ContractToPublish.from_hash(
consumer_name: "Foo",
provider_name: "Bar",
decoded_content: decoded_contract,
specification: "pact",
on_conflict: on_conflict
)
end

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) }

context "when a pact already exists" do
before do
allow(PactBroker.configuration).to receive(:allow_dangerous_contract_modification).and_return(allow_dangerous_contract_modification)
td.create_pact_with_hierarchy("Foo", "1", "Bar", existing_json_content)
end

let(:existing_json_content) { td.random_json_content("Foo", "Bar") }

context "when allow_dangerous_contract_modification=false and the pact content is different" do
let(:allow_dangerous_contract_modification) { false }

it "returns errors" do
expect(subject).to_not be_empty
end
end

context "when allow_dangerous_contract_modification=false and the pact content is the same" do
let(:allow_dangerous_contract_modification) { false }
let(:existing_json_content) { decoded_contract }

it { is_expected.to be_empty }
end

context "when allow_dangerous_contract_modification=true and the pact content is different" do
let(:allow_dangerous_contract_modification) { true }

it { is_expected.to be_empty }
end
end

context "when no pacts exist" do
it { is_expected.to be_empty }
end
end
end
end
end

0 comments on commit 956227f

Please sign in to comment.