From 851925b2d68d80fe94b594b03a6f367f27e2d45d Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 8 May 2013 14:42:13 -0700 Subject: [PATCH] Webservice client unit tests and fixes for bugs discovered while creating them --- .gitignore | 3 + phpunit.xml.dist | 15 + src/GeoIP2/Exception/GenericException.php | 7 + src/GeoIP2/Exception/HttpException.php | 4 +- src/GeoIP2/Exception/WebserviceException.php | 16 + .../Model/{CityISPOrg.php => CityIspOrg.php} | 2 +- src/GeoIP2/Model/Omni.php | 2 +- src/GeoIP2/Webservice/Client.php | 75 +++-- tests/GeoIP2/Test/Webservice/ClientTest.php | 307 ++++++++++++++++++ tests/bootstrap.php | 8 + 10 files changed, 405 insertions(+), 34 deletions(-) create mode 100644 phpunit.xml.dist rename src/GeoIP2/Model/{CityISPOrg.php => CityIspOrg.php} (53%) create mode 100644 tests/GeoIP2/Test/Webservice/ClientTest.php create mode 100644 tests/bootstrap.php diff --git a/.gitignore b/.gitignore index 3b1a674..6ba24cd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,6 @@ composer.lock composer.phar phpunit.xml vendor/ +*.sw? +t.php +*.old diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..0f0e810 --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,15 @@ + + + + + + ./tests/GeoIP2/Test/ + + + + + + ./src/GeoIP2/ + + + diff --git a/src/GeoIP2/Exception/GenericException.php b/src/GeoIP2/Exception/GenericException.php index e69de29..b12d3ac 100644 --- a/src/GeoIP2/Exception/GenericException.php +++ b/src/GeoIP2/Exception/GenericException.php @@ -0,0 +1,7 @@ +code = $code; + parent::__construct($message, null, $previous); } } \ No newline at end of file diff --git a/src/GeoIP2/Exception/WebserviceException.php b/src/GeoIP2/Exception/WebserviceException.php index e69de29..c422110 100644 --- a/src/GeoIP2/Exception/WebserviceException.php +++ b/src/GeoIP2/Exception/WebserviceException.php @@ -0,0 +1,16 @@ +httpStatus = $httpStatus; + parent::__construct($message, $code, $uri, $previous); + } + +} \ No newline at end of file diff --git a/src/GeoIP2/Model/CityISPOrg.php b/src/GeoIP2/Model/CityIspOrg.php similarity index 53% rename from src/GeoIP2/Model/CityISPOrg.php rename to src/GeoIP2/Model/CityIspOrg.php index 4be358d..0d43417 100644 --- a/src/GeoIP2/Model/CityISPOrg.php +++ b/src/GeoIP2/Model/CityIspOrg.php @@ -2,7 +2,7 @@ namespace GeoIP2\Model; -class CityISPOrg extends City +class CityIspOrg extends City { } \ No newline at end of file diff --git a/src/GeoIP2/Model/Omni.php b/src/GeoIP2/Model/Omni.php index 629611b..8c676dd 100644 --- a/src/GeoIP2/Model/Omni.php +++ b/src/GeoIP2/Model/Omni.php @@ -2,7 +2,7 @@ namespace GeoIP2\Model; -class Omni extends CityISPOrg +class Omni extends CityIspOrg { } diff --git a/src/GeoIP2/Webservice/Client.php b/src/GeoIP2/Webservice/Client.php index 359b305..44b5b31 100644 --- a/src/GeoIP2/Webservice/Client.php +++ b/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 = $request->send(); + $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 (Exception $e) { + catch (RuntimeException $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) { - $this->handle4xx($response, $uri); - } - elseif ($status >= 500 && $status <= 599){ - $this->handle5xx($response, $uri); - } - else { - $this->hanldeNon200($reponse, $uri); - } - } + $body = array(); - private function handle4xx($response, $uri) - { if ( $response->getContentLength() > 0 ) { if( strstr($response->getContentType(), 'json')) { try { $body = $response->json(); - if (!$body['code'] || $body['error'] ){ - throw new GenericException('Response contains JSON but it does not specify code or error keys'); + if (!isset($body['code']) || !isset($body['error']) ){ + throw new GenericException('Response contains JSON but it does not specify code or error keys: ' . $response->getBody()); } } - // XXX - don't catch all exceptions - catch (Exception $e){ + catch (RuntimeException $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); diff --git a/tests/GeoIP2/Test/Webservice/ClientTest.php b/tests/GeoIP2/Test/Webservice/ClientTest.php new file mode 100644 index 0000000..74510a0 --- /dev/null +++ b/tests/GeoIP2/Test/Webservice/ClientTest.php @@ -0,0 +1,307 @@ + 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); + } +} \ No newline at end of file diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..20e17ed --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,8 @@ +add('GeoIP2\Test', __DIR__); +