search

7digital / SevenDigital.Api.Wrapper

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

Basic json support for payloads only #155

Closed gregsochanik closed 10 years ago

gregsochanik commented 10 years ago

This PR will allow a user to specify one of two out of the box IPayloadSerializer strategies (i.e. Json and Xml). It defaults to XmlPayloadSerializer and using Newtonsoft.Json to handle the Json serialization.

I was going to try and do both Request and Response de/serialization in one PR, but they are 2 quite different animals.

This work is to just get the engine for handling json in place, rather than how we implement the client interface re issue #104

I will supply the "Accept -> Response" version if we're happy with this, which will work in a similar way, in as far as the "Accept" header mime-type will be locked down to either application/xml or application/json rather than relying on content negotiation for now. This is mainly because returning json relies on us having decorated the Schema DTOs with the correct [JsonProperty] (etc) attributes in order to deserialize as expected.

AnthonySteele commented 10 years ago

I have no objection to depending on NewtonSoft.Json. We should watch and be careful with how many and what nuget packages we depend on, but this one is necessary, proven and widely used.

I question exposing .WithPayload(payload, new JsonPayloadSerializer()) though. Something simpler like .WithJsonPayload(payload) or .WithPayload(payload, PayloadFormat.Json) would be simpler and allow us to change things under the hood a lot more without breaking the interface.

gregsochanik commented 10 years ago

Sure - shall we go with .WithPayload(payload, PayloadFormat.Json), where PayloadFormat is an enum? Seems simpler / cleaner than a separate method for each one.

AnthonySteele commented 10 years ago

I'm happy with that.

gregsochanik commented 10 years ago

Right - there we go, what do we think?

gregsochanik commented 10 years ago

All tests pass

AnthonySteele commented 10 years ago

One thought - there's no support for form-url encoded body via this mechanism, only via the post params. Do you regard that as an inconsistency or as a good thing?

gregsochanik commented 10 years ago

Well, it's a little tricky as form-url encoded has been traditionally inferred from the fact that the client has specified parameters. As Parameters is backed by a key value collection, it's a better fit than passing a complex dto.

But saying that, it would be easy enough to add a 3rd PayloadFormat and corresponding class serializing to a querystring (as long as the DTO is just a list of keys and values).

Another issue is that when signing, the Api uses the data supplied in the form-urlencoded payload to create the signature, but with xml/json payloads it just uses the url. So from the Apis perspective, they are quite different.