7digital / SevenDigital.Api.Wrapper

A fluent c# wrapper for the 7digital API
MIT License
16 stars 29 forks source link

POST/PUT with a body and query string parameters #233

Closed johnjcorcoran closed 8 years ago

gregsochanik commented 8 years ago

This is great, as the current behaviour of WithParameter adding a parameter to the POST body is confusing. It's been agreed that WithParameter should refer to querystrings parameters to reflect the difference between the qs request params and the POSTed entity.

But, the problem is, this could cause a breaking change in use cases where adding the param to the formurlencoded POST body is relied on, esp if the param is not in the api qs when POSTing whitelist.

So, this will need a major version bump and a bit of explanation in the release notes.

Integrating against this new version should be fine for a majority of users, but if a user has specified WithParameter() with a parameter that's not in the whitelist, it will not work as expected.

Other than that, we need to double check that the default behaviour for POSTing an entity is that it is sent as formurlencoded before this is released.

johnjcorcoran commented 8 years ago

Looks like the default behaviour for POSTing when not explicitly providing a Content Type or Payload Format is to assume XML, based on the WithPayload overloads in FluentApi.

gregsochanik commented 8 years ago

Hmmm - that's annoying. TBH though, as this will be a major release incrementation, I'm not too bothered, we just have to let users know that they must now explicitly specify the contenttype when sending an entity. I think we should deal with ensuring the correct content type is sent as a separate Issue, just so that you can get this change out and start using it.

gregsochanik commented 8 years ago

2 tests currently failing btw:

OAuthRequestTokenTest.Can_handle_odd_characters_in_post_signing_process OAuthRequestTokenTest.Should_allow_POSTing_to_request_token_endpoint

gregsochanik commented 8 years ago

All tests passing, merge at will