[API Connection Manager] Support for non- OAuth2 APIs

[daithi-coombes]

These issues are coppied from: https://github.com/cityindex/labs.cityindex.com/issues/94

[daithi-coombes]

David Laing: What is the correct pattern for supporting a non OAuth2 API?

In specific, I'm thinking of:

The CityIndex API An OAuth1 API (specifically Dropbox)

[daithi-coombes]

in the end we decided to drop support for non oauth2 providers, it shouldn't be too much to add the support though.

Initially to declare a non oauth2 provider by declaring in the module file:

$service = array(
    ...
);

and for oauth2

$oauth2 = array(
    ...
);

The API class then would test the availability of the vars when the module file was loaded to see if it was an oauth2 or other:

require_once( $module_index_file );

//non oauth2
if(isset($service)){
    ...
);

//oauth2
elseif(isset($oauth2)){
    ...
);

//error
else{
    throw/print error
}

[daithi-coombes]

David Laing:

This technique of extension via magic arrays seems wrong to me.

Rather, wouldn't it make sense to use standard OO method overrides to enable changing of base API_Connection_Manager functionality?

As an example, look at how HybridAuth handles extensions for different APIs, specifically for GitHub

class Hybrid_Providers_GitHub extends Hybrid_Provider_Model_OAuth2
{ 
    // default permissions  
    // (no scope) => public read-only access (includes public user profile info, public repo info, and gists).
    public $scope = "";

    /**
    * IDp wrappers initializer 
    */
    function initialize() 
    {
        parent::initialize();

        // Provider api end-points
        $this->api->api_base_url  = "https://api.github.com/";
        $this->api->authorize_url = "https://github.com/login/oauth/authorize";
        $this->api->token_url     = "https://github.com/login/oauth/access_token";
    }

    /**
    * load the user profile from the IDp api client
    */
    function getUserProfile()
    {
        $data = $this->api->api( "user" ); 

        if ( ! isset( $data->id ) ){
            throw new Exception( "User profile request failed! {$this->providerId} returned an invalide response.", 6 );
        }

        $this->user->profile->identifier  = @ $data->id; 
        $this->user->profile->displayName = @ $data->name;

        ....[snip]...

        return $this->user->profile;
    }
}

The big advantage to this technique IMHO (besides the fact that it makes it clearer what each of the overridden methods are doing) is that you have a place to add some additional helper methods (ie, extensions to request) for a specific service.

[daithi-coombes]

There were two main reasons that I stuck with arrays: 1) as php is traditionally a scripting language arrays would be more familiar to 100% of programmers from beginners to experts 2) the second was the still debated myth that php handles arrays better than objects.

The only main advantage to using extended classes would be astethics as all thats needed in the module file is parameters, getting user info etc and other calls to a provider are currently made using:

$res = $this->api->request($dto->slug, array(
    'uri' => "https://www.googleapis.com/oauth2/v1/userinfo?access_token={$dto->access_token}",
    'headers' => array(
        'Authorization' => "Bearer {$dto->access_token}"
    ),
    'method' => 'get',
    'access_token' => $dto->access_token
));

or at a minimum:

$res = $this->api->request($dto->slug, array(
    'uri' => "https://graph.facebook.com/me?access_token={$dto->access_token}",
    'method' => 'get',
    'access_token' => $dto->access_token
));

so the current architecture the module file only holds the parameters, the api only logs into the provider and the dev's plugin then sets the params for the providers services it wants to access.

My preference would be to use a class architecture for the module file but from an astethics pov only.

[daithi-coombes]

Using the array() extension mechanism I'm struggling to figure out how to override the functionality of one of the base methods; for example $api->request().

Take for example a non OAuth2 API, such as Dropbox (OAuth1), Twitter (OAuth Echo) or Mailchimp (OAuth - custom)

In all these cases there is no way for me to override the functionality of the request() method by just changing some of the array values. Every time you extend the allowed values in the array to handle another variation on the request() method functionality, another if() block gets stuck into the base request() method.

In the long run:

1. That request() method is going to become a really complex mess
2. Anyone writing an extension module is forced to get you (as the base request() method maintainer) to make a change to the base method; which is a hassle for all involved.

Rather than debate this at the philosophic level; how about we implement some different using both methods APIs to give us some more data to help make a decision.

Using the current API_Connection_Manager array extension mechanism, please could you implement modules for:

1. Dropbox (OAuth 1) - showing how to get a list of files
2. Twitter (Oauth echo) - showing how to get a list of tweets
3. CIAPI (custom auth) - showing how to get a list of trades

I will do the same using HybridAuth (the OO extension technique); and then we can compare to to implementations.

Sound like a plan?

[daithi-coombes]

yep sounds good

[mrdavidlaing]

Some samples built on top of HybridAuth / wordpress-social-login

1. Dropbox - note how I was able to handle passing an extra parameter oauth_callback to Dropbox's authorize method by overriding loginBegin()
2. Twitter - interestingly HybridAuth seems to take Twitter's implementation of OAuth1 and the base standard, so there is no need to override loginBegin() or loginFinish() from the base class

What stands out for me as successful with the OO extension technique is that all the subtle variation has been accommodated for each API at the loginFinish() step, yet that base loginFinish() method hasn't become cluttered with lots of "special case" conditionals.

Also, I was able to create my Dropbox extension without having to change anything in the base Provider_Model_OAuth1.php class; thus I wasn't worried about breaking any of the other providers. Quite a nice example of the Open Closed principal at work

[mrdavidlaing]

Its worth noting that at the same time as I used the OO extension technique to add a Dropbox provider class; I also used the array() extension mechanism to add an additional block to the Provider's settings screen Its worth noting that in the very simple case of just needing to collect an additional 2 fields of data, the array extension mechanism was the better of the two.

[mrdavidlaing]

For completeness; here is some code showing how another plugin can use the Twitter & Dropbox APIs

$twitter_api = $this->get_api_adapter("Twitter");
if (is_wp_error($twitter_api))
{
    echo "<a href='".$twitter_api->get_error_data()."' target='_blank'>Connect to Twitter</a>";
} else {
    $tweets = $twitter_api->api()->get('statuses/user_timeline.json');

An excerpt from a more complete example

[daithi-coombes]

@ryanholder @mrdavidlaing Think this can definitely be closed...

we are using class inheritance now that allows the overwriting of methods. https://github.com/david-coombes/api-connection-manager/blob/master/class-api-con-mngr-module.php