Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pact FFI with_body and Rust API interaction.request_json_body produce inconsistent contracts #271

Open
adamdama opened this issue Apr 28, 2023 · 3 comments

Comments

@adamdama
Copy link

I think I have found an inconsistency between pact_ffi and the rust implementation when creating sync message contracts. When using pactffi_with_body and adding a JSON body to the contract a metadata field is added with a contentType property [1]. However with the Rust implementation this property is not added [2]. Whats more is that it does not seem possible to add this metadata via the Rust SyncMessageBuilder API [2] or by using .contents_from [3]

[1] Pact FFI:

response.metadata.insert("contentType".to_string(), json!(content_type));

[2] Pact-Rust:
https://github.com/pact-foundation/pact-reference/blob/master/rust/pact_consumer/src/builders/sync_message_builder.rs#L272

[3] Pact-Rust:
https://github.com/pact-foundation/pact-reference/blob/master/rust/pact_consumer/src/builders/sync_message_builder.rs#L133

Is it necessary for the FFI to add this property or should the Rust API support configuring it? I am not sure which way round it should be but at the moment the two produced contracts are different which has mean implementing workarounds in the plugin we are creating.

Pact FFI Version: 0.4.4
Pact Consumer Version: 0.10.4

@rholshausen
Copy link
Contributor

rholshausen commented May 2, 2023

pactffi_with_body adds the property because the content type is an explicit parameter and needs to support older V3 interactions.

SyncMessageBuilder will add the content type to the body if the key pact:content-type is provided in the call to contents_from. This is different because this is a V4 interaction where the content type is now associated with the body.

We could update pactffi_with_body to only add the content type where it needs to support V3 types (i.e. SyncMessage will never be written to a V3 or before Pact), or maybe move the adding of the content type metadata to where the V3 interaction is converted.

@adamdama
Copy link
Author

adamdama commented May 12, 2023

From what you have said it sounds to me like the best option would be:

maybe move the adding of the content type metadata to where the V3 interaction is converted.

If it is being added to the metadata to ensure V3 support then at the point of conversion to V3 sounds most sensible. Otherwise if using the other approach you would then create the same discrepancy between the handling of sync message interactions and other V4 interaction types.

@rholshausen
Copy link
Contributor

I've come across this problem somewhere else, so I'm going to make SyncMessageBuilder consistent with the other behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants