Skip to content

Commit

Permalink
feat: improve performance of publication for very large pacts by calc…
Browse files Browse the repository at this point in the history
…ulating the content SHA only once per request
  • Loading branch information
bethesque committed Jan 17, 2024
1 parent fb38b85 commit a947e40
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 42 deletions.
7 changes: 6 additions & 1 deletion lib/pact_broker/api/decorators/publish_contract_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ class PublishContractDecorator < BaseDecorator
property :provider_name
property :specification
property :content_type
property :decoded_content
property :decoded_content, setter: -> (fragment:, represented:, user_options:, **) {
represented.decoded_content = fragment
# Set the pact version sha when we set the content
represented.pact_version_sha = user_options.fetch(:sha_generator).call(fragment)
}
property :on_conflict, default: "overwrite"

end
end
end
Expand Down
10 changes: 7 additions & 3 deletions lib/pact_broker/api/resources/pact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ def from_json
subscribe(PactBroker::Integrations::EventListener.new) do
handle_webhook_events do
if request.patch? && resource_exists?
@pact = pact_service.merge_pact(pact_params)
@pact = pact_service.merge_pact(pact_params.merge(pact_version_sha: pact_version_sha))
else
@pact = pact_service.create_or_update_pact(pact_params)
@pact = pact_service.create_or_update_pact(pact_params.merge(pact_version_sha: pact_version_sha))
end
end
end
Expand Down Expand Up @@ -114,7 +114,7 @@ def pact
end

def disallowed_modification?
if request.really_put? && pact_service.disallowed_modification?(pact, pact_params.json_content)
if request.really_put? && pact_service.disallowed_modification?(pact, pact_version_sha)
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
Expand All @@ -126,6 +126,10 @@ def disallowed_modification?
def schema
api_contract_class(:put_pact_params_contract)
end

def pact_version_sha
@pact_version_sha ||= pact_service.generate_sha(pact_params.json_content)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/pact_broker/api/resources/publish_contracts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def policy_record_context
private

def parsed_contracts
@parsed_contracts ||= decorator_class(:publish_contracts_decorator).new(PactBroker::Contracts::ContractsToPublish.new).from_hash(params)
@parsed_contracts ||= decorator_class(:publish_contracts_decorator).new(PactBroker::Contracts::ContractsToPublish.new).from_hash(params, { user_options: { sha_generator: PactBroker.configuration.sha_generator } } )
end

def params
Expand Down
9 changes: 4 additions & 5 deletions lib/pact_broker/contracts/contract_to_publish.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
module PactBroker
module Contracts
ContractToPublish = Struct.new(:consumer_name, :provider_name, :decoded_content, :content_type, :specification, :on_conflict) do
# rubocop: disable Metrics/ParameterLists
def self.from_hash(consumer_name: nil, provider_name: nil, decoded_content: nil, content_type: nil, specification: nil, on_conflict: nil)
new(consumer_name, provider_name, decoded_content, content_type, specification, on_conflict)
ContractToPublish = Struct.new(:consumer_name, :provider_name, :decoded_content, :content_type, :specification, :on_conflict, :pact_version_sha, keyword_init: true) do

def self.from_hash(hash)
new(**hash)
end
# rubocop: enable Metrics/ParameterLists

def pact?
specification == "pact"
Expand Down
4 changes: 2 additions & 2 deletions lib/pact_broker/contracts/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ 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)
if existing_pact && pact_service.disallowed_modification?(existing_pact, contract_to_publish.pact_version_sha)
add_pact_conflict_notice(notices, parsed_contracts, contract_to_publish, existing_pact.json_content, contract_to_publish.decoded_content)
end
end
Expand Down Expand Up @@ -134,7 +134,7 @@ def create_pacts(parsed_contracts, base_url)
pact_params = create_pact_params(parsed_contracts, contract_to_publish)
existing_pact = pact_service.find_pact(pact_params)
listener = TriggeredWebhooksCreatedListener.new
created_pact = create_or_merge_pact(contract_to_publish.merge?, existing_pact, pact_params, listener)
created_pact = create_or_merge_pact(contract_to_publish.merge?, existing_pact, pact_params.merge(pact_version_sha: contract_to_publish.pact_version_sha), listener)
notices.concat(notices_for_pact(parsed_contracts, contract_to_publish, existing_pact, created_pact, listener, base_url))
created_pact
end
Expand Down
2 changes: 1 addition & 1 deletion lib/pact_broker/pacts/pact_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.from_path_info path_info
)
end

def self.from_request request, path_info
def self.from_request(request, path_info)
json_content = request.body.to_s
parsed_content = begin
parsed = JSON.parse(json_content, PACT_PARSING_OPTIONS)
Expand Down
23 changes: 11 additions & 12 deletions lib/pact_broker/pacts/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ module Service
extend PactBroker::Messages
extend SquashPactsForVerification

def generate_sha(json_content)
PactBroker.configuration.sha_generator.call(json_content)
end

def find_latest_pact params
pact_repository.find_latest_pact(params[:consumer_name], params[:provider_name], params[:tag], params[:branch_name])
end
Expand Down Expand Up @@ -154,10 +158,12 @@ def find_for_verification_publication(pact_params, consumer_version_selector_has
end

# Overwriting an existing pact with the same consumer/provider/consumer version number
# by creating a new revision (that is, a new PactPublication with an incremented revision number)
# Modifing pacts is strongly discouraged now, and support for it will be dropped in the next major version of the Pact Broker
def create_pact_revision params, existing_pact
logger.info("Updating existing pact publication", params.without(:json_content))
logger.debug("Content #{params[:json_content]}")
pact_version_sha = generate_sha(params[:json_content])
pact_version_sha = params.fetch(:pact_version_sha)
json_content = add_interaction_ids(params[:json_content])
update_params = { pact_version_sha: pact_version_sha, json_content: json_content }
updated_pact = pact_repository.update(existing_pact.id, update_params)
Expand All @@ -178,21 +184,20 @@ def create_pact_revision params, existing_pact

private :create_pact_revision

def disallowed_modification?(existing_pact, new_json_content)
!PactBroker.configuration.allow_dangerous_contract_modification && existing_pact && existing_pact.pact_version_sha != generate_sha(new_json_content)
def disallowed_modification?(existing_pact, new_pact_version_sha)
!PactBroker.configuration.allow_dangerous_contract_modification && existing_pact && existing_pact.pact_version_sha != new_pact_version_sha
end

# When no publication for the given consumer/provider/consumer version number exists
def create_pact params, version, provider
def create_pact(params, version, provider)
logger.info("Creating new pact publication", params.without(:json_content))
logger.debug("Content #{params[:json_content]}")
pact_version_sha = generate_sha(params[:json_content])
json_content = add_interaction_ids(params[:json_content])
pact = pact_repository.create(
version_id: version.id,
provider_id: provider.id,
consumer_id: version.pacticipant_id,
pact_version_sha: pact_version_sha,
pact_version_sha: params.fetch(:pact_version_sha),
json_content: json_content,
version: version
)
Expand All @@ -212,12 +217,6 @@ def create_pact params, version, provider

private :create_pact

def generate_sha(json_content)
PactBroker.configuration.sha_generator.call(json_content)
end

private :generate_sha

def add_interaction_ids(json_content)
Content.from_json(json_content).with_ids.to_json
end
Expand Down
8 changes: 7 additions & 1 deletion lib/pact_broker/test/test_data_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,13 @@ def create_label label_name
def publish_pact(consumer_name:, provider_name:, consumer_version_number: , tags: nil, branch: nil, build_url: nil, json_content: nil)
json_content = json_content || random_json_content(consumer_name, provider_name)
contracts = [
PactBroker::Contracts::ContractToPublish.from_hash(consumer_name: consumer_name, provider_name: provider_name, decoded_content: json_content, content_type: "application/json", specification: "pact")
PactBroker::Contracts::ContractToPublish.from_hash(
consumer_name: consumer_name,
provider_name: provider_name,
decoded_content: json_content,
content_type: "application/json",
specification: "pact",
pact_version_sha: PactBroker::Pacts::GenerateSha.call(json_content))
]
contracts_to_publish = PactBroker::Contracts::ContractsToPublish.from_hash(
pacticipant_name: consumer_name,
Expand Down
9 changes: 6 additions & 3 deletions spec/lib/pact_broker/contracts/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ module Contracts
let(:branch) { "main" }
let(:contracts) { [contract_1] }
let(:contract_1) do
ContractToPublish.from_hash(
ContractToPublish.new(
consumer_name: "Foo",
provider_name: "Bar",
decoded_content: decoded_contract,
pact_version_sha: PactBroker::Pacts::GenerateSha.call(decoded_contract),
specification: "pact",
on_conflict: on_conflict
)
Expand Down Expand Up @@ -149,17 +150,19 @@ module Contracts
let(:branch) { "main" }
let(:contracts) { [contract_1] }
let(:contract_1) do
ContractToPublish.from_hash(
ContractToPublish.new(
consumer_name: "Foo",
provider_name: "Bar",
decoded_content: decoded_contract,
specification: "pact",
on_conflict: on_conflict
on_conflict: on_conflict,
pact_version_sha: new_pact_version_sha
)
end

let(:contract_hash) { { consumer: { name: "Foo" }, provider: { name: "Bar" }, interactions: [{a: "b"}] } }
let(:decoded_contract) { contract_hash.to_json }
let(:new_pact_version_sha) { PactBroker::Pacts::GenerateSha.call(decoded_contract) }

subject { Service.conflict_notices(contracts_to_publish, base_url: "base_url") }

Expand Down
15 changes: 2 additions & 13 deletions spec/lib/pact_broker/pacts/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ module Pacts
consumer_name: "Foo",
provider_name: "Bar",
consumer_version_number: "1",
json_content: json_content
json_content: json_content,
pact_version_sha: PactBroker::Pacts::GenerateSha.call(json_content)
}
end
let(:content) { double("content") }
Expand All @@ -48,12 +49,6 @@ module Pacts
subject { Service.create_or_update_pact(params) }

context "when no pact exists with the same params" do
it "creates the sha before adding the interaction ids" do
expect(PactBroker::Pacts::GenerateSha).to receive(:call).ordered
expect(content).to receive(:with_ids).ordered
subject
end

it "saves the pact interactions/messages with ids added to them" do
expect(pact_repository).to receive(:create).with hash_including(json_content: json_content_with_ids)
subject
Expand Down Expand Up @@ -135,12 +130,6 @@ module Pacts

let(:expected_event_context) { { consumer_version_tags: ["dev"] } }

it "creates the sha before adding the interaction ids" do
expect(PactBroker::Pacts::GenerateSha).to receive(:call).ordered
expect(content).to receive(:with_ids).ordered
subject
end

it "saves the pact interactions/messages with ids added to them" do
expect(pact_repository).to receive(:update).with(anything, hash_including(json_content: json_content_with_ids))
subject
Expand Down

0 comments on commit a947e40

Please sign in to comment.