Merge pull request #323 from JeroenDeDauw/NominatimGeocoder

Catch file fecthing errors in NominatimGeocoder
ProfessionalWiki · May 10, 2017 · 29876d6 · 29876d6
2 parents 6206737 + bb2c4a0
commit 29876d6
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 5 deletions.
diff --git a/src/Geocoders/NominatimGeocoder.php b/src/Geocoders/NominatimGeocoder.php
@@ -4,6 +4,7 @@
 
 use DataValues\Geo\Values\LatLongValue;
 use FileFetcher\FileFetcher;
+use FileFetcher\FileFetchingException;
 
 /**
  * Webservice documentation: http://wiki.openstreetmap.org/wiki/Nominatim
@@ -28,7 +29,12 @@ public function __construct( FileFetcher $fileFetcher ) {
 	 * @return LatLongValue|null
 	 */
 	public function geocode( $address ) {
-		$response = $this->fileFetcher->fetchFile( $this->getRequestUrl( $address ) );
+		try {
+			$response = $this->fileFetcher->fetchFile( $this->getRequestUrl( $address ) );
+		}
+		catch ( FileFetchingException $ex ) {
+			return null;
+		}
 
 		$jsonResponse = json_decode( $response );
 
@@ -38,7 +44,9 @@ public function geocode( $address ) {
 
 		$location  = $jsonResponse[0];
 
-		if ( !$location->lat || !$location->lon ) return null;
+		if ( !isset( $location->lat ) || ! isset( $location->lon ) ) {
+			return null;
+		}
 
 		return new LatLongValue( (float)$location->lat, (float)$location->lon );
 	}

diff --git a/tests/Unit/Geocoders/NominatimGeocoderTest.php b/tests/Unit/Geocoders/NominatimGeocoderTest.php
@@ -14,9 +14,11 @@
  */
 class NominatimGeocoderTest extends \PHPUnit_Framework_TestCase {
 
+	const NEW_YORK_FETCH_URL = 'https://nominatim.openstreetmap.org/search?format=jsonv2&limit=1&q=New+York';
+
 	public function testHappyPath() {
 		$fileFetcher = new InMemoryFileFetcher( [
-			'https://nominatim.openstreetmap.org/search?format=jsonv2&limit=1&q=New+York'
+			self::NEW_YORK_FETCH_URL
 				=> '[{"place_id":"97961780","licence":"Data © OpenStreetMap contributors, ODbL 1.0. http:\/\/www.openstreetmap.org\/copyright","osm_type":"way","osm_id":"161387758","boundingbox":["40.763858","40.7642664","-73.9548572","-73.954092"],"lat":"40.7642499","lon":"-73.9545249","display_name":"NewYork Hospital Drive, Upper East Side, Manhattan, New York County, New York City, New York, 10021, United States of America","place_rank":"27","category":"highway","type":"service","importance":0.275}]'
 		] );
 
@@ -26,8 +28,33 @@ public function testHappyPath() {
 		$this->assertSame( -73.9545249, $geocoder->geocode( 'New York' )->getLongitude() );
 	}
 
-	// TODO: test no result found case
-	// TODO: test network failure case
+	public function testWhenFetcherThrowsException_nullIsReturned() {
+		$geocoder = new NominatimGeocoder( new InMemoryFileFetcher( [] ) );
+
+		$this->assertNull( $geocoder->geocode( 'New York' ) );
+	}
+
+	/**
+	 * @dataProvider invalidResponseProvider
+	 */
+	public function testWhenFetcherReturnsInvalidResponse_nullIsReturned( $invalidResponse ) {
+		$geocoder = new NominatimGeocoder( new InMemoryFileFetcher( [
+			self::NEW_YORK_FETCH_URL => $invalidResponse
+		] ) );
+
+		$this->assertNull( $geocoder->geocode( 'New York' ) );
+	}
+
+	public function invalidResponseProvider() {
+		return [
+			'Not JSON' => [ '~=[,,_,,]:3' ],
+			'Not a JSON array' => [ '42' ],
+			'Empty JSON array' => [ '[]' ],
+			'Missing lon key' => [ '[{"lat":"40.7642499","FOO":"-73.9545249"}]' ],
+			'Missing lat key' => [ '[{"FOO":"40.7642499","lon":"-73.9545249"}]' ],
+		];
+	}
+
 	// TODO: test malicious address escaping
 
 }