7digital/SevenDigital.Api.Wrapper — pull request #114 by mattgray (2014-01-03 17:09:07+00:00)
Title: Crenna oauth

Hello, Dan W and I did a refactor so that we use the Crenna lib to do OAuth (just gets so far)

This PR contains quite a few unrelated changes, and it is huge, so it's just for review - we would redo some of these changes as smaller commits in future (the renames and test changes)

We stumbled on to a lot of issues and muddling of concepts when making this change:

IHttpClient
---------------

We didn't want to change the HttpClient at all, because user of the wrapper can supply their own implementations. However, having HttpClient.Post take a parameter dictionary rather than a simple string post body is messy and leads to confusion around whether the parameters should be encoded or not inside HttpClient

RequestData
-------------------
We didn't want to change RequestData at all, because it's used as the cache key, and wrapper users supply their own cache implementation. However, RequestData conflates user-supplied data (parameters, tokens, etc) with data from the schema.

ConstructEndpoint being public
---------------------------------------------
The method RequestHandler.ConstructEndpoint should be private, having tests call that seems smelly to me.

ConstructEndpoint returns url with parameters
---------------------------------------------------------------------------
It's _much easier_ to do OAuth when you keep the request URI and the query parameters separate until the last possible moment. So we renamed it to ApiRequest, which has the two things separate. This could be taken further, I think even the notion of different request handlers for HTTP Verbs is a bit faulty, there is no need to keep them separate, because the process of signing them is more or less the same (the only difference being whether the form-urlencoded parameters are appended to the URI as a querystring or written to the HTTP request body)

anyway.. I think that's enough for now! Be keen to talk this through with some people next week / whenever!