Webservice client unit tests and fixes for bugs discovered while creating them

.gitignore

@@ -3,3 +3,6 @@ composer.lock
 composer.phar
 phpunit.xml
 vendor/
+*.sw?
+t.php
+*.old

phpunit.xml.dist

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<phpunit bootstrap="./tests/bootstrap.php" colors="true">
+    <testsuites>
+        <testsuite name="GeoIP2 Test Suite">
+            <directory suffix="Test.php">./tests/GeoIP2/Test/</directory>
+        </testsuite>
+    </testsuites>
+
+    <filter>
+        <whitelist>
+            <directory suffix=".php">./src/GeoIP2/</directory>
+        </whitelist>
+    </filter>
+</phpunit>

src/GeoIP2/Exception/GenericException.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace GeoIP2\Exception;
+
+class GenericException extends \Exception
+{
+}

src/GeoIP2/Exception/HttpException.php

@@ -4,11 +4,13 @@ namespace GeoIP2\Exception;
 
 class HttpException extends \Exception
 {
+  public $code;
 
   public function __construct($message, $code, $uri,
                               Exception $previous = null)
   {
-    parent::__construct($message, $code, $previous);
+    $this->code = $code;
+    parent::__construct($message, null, $previous);
   }
 
 }

src/GeoIP2/Exception/WebserviceException.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace GeoIP2\Exception;
+
+class WebserviceException extends HttpException
+{
+  public $httpStatus;
+
+  public function __construct($message, $code, $httpStatus, $uri,
+                              Exception $previous = null)
+  {
+    $this->httpStatus = $httpStatus;
+    parent::__construct($message, $code, $uri, $previous);
+  }
+
+}

src/GeoIP2/Model/CityIspOrg.php

@@ -2,7 +2,7 @@
 
 namespace GeoIP2\Model;
 
-class CityISPOrg extends City
+class CityIspOrg extends City
 {
 
 }

src/GeoIP2/Model/Omni.php

@@ -2,7 +2,7 @@
 
 namespace GeoIP2\Model;
 
-class Omni extends CityISPOrg
+class Omni extends CityIspOrg
 {
 
 }

src/GeoIP2/Webservice/Client.php

@@ -2,14 +2,17 @@
 
 namespace GeoIP2\Webservice;
 
-use GeoIP2\Error\Generic;
+use GeoIP2\Exception\GenericException;
 use GeoIP2\Exception\HttpException;
-use GeoIP2\Error\Webservice;
+use GeoIP2\Exception\WebserviceException;
 use GeoIP2\Model\City;
-use GeoIP2\Model\CityISPOrg;
+use GeoIP2\Model\CityIspOrg;
 use GeoIP2\Model\Country;
 use GeoIP2\Model\Omni;
 use Guzzle\Http\Client as GuzzleClient;
+use Guzzle\Common\Exception\RuntimeException;
+use Guzzle\Http\Exception\ClientErrorResponseException;
+use Guzzle\Http\Exception\ServerErrorResponseException;
 
 class Client
 {
@@ -18,12 +21,16 @@ class Client
   private $licenseKey;
   private $languages;
   private $baseUri = 'https://geoip.maxmind.com/geoip/v2.0';
+  private $guzzleClient;
 
-  public function __construct($userId, $licenseKey, $languages=array('en'))
+  public function __construct($userId, $licenseKey, $languages=array('en'),
+                              $guzzleClient = null)
   {
     $this->userId = $userId;
     $this->licenseKey = $licenseKey;
     $this->languages = $languages;
+    // To enable unit testing
+    $this->guzzleClient = $guzzleClient;
   }
 
   public function city($ipAddress = 'me')
@@ -36,9 +43,9 @@ class Client
     return $this->responseFor('country', 'Country', $ipAddress);
   }
 
-  public function cityISPOrg($ipAddress = 'me')
+  public function cityIspOrg($ipAddress = 'me')
   {
-    return $this->responseFor('city_isp_org', 'CityISPOrg', $ipAddress);
+    return $this->responseFor('city_isp_org', 'CityIspOrg', $ipAddress);
   }
 
   public function omni($ipAddress = 'me')
@@ -50,67 +57,69 @@ class Client
   {
     $uri = implode('/', array($this->baseUri, $path, $ipAddress));
 
-    $client = new GuzzleClient();
+    $client = $this->guzzleClient ? $this->guzzleClient : new GuzzleClient();
     $request = $client->get($uri, array('Accept' => 'application/json'));
     $request->setAuth($this->userId, $this->licenseKey);
     $ua = $request->getHeader('User-Agent');
     $ua = "GeoIP2 PHP API ($ua)";
     $request->setHeader('User-Agent', $ua);
 
+    $response = null;
+    try{
       $response = $request->send();
+    }
+    catch (ClientErrorResponseException $e) {
+      $this->handle4xx($e->getResponse(), $uri); 
+    }
+    catch (ServerErrorResponseException $e) {
+      $this->handle5xx($e->getResponse(), $uri); 
+    }
 
-    if ($response->isSuccessful()) {
+    if ($response && $response->isSuccessful()) {
       $body = $this->handleSuccess($response, $uri);
       $class = "GeoIP2\\Model\\" . $class;
       return new $class($body, $this->languages);
     }
+    else {
+      $this->handleNon200($response, $uri);
+    }
   }
 
   private function handleSuccess($response, $uri)
   {
-    // XXX - handle exceptions
+    if ($response->getContentLength() == 0) {
+      throw new GenericException("Received a 200 response for $uri but did not receive a HTTP body.");
+    }
+
     try {
       return $response->json();
     }
-    // XXX - figure out what sort of exception to catch
+    catch (RuntimeException $e) {
-    catch (Exception $e) {
       throw new GenericException("Received a 200 response for $uri but could not decode the response as JSON: " . $e->getMessage());
 
     }
   }
 
-  private function handleError($response, $uri)
+  private function handle4xx($response, $uri)
   {
     $status = $response->getStatusCode();
 
-    if ($status >= 400 && $status <= 499) {
+    $body = array();
-      $this->handle4xx($response, $uri);
-    }
-    elseif ($status >= 500 && $status <= 599){
-      $this->handle5xx($response, $uri);
-    }
-    else {
-      $this->hanldeNon200($reponse, $uri);
-    }
-  }
 
-  private function handle4xx($response, $uri)
-  {
     if ( $response->getContentLength() > 0 ) {
       if( strstr($response->getContentType(), 'json')) {
         try {
           $body = $response->json();
-          if (!$body['code'] || $body['error'] ){
+          if (!isset($body['code']) || !isset($body['error']) ){
-            throw new GenericException('Response contains JSON but it does not specify code or error keys');
+            throw new GenericException('Response contains JSON but it does not specify code or error keys: ' . $response->getBody());
           }
         }
-        // XXX - don't catch all exceptions
+        catch (RuntimeException $e){
-        catch (Exception $e){
           throw new HttpException("Received a $status error for $uri but it did not include the expected JSON body: " . $e->getMessage(), $status, $uri);
         }
       }
       else {
-        throw new HttpException("Received a $status error for $uri with the following body: $content",
+        throw new HttpException("Received a $status error for $uri with the following body: " . $response->getBody(),
                                 $status, $uri);
       }
     }
@@ -119,17 +128,21 @@ class Client
                               $status, $uri);
     }
 
-    throw new WebserviceException($body['error'], $status, $uri);
+    throw new WebserviceException($body['error'], $body['code'], $status, $uri);
   }
 
   private function handle5xx($response, $uri)
   {
+    $status = $response->getStatusCode();
+
     throw new HttpException("Received a server error ($status) for $uri",
                             $status,$uri);
   }
 
   private function handleNon200($response, $uri)
   {
+    $status = $response->getStatusCode();
+
     throw new HttpException("Received a very surprising HTTP status " .
                             "($status) for $uri",
                             $status, $uri);

tests/GeoIP2/Test/Webservice/ClientTest.php

@@ -0,0 +1,307 @@
+<?php
+
+namespace GeoIP2\Test\Webservice;
+
+use GeoIP2\Webservice\Client;
+use Guzzle\Http\Client as GuzzleClient;
+use Guzzle\Http\Message\Response;
+use Guzzle\Plugin\Mock\MockPlugin;
+
+class ClientTest extends \PHPUnit_Framework_TestCase
+{
+
+  private $country 
+    = array(
+            'continent' => array(
+                                 'continent_code' => 'NA',
+                                 'geoname_id'     => 42,
+                                 'names'          => array( 'en' => 'North America' ),
+                                 ),
+            'country' => array(
+                               'geoname_id' => 1,
+                               'iso_code'   => 'US',
+                               'names'      => array( 'en' => 'United States of America' ),
+                               ),
+            'traits' => array(
+                              'ip_address' => '1.2.3.4',
+                              ),
+            );
+
+
+  private function getResponse($ip) {
+    $responses = array(
+                       '1.2.3.4' => $this->response(
+                                                    'country',
+                                                    200,
+                                                    $this->country
+                                                    ),
+                       'me' => $this->response(
+                                               'country',
+                                               200,
+                                               $this->country
+                                               ),
+                       '1.2.3.5' => $this->response('country', 200),
+                       '2.2.3.5' => $this->response('country', 200, 'bad body'),
+                       '1.2.3.6'=> $this->response(
+                                                   'error', 400,
+                                                   array(
+                                                         'code'  => 'IP_ADDRESS_INVALID',
+                                                         'error' => 'The value "1.2.3" is not a valid ip address'
+                                                         )
+                                                   ),
+                       '1.2.3.7' => $this->response(
+                                                    'error',
+                                                    400
+                                                    ),
+                       '1.2.3.8' => $this->response(
+                                                    'error',
+                                                    400,
+                                                    array( 'weird' => 42 )
+                                                    ),
+                       '1.2.3.9' => $this->response(
+                                                    'error',
+                                                    400,
+                                                    null,
+                                                    'bad body'
+                                                    ),
+                       '1.2.3.10' => $this->response(
+                                                     null,
+                                                     500
+                                                     ),
+                       '1.2.3.11' => $this->response(
+                                                     null,
+                                                     300
+                                                     ),
+                       '1.2.3.12' => $this->response(
+                                                     'error',
+                                                     406,
+                                                     'Cannot satisfy your Accept-Charset requirements',
+                                                     null,
+                                                     'text/plain'
+                                                     ),
+                       );
+    return $responses[$ip];
+  }
+
+  public function testCountry() {
+    $country = $this->client($this->getResponse('1.2.3.4'))->country('1.2.3.4' );
+
+    $this->assertInstanceOf('GeoIP2\Model\Country', $country);
+
+    $this->assertEquals(42, $country->continent->geonameId,
+                        'continent geoname_id is 42');
+
+    $this->assertEquals('NA',  $country->continent->continentCode,
+                        'continent continent_code is NA');
+
+    $this->assertEquals(array('en' => 'North America'),
+                        $country->continent->names, 'continent names');
+
+    $this->assertEquals('North America', $country->continent->name,
+                        'continent name is North America');
+
+    $this->assertEquals(1, $country->country->geonameId,
+                        'country geoname_id is 1');
+
+    $this->assertEquals('US', $country->country->isoCode,
+                        'country iso_code is US');
+
+    $this->assertEquals(array( 'en' => 'United States of America' ),
+                        $country->country->names, 'country names');
+
+    $this->assertEquals('United States of America',
+                        $country->country->name,
+                        'country name is United States of America');
+  }
+
+  public function testMe()
+  {
+    $client = $this->client($this->getResponse('me'));
+
+    $this->assertInstanceOf('GeoIP2\Model\CityIspOrg',
+                            $client->cityIspOrg('me' ),
+                            'can set ip parameter to me');
+  }
+
+  /**
+   * @expectedException GeoIP2\Exception\GenericException
+   * @expectedExceptionMessage Received a 200 response for https://geoip.maxmind.com/geoip/v2.0/country/1.2.3.5 but did not receive a HTTP body.
+   */
+  public function testNoBodyException() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.5'));
+
+    $client->country('1.2.3.5');
+  }
+
+  /**
+   * @expectedException GeoIP2\Exception\GenericException
+   * @expectedExceptionMessage Received a 200 response for https://geoip.maxmind.com/geoip/v2.0/country/2.2.3.5 but could not decode the response as JSON:
+   */
+  public function testBadBodyException() 
+  {
+    $client = $this->client($this->getResponse('2.2.3.5'));
+
+    $client->country('2.2.3.5');
+  }
+
+
+  /**
+   * @expectedException GeoIP2\Exception\WebserviceException
+   * @expectedExceptionCode IP_ADDRESS_INVALID
+   * @expectedExceptionMessage The value "1.2.3" is not a valid ip address
+   */
+  public function testInvalidIPException() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.6'));
+
+    $client->country('1.2.3.6');
+
+  }
+
+  /**
+   * @expectedException GeoIP2\Exception\HttpException
+   * @expectedExceptionCode 400
+   * @expectedExceptionMessage with no body
+   */
+  public function testNoErrorBodyIPException() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.7'));
+
+    $client->country('1.2.3.7');
+
+  }
+
+  /**
+   * @expectedException GeoIP2\Exception\GenericException
+   * @expectedExceptionMessage Response contains JSON but it does not specify code or error keys
+   */
+  public function testWeirdErrorBodyIPException() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.8'));
+
+    $client->country('1.2.3.8');
+
+  }  
+
+  /**
+   * @expectedException GeoIP2\Exception\HttpException
+   * @expectedExceptionCode 400
+   * @expectedExceptionMessage did not include the expected JSON body
+   */
+  public function testInvalidErrorBodyIPException() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.9'));
+
+    $client->country('1.2.3.9');
+
+  }
+
+  /**
+   * @expectedException GeoIP2\Exception\HttpException
+   * @expectedExceptionCode 500
+   * @expectedExceptionMessage Received a server error (500)
+   */
+  public function test500PException() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.10'));
+
+    $client->country('1.2.3.10');
+
+  }
+
+  /**
+   * @expectedException GeoIP2\Exception\HttpException
+   * @expectedExceptionCode 300
+   * @expectedExceptionMessage Received a very surprising HTTP status (300)
+   */
+  public function test3xxException() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.11'));
+
+    $client->country('1.2.3.11');
+
+  }
+
+  /**
+   * @expectedException GeoIP2\Exception\HttpException
+   * @expectedExceptionCode 406
+   * @expectedExceptionMessage Received a 406 error for https://geoip.maxmind.com/geoip/v2.0/country/1.2.3.12 with the following body: Cannot satisfy your Accept-Charset requirements
+   */
+  public function test406Exception() 
+  {
+    $client = $this->client($this->getResponse('1.2.3.12'));
+
+    $client->country('1.2.3.12');
+
+  }
+
+  public function testParams() {
+    $plugin = new MockPlugin();
+    $plugin->addResponse($this->getResponse('1.2.3.4'));
+    $guzzleClient = new GuzzleClient();
+    $guzzleClient->addSubscriber($plugin);
+
+    $client = new Client(42, 'abcdef123456', array('en'),
+                         $guzzleClient);
+    $client->country('1.2.3.4');
+
+    $request = $plugin->getReceivedRequests()[0];
+
+    $this->assertEquals('https://geoip.maxmind.com/geoip/v2.0/country/1.2.3.4',
+                        $request->getUrl(),
+                        'got expected URI for Country request');
+    $this->assertEquals('GET', $request->getMethod(), 'request is a GET');
+
+    $this->assertEquals('application/json', $request->getHeader('Accept'),
+                        'request sets Accept header to application/json');
+
+    $this->assertStringMatchesFormat('GeoIP2 PHP API (Guzzle%s)',
+                                     $request->getHeader('User-Agent') . '',
+                                     'request sets Accept header to application/json');
+
+
+  }
+
+
+  private function client($response, $languages=array('en')) {
+    $plugin = new MockPlugin();
+    $plugin->addResponse($response);
+    $guzzleClient = new GuzzleClient();
+    $guzzleClient->addSubscriber($plugin);
+
+    $client = new Client(42, 'abcdef123456', $languages, 
+                         $guzzleClient);
+
+    return $client;
+  }
+
+  private function response($endpoint, $status, $body=null,
+                            $bad=null, $contentType=null)
+  {
+    $headers = Array();
+    if( $contentType) {
+      $headers['Content-Type'] = $contentType;
+    }
+    elseif ( $status == 200 || ( $status >= 400 && $status < 500 ) ) {
+      $headers['Content-Type'] = 'application/vnd.maxmind.com-'
+        . $endpoint . '+json; charset=UTF-8; version=1.0;';
+    }
+
+    if ($bad) {
+      $body = '{ invalid: }';
+    }
+    elseif (is_array($body)) {
+      $body =  json_encode($body);
+    }
+
+    $headers['Content-Length'] = strlen($body);
+
+    return new Response($status, $headers, $body);
+  }
+
+  public function testTest()
+  {
+    $this->assertEquals(1,1);
+  }
+}

tests/bootstrap.php

@@ -0,0 +1,8 @@
+<?php
+
+if (!$loader = @include __DIR__.'/../vendor/autoload.php') {
+    die('Project dependencies missing');
+}
+
+$loader->add('GeoIP2\Test', __DIR__);
+