pact-net
pact-net copied to clipboard
Feat multipart file upload support
This PR aims to add partial support for Multipart/form-data requests to be added to the Pact File. It introduces a WithMultipartSingleFileUpload() method that allows the user to to specify a request of multipart/form-data in which a single part is uploaded, which is a file.
This functionality is a common use case and uses the underlying Rust FFI method designed for this express purpose. Note this update does not enable Pact-Net to fully support all types of multipart/form-data requests as it does not support requests with multiple parts included.
see issue: https://github.com/pact-foundation/pact-net/issues/410
TODO
- [x] Add documentation for the new method
Nice, I'll give this a review later on 👍
In addition to the comments, I'd like to see an additional test for the verifier side so that we can confirm the entire round-trip works.
@adamrodger I wasn't 100% sure where to add these but I have added tests into Samples/EventAPI consumer and provider to generate and verify a pact file with this type of body. It includes the API endpoint used. let me know what you think or if you had something else in mind :)
Any estimate on when this will be completed?
No, the issue is upstream here: https://github.com/pact-foundation/pact-reference/issues/171.
If you are willing to help we'd appreciate it.
Will need to split out these changes but just wanted to say I revisited this, with some updates to pact-ref which are proposed here for consistent cross platform payload matching - https://github.com/pact-foundation/pact-reference/pull/429
continuation of this PR https://github.com/YOU54F/pact-net/pull/1
also adds in linux arm64 + musl arm64/x64 targets and slimmed binaries and tests them with docker (exception for arm64, which is tested locally as dotnet doesn't support running under qemu), so I need to pull those out. Just wanted to show that we are getting to a better place and hopefully closer to getting this merged.
Thanks @Inksprout for starting it off! Hope you are doing well.
Thanks for the review so far and concerns raised @adamrodger
I think this PR should be closed and an RFC raised with a proposed new API so we can discuss. Nobody likes pushing up a PR only to be told the API won't work or something, so let's get agreement on that first.
I think I started outlining a potential API in the feature request issue for this, so we can carry on the discussion there. The important thing is that there needs to be some kind of fluent builder to allow you to add multiple parts with independent content types and that needs to fit into the existing typestate pattern to prevent you from then being able to set any other body type.
I think this PR should be closed and an RFC raised with a proposed new API so we can discuss.
Yeah that is fine with me, I'm not attached to this API in particular, I just used it as a test bed to test out the content type matching in order to get consistent behaviour across platforms and archs
It would be nice to thank the original contributor for their attempt however, when it is closed.
Nobody likes pushing up a PR only to be told the API won't work or something, so let's get agreement on that first.
I don't think that is a universal statement, I don't mind at all. I tend to experiment on my own and propose something via code, and take from that the learning of getting up to that point, rather than heavily investing in proposing something relatively concrete