pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
91 stars 46 forks source link

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

Open adamdama opened 1 year ago

adamdama commented 1 year ago

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: https://github.com/pact-foundation/pact-reference/blob/ac136ed57941fb09ee35cc34baf84da0c8d1409e/rust/pact_ffi/src/mock_server/handles.rs#L1286

[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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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