[jira] [Commented] (OLTU-98) Improvement in the OAuthAccessTokenResponse subclasses

["Sam Gorial (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OLTU-98?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13636591#comment-13636591 ] 

Sam Gorial commented on OLTU-98:
--------------------------------

Hi there; here are my thoughts on this -

Using the OAuthProviderType enum to declare custom TokenResponse classes will work as long as the client is talking to one of those pre-defined OAuthProviderType's.  But what if they just opt to go the old fashioned way w/ URLs?  The    Providers are no longer in the picture, but the end-point may still require a custom response mapping.

One possible idea is to use the Strategy pattern:  

Declare a new interface (e.g. ResponseBodyMapper) which would map the [String body] to [Map<String, Object> parameters].  We would make the default implementation follow the specification (i.e. use application/json).  The ResponseBodyMapper field should probably be declared inside OAuthClientResponse, since all 3 sub-types (token, authorization, resource) may not be spec compliant.

It would therefore be possible for clients to define and use custom ResponseBodyMappers (i.e. url-encoded, pipe-delimited plain text, whatever!), in one of 2 ways:

a) For those clients that use OAuthProviderType while building the request, then the enum would itself declare which ResponseBodyMapper to use.  Perhaps a null would signify to use the default.

b) For those clients that use the old authorizationLocation(String), I suggest we add another builder method called responseBodyMapper(Class) which would set this impl class into the client request.

There are some considerations here:

1) We are assuming that the only "customization" currently supported in OAuthAccessTokenResponse's is in the format of the body, and not in any of the getXXX() methods

2) We are implying that OAuthAccessTokenResponse becomes a concrete class again, with no children (no more need for GitHubTokenResponse), just like OAuthAuthzResponse and OAuthResourceResponse.  This should hopefully simplify things a bit.

3) The custom ResponseBodyMapper (if applicable) would be added to OAuthClientRequest by the builder (see b above)

4) In HttpClient.execute, we could possibly remove responseClass from the method signature, since all responses are of the same type now.  Same goes for OAuthClient.accessToken.

5) I would suggest renaming OAuthClientResponseFactory.createCustomResponse() to just createResponse().  Hopefully most of the time the response will NOT be custom :) and therefore we won't scare developers with this term.

All of the above was written with my limited understanding of the code-base, so please give comments and corrections where needed!!  I haven't actually verified whether this will all work when implemented (but I think it will).

Thx,
Sam
                
> Improvement in the OAuthAccessTokenResponse subclasses
> ------------------------------------------------------
>
>                 Key: OLTU-98
>                 URL: https://issues.apache.org/jira/browse/OLTU-98
>             Project: Apache Oltu
>          Issue Type: Improvement
>          Components: oauth2-client
>            Reporter: Antonio Sanso
>              Labels: newbie
>
> The  OAuthAccessTokenResponse subclasses are currently a bit disorganized.
> E.g. the Facebook example in [0] does use GitHubTokenResponse and so on...
> It would be nice to  have correct naming and proper usage.
> [0] https://cwiki.apache.org/confluence/display/OLTU/OAuth+2.0+Client+Quickstart

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira