REST Refactoring

[progmem]

The following PR is essentially a more thorough rework of #134 with the following in mind:

• Additional work has been done to the test suite to verify that OAuth2 is being tested alongside RestClient, as the previous test suite only hit RestClient
• client.rb has had a separation of responsibilities, allowing for REST verb handling by way of the subclasses in client/rest_providers
• The logic for RestClient and OAuth2::AccessToken have been separated out and reimplemented into their own subclasses under FHIR::Client::RestProviders
• An additional FHIR::Client::Errors subclass has been added, containing a NotImplemented exception along with a 1-for-1 mapping of the Net::HTTPResponse response codes into FHIR::Client::Errors codes, allowing HTTP response codes to be raised.
  • As an example, the Net::HTTPResponse::HTTPUnauthorized constant maps to the raise-able FHIR::Client::Errors::HTTPUnauthorized

As with #134, this also contains OAuth2::AccessToken refreshing logic on both FHIR::Client::Errors::HTTPUnauthorized as well as pre-emptive refreshing if the token is currently in a known-expired state.

[progmem]

From @radamson on #134, in order to migrate discussion:

Awesome @progmem. I haven't had a chance to go through the refactoring work you've done, but this type of work is something we've been talking about for a while so I'm happy to see it!

I'm ok with opening a new PR for that work so we can have a place to host discussion, but it might be a little preemptive to close this one considering the scope creep (although I suppose we could always reopen it). I'll probably have a better sense once I've had a chance to read through you're changes.

One thing that might be worth thinking about in the meantime, and what I'll be thinking about when I'm reviewing, is how we'll ultimately want the API and the implementation to work. I was having a discussion with @arscan and @jawalonoski who brought up some ideas that we may want to at least consider with this refactor: i.e.

• Should we continue to have both the OAuth2 and RestClient implementations under the hood? Right now they are suited to their own use cases, but it requires specialized error handling and more complicated logic as we've seen.
• Should we remove the OAuth2 client and augment or RestClient with OAuth2?
• Should we remove the RestClient and just use the OAuth2 client all the time, if possible?

Simplifying the implementation to only have a single client dependency would help alleviate some of the issues we've been seeing and (hopefully) make future maintenance easier. Thoughts on this @progmem?

I'll take a look at the refactoring work you've done and thanks again for the PRs!

With regards to having both RestClient and OAuth2 implementations, I think having both allows for certain flexibility, especially considering that someone may have a completely-private FHIR server available to them that would allow them to access it via a fixed Bearer token (e.g. API-specific passwords) or no authentication at all. The real-world use would be limited, but speaking from a development standpoint an individidual may be wanting to build a proof-of-concept that may not wish to focus on authentication/authorization initially.

One consideration to keep in mind is that the separation of RestClient and OAuth2 REST implementations revealed to me that the OAuth2 implementation is significantly simpler. I believe this is due to the response structure coming back from Faraday, the HTTP client that backs OAuth2. While I wouldn't want to remove REST-without-OAuth, I think migrating from RestClient to Faraday could be worth investigating due to the following:

• If the return object is the same as OAuth2, then we should be able to process it in the same fashion as OAuth2 does.
• If the API is the same as OAuth2, then we could completely reduce this back down so the entire implementation lives in client.rb again.