PHP wrapper around an API - best practices

Question

Here's a PHP wrapper around a public OAuth2 REST API. I would love to hear your thoughts and comments. Testing was a bit frustrating since I tried for my first time. Not sure if I have done the mocking properly.

You can see the whole project on Github

class Feedly {
 private
 $_apiBaseUrl = "https://cloud.feedly.com",
 $_authorizePath = "/v3/auth/auth",
 $_accessTokenPath = "/v3/auth/token",
 $_storeAccessTokenToSession;
 /**
 * @param boolean $sandbox Enable/Disable Sandbox Mode
 * @param boolean $storeAccessTokenToSession Choose whether to store the Access token
 * to $_SESSION or not
 */
 public function __construct($sandbox=FALSE, $storeAccessTokenToSession=TRUE) {
 $this->_storeAccessTokenToSession = $storeAccessTokenToSession;
 if($this->_storeAccessTokenToSession) session_start();
 if($sandbox) $this->_apiBaseUrl = "https://sandbox.feedly.com";
 }
 /**
 * Return authorization URL
 * @param string $client_id Client's ID provided by Feedly's Administrators
 * @param string $redirect_uri Endpoint to reroute with the results
 * @param string $response_type
 * @param string $scope
 *
 * @return string Authorization URL
 */
 public function getLoginUrl ($client_id, $redirect_uri,
 $response_type="code", $scope="https://cloud.feedly.com/subscriptions") {
 return($this->_apiBaseUrl . $this->_authorizePath . "?" .
 http_build_query(array(
 "client_id"=>$client_id,
 "redirect_uri"=>$redirect_uri,
 "response_type"=>$response_type,
 "scope"=>$scope
 )
 )
 );
 }
 /**
 * Exchange a `code` got from `getLoginUrl` for an `Access Token`
 * @param string $client_id Client's ID provided by Feedly's Administrators
 * @param string $client_secret Client's Secret provided by Feedly's Administrators
 * @param string $auth_code Code obtained from `getLoginUrl`
 * @param string $redirect_url Endpoint to reroute with the results
 */
 public function GetAccessToken($client_id, $client_secret, $auth_code,
 $redirect_url) {
 $r = null;
 if (($r = @curl_init($this->_apiBaseUrl . $this->_accessTokenPath)) == false) {
 throw new Exception("Cannot initialize cUrl session.
 Is cUrl enabled for your PHP installation?");
 }
 curl_setopt($r, CURLOPT_RETURNTRANSFER, 1);
 curl_setopt($r, CURLOPT_ENCODING, 1);
 curl_setopt($r, CURLOPT_SSL_VERIFYPEER, false);
 curl_setopt($r, CURLOPT_CAINFO, "C:\wamp\bin\apache\Apache2.2.21\cacert.crt");
 // Add client ID and client secret to the headers.
 curl_setopt($r, CURLOPT_HTTPHEADER, array (
 "Authorization: Basic " . base64_encode($client_id . ":" .
 $client_secret),
 ));
 $post_fields = "code=" . urlencode($auth_code) .
 "&client_id=" . urlencode($client_id) .
 "&client_secret=" . urlencode($client_secret) .
 "&redirect_uri=" . urlencode($redirect_url) .
 "&grant_type=authorization_code";
 curl_setopt($r, CURLOPT_POST, true);
 curl_setopt($r, CURLOPT_POSTFIELDS, $post_fields);
 $response = curl_exec($r);
 $http_status = curl_getinfo($r, CURLINFO_HTTP_CODE);
 $tmpr = json_decode($response, true);
 curl_close($r);
 if($http_status!==200)
 throw new Exception("Response from API: " . $tmpr['errorMessage']);
 if($this->_storeAccessTokenToSession){
 if(!isset($_SESSION['access_token'])){
 $_SESSION['access_token'] = $tmpr['access_token'];
 session_write_close();
 }
 }
 return $response;
 }
 /**
 * cUrl Initiliazation
 * @param string $url URL to query
 * @param string $token Access Token in case we don't store it to $_SESSION
 */
 private function InitCurl($url, $token=NULL) {
 $r = null;
 if (($r = @curl_init($url)) == false) {
 throw new Exception("Cannot initialize cUrl session.
 Is cUrl enabled for your PHP installation?");
 }
 curl_setopt($r, CURLOPT_RETURNTRANSFER, 1);
 curl_setopt($r, CURLOPT_ENCODING, 1);
 curl_setopt($r, CURLOPT_SSL_VERIFYPEER, false);
 curl_setopt($r, CURLOPT_CAINFO, "C:\wamp\bin\apache\Apache2.2.21\cacert.crt");
 $access_token = is_null($token) ? $this->_getAccessTokenFromSession() : $token;
 curl_setopt($r, CURLOPT_HTTPHEADER, array (
 "Authorization: OAuth " . $access_token
 ));
 return($r);
 }
 /**
 * Query a URL with GET using cUrl after initialization
 * @param string $url URL to query
 * @param string $get_params Parameters to pass to URL as GET params
 * @param string $token Access Token in case we don't store it to $_SESSION
 */
 public function ExecGetRequest($url, $get_params=NULL, $token=NULL) {
 $url = $this->_apiBaseUrl . $url;
 if(is_array($get_params))
 $r = $this->InitCurl($url .'?',
 http_build_query($url, $get_params), $token);
 else
 $r = $this->InitCurl($url, $token);
 $response = curl_exec($r);
 if ($response == false) {
 die("curl_exec() failed. Error: " . curl_error($r));
 }
 $http_status = curl_getinfo($r, CURLINFO_HTTP_CODE);
 $tmpr = json_decode($response, true);
 curl_close($r);
 if($http_status!==200)
 throw new Exception("Something went wrong: " . $tmpr['errorMessage']);
 else
 return $response;
 }
 /**
 * Make a POST request
 * @param string $url URL to query
 * @param string $get_params Parameters to pass to URL as GET params
 * @param string $post_params Parameters to pass to URL as POST params
 * @param string $token Access Token in case we don't store it to $_SESSION
 */
 public function ExecPostRequest($url, $get_params=NULL, $post_params=NULL, $token=NULL) {
 $url = $this->_apiBaseUrl . $url;
 if(is_array($get_params))
 $r = $this->InitCurl($url .'?',
 http_build_query($url, $get_params), $token);
 else
 $r = $this->InitCurl($url, $token);
 $post_fields = http_build_query($post_params);
 curl_setopt($r, CURLOPT_POST, true);
 curl_setopt($r, CURLOPT_POSTFIELDS, $post_fields);
 $response = curl_exec($r);
 if ($response == false) {
 die("curl_exec() failed. Error: " . curl_error($r));
 }
 $http_status = curl_getinfo($r, CURLINFO_HTTP_CODE);
 $tmpr = json_decode($response, true);
 curl_close($r);
 if($http_status!==200)
 throw new Exception("Something went wrong: " . $tmpr['errorMessage']);
 else
 return $response;
 }
 /**
 * @see http://developer.feedly.com/v3/profile/#get-the-profile-of-the-user
 * @param string $token Access Token in case we don't store it to $_SESSION
 * @return json Response from the server
 */
 public function getProfile($token=NULL) {
 return $this->ExecGetRequest('/v3/profile', NULL, $token);
 }
 /**
 * @see http://developer.feedly.com/v3/profile/#update-the-profile-of-the-user
 * @param string $email
 * @param string $givenName
 * @param string $familyName
 * @param string $picture
 * @param boolean $gender
 * @param string $locale
 * @param string $reader google reader id
 * @param string $twitter twitter handle. example: edwk
 * @param string $facebook facebook id
 * @param string $token Access Token in case we don't store it to $_SESSION
 * @return json Response from the server
 */
 public function setProfile($token=NULL, $email=NULL, $givenName=NULL, $familyName=NULL,
 $picture=NULL, $gender=NULL, $locale=NULL,
 $reader=NULL, $twitter=NULL, $facebook=NULL) {
 return $this->ExecPostRequest('/v3/profile', NULL, array(
 'email'=>$email,
 'givenName'=>$givenName,
 'familyName'=>$familyName,
 'picture'=>$picture,
 'gender'=>$gender,
 'locale'=>$locale,
 'reader'=>$reader,
 'twitter'=>$twitter,
 'facebook'=>$facebook
 ), $token);
 }
 /**
 * @see http://developer.feedly.com/v3/preferences/#get-the-preferences-of-the-user
 * @param string $token Access Token in case we don't store it to $_SESSION
 * @return json Response from the server
 */
 public function getPreferences($token=NULL) {
 return $this->ExecGetRequest('/v3/preferences', NULL, $token);
 }
 /* More code happens here, check the full version 
 *https://github.com/stakisko/feedly-api/blob/master/feedly.php 
 */
 /**
 * @return string Access Token from $_SESSION
 */
 protected function _getAccessTokenFromSession(){
 if(isset($_SESSION['access_token'])){
 return $_SESSION['access_token'];
 }else {
 throw new Exception("No access token", 1);
 }
 }
}
<?php
include_once './feedly.php';
include_once './vendor/autoload.php';
class FeedlyAPITest extends PHPUnit_Framework_TestCase
{
 private $instance;
 function __construct(){
 ini_set("session.use_cookies", 0);
 $this->instance = new Feedly(true, false);
 }
 /**
 * Test valid returned URL for authorization
 */
 public function testGetLoginURL()
 {
 $this->assertNotEmpty($this->instance->getLoginUrl("sandbox", "http://localhost"));
 }
 /**
 * Testing GetAccessToken on failure
 * will throw exception
 */
 public function testGetAccessTokenThrowsExceptionOnFailure()
 {
 try {
 $this->instance->GetAccessToken();
 } catch (Exception $expected) {
 return;
 }
 $this->fail();
 }
 /**
 * Testing GetAccessToken
 */
 public function testGetAccessToken()
 {
 $json = '
 {
 "access_token": 1385150462,
 "stuff": {
 "this": 2,
 "that": 4,
 "other": 1
 }
 } ';
 $feedly = $this->getMock('Feedly', array('GetAccessToken'), array(true, false));
 $feedly->expects($this->any())
 ->method('GetAccessToken')
 ->will($this->returnValue($json));
 $this->assertEquals($json, $feedly->GetAccessToken("sandbox", "FUFNPXDNP2J0BF7RCEUZ", "", "http://localhost"));
 }
 /**
 * Testing a GET Request to API without providing an Access Token
 * will throw exception
 */
 public function testExecGetRequestWithoutAccessTokenThrowsException(){
 try {
 $this->instance->ExecGetRequest('/v3/profile');
 } catch (Exception $expected) {
 return;
 }
 $this->fail();
 }
 /**
 * Testing a GET Request to API
 */
 public function testExecGetRequest(){
 $json = '
 {
 "access_token": 1385150462,
 "stuff": {
 "this": 2,
 "that": 4,
 "other": 1
 }
 } ';
 $feedly = $this->getMock('Feedly', array('ExecGetRequest'), array(true, false));
 $feedly->expects($this->any())
 ->method('ExecGetRequest')
 ->will($this->returnValue($json));
 $this->assertEquals($json, $feedly->ExecGetRequest("/dum/url"));
 }
 /**
 * Testing a POST Request to API
 */
 public function testExecPostRequest(){
 $json = '
 {
 "access_token": 1385150462,
 "stuff": {
 "this": 2,
 "that": 4,
 "other": 1
 }
 } ';
 $feedly = $this->getMock('Feedly', array('ExecPostRequest'), array(true, false));
 $feedly->expects($this->any())
 ->method('ExecPostRequest')
 ->will($this->returnValue($json));
 $this->assertEquals($json, $feedly->ExecPostRequest('/dummy/url', NULL, array(
 'email'=>'[email protected]',
 'givenName'=>''
 )));
 }
}

• Elias Van Ootegem: Please, Please subscribe to the, as of yet, unofficial coding standards as described here. The more people adopt these, the better. Symfony, Zend, Doctrine, PEAR,... all subscribe to these conventions

Answer 1

Two minor notes:

1. The following is not too easy to read:

   curl_setopt($r, CURLOPT_ENCODING, 1);
   

   Readers have to be really familar with curl parameters or have to check the documentation. It says the following:

   The contents of the "Accept-Encoding: " header. This enables decoding of the response. Supported encodings are "identity", "deflate", and "gzip". If an empty string, "", is set, a header containing all supported encoding types is sent.

   But it's 1, which isn't a string. 1 == ""? I suggest the following:

   curl_setopt($r, CURLOPT_ENCODING, ALL_SUPPORTED_ENCODING_TYPES);
   

   where ALL_SUPPORTED_ENCODING_TYPES is a constant with an empty string value. (I suppose 1 == "".)

2. Comments like this is unnecessary:

   /**
    * Testing GetAccessToken
    */
   public function testGetAccessToken()
   

   The function name says the same, so I'd remove the comment. In the following case I'd rename the function to testValidReturnedUrlForAuthorization() and remove the comment:

    /**
    * Test valid returned URL for authorization
    */
   public function testGetLoginURL()
   

   (Clean Code by Robert C. Martin: Chapter 4: Comments, Noise Comments)

• kbariotis: You have absolutely right. I ve missed that! Fixed. Thank you.