Merge pull request #214 from JeroenDeDauw/mawiki

Refactor new geocoder and its test

Maps.php

@@ -11,6 +11,7 @@
  */
 
 use DataValues\Geo\Parsers\GeoCoordinateParser;
+use FileFetcher\SimpleFileFetcher;
 use Maps\CircleParser;
 use Maps\DistanceParser;
 use Maps\ImageOverlayParser;
@@ -151,7 +152,13 @@
 	$GLOBALS['wgHooks']['GeocoderFirstCallInit'][] = 'MapsGeocoderusGeocoder::register';
 
 	// Registration of the OSM Nominatim service geocoder.
-	$GLOBALS['wgHooks']['GeocoderFirstCallInit'][] = 'MapsNominatimGeocoder::register';
+	$GLOBALS['wgHooks']['GeocoderFirstCallInit'][] = function() {
+		\Maps\Geocoders::registerGeocoder(
+			'nominatim',
+			new \Maps\Geocoders\NominatimGeocoder( new SimpleFileFetcher() )
+		);
+		return true;
+	};
 
 	// Mapping services
 

composer.json

@@ -30,7 +30,9 @@
 		"php": ">=5.5",
 		"composer/installers": "^1.0.1",
 		"mediawiki/validator": "^2.0.2",
-		"data-values/geo": "~1.0"
+		"data-values/geo": "~1.0",
+		"jeroen/file-fetcher": "~3.1",
+		"jeroen/simple-cache": "~2.0"
 	},
 	"require-dev": {
 		"data-values/common": "~0.3.1"
@@ -49,6 +51,11 @@
 			"tests/phpunit/parserhooks/ParserHookTest.php"
 		]
 	},
+	"autoload-dev": {
+		"psr-4": {
+			"Maps\\Tests\\TestDoubles\\": "tests/TestDoubles/"
+		}
+	},
 	"extra": {
 		"branch-alias": {
 			"dev-master": "3.8.x-dev"

includes/Geocoders.php

@@ -5,6 +5,7 @@
 use DataValues\Geo\Formatters\GeoCoordinateFormatter;
 use DataValues\Geo\Parsers\GeoCoordinateParser;
 use DataValues\Geo\Values\LatLongValue;
+use MapsDecoratedGeocoder;
 use MWException;
 use ValueParsers\ParseException;
 
@@ -310,10 +311,10 @@ protected static function cacheWrite( $address, LatLongValue $coordinates ) {
 	 * @since 0.7
 	 *
 	 * @param string $geocoderIdentifier
-	 * @param string $geocoderClassName
+	 * @param string|\Maps\Geocoders\Geocoder $geocoder
 	 */
-	public static function registerGeocoder( $geocoderIdentifier, $geocoderClassName ) {
-		self::$registeredGeocoders[$geocoderIdentifier] = $geocoderClassName;
+	public static function registerGeocoder( $geocoderIdentifier, $geocoder ) {
+		self::$registeredGeocoders[$geocoderIdentifier] = $geocoder;
 	}
 
 	/**
@@ -346,14 +347,21 @@ protected static function getValidGeocoderInstance( $geocoderIdentifier ) {
 	protected static function getGeocoderInstance( $geocoderIdentifier ) {
 		if ( !array_key_exists( $geocoderIdentifier, self::$geocoders ) ) {
 			if ( array_key_exists( $geocoderIdentifier, self::$registeredGeocoders ) ) {
-				$geocoder = new self::$registeredGeocoders[$geocoderIdentifier]( $geocoderIdentifier );
-
-				//if ( $service instanceof iMappingService ) {
-					self::$geocoders[$geocoderIdentifier] = $geocoder;
-				//}
-				//else {
-				//	throw new MWException( 'The geocoder linked to identifier ' . $geocoderIdentifier . ' does not implement .' );
-				//}
+				if ( is_string( self::$registeredGeocoders[$geocoderIdentifier] ) ) {
+					$geocoder = new self::$registeredGeocoders[$geocoderIdentifier]( $geocoderIdentifier );
+				}
+				elseif ( self::$registeredGeocoders[$geocoderIdentifier] instanceof \Maps\Geocoders\Geocoder ) {
+					$geocoder = new MapsDecoratedGeocoder(
+						self::$registeredGeocoders[$geocoderIdentifier],
+						$geocoderIdentifier
+					);
+				}
+				else {
+					throw new MWException( 'Need either class name or Geocoder instance' );
+				}
+
+
+				self::$geocoders[$geocoderIdentifier] = $geocoder;
 			}
 			else {
 				throw new MWException( 'There is geocoder linked to identifier ' . $geocoderIdentifier . '.' );

includes/geocoders/Maps_DecoratedGeocoder.php

@@ -0,0 +1,42 @@
+<?php
+
+use Maps\Geocoders\Geocoder;
+
+/**
+ * @since 3.8
+ *
+ * @licence GNU GPL v2+
+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ */
+final class MapsDecoratedGeocoder extends \Maps\Geocoder {
+
+	private $geocoder;
+
+	/**
+	 * @param Geocoder $geocoder
+	 * @param string $identifier
+	 */
+	public function __construct( Geocoder $geocoder, $identifier ) {
+		$this->geocoder = $geocoder;
+
+		parent::__construct( $identifier );
+	}
+
+	public function geocode( $address ) {
+		$result = $this->geocoder->geocode( $address );
+
+		if ( $result === null ) {
+			return false;
+		}
+
+		return [
+			'lat' => $result->getLatitude(),
+			'lon' => $result->getLongitude(),
+		];
+	}
+
+	protected function getRequestUrl( $address ) {}
+
+	protected function parseResponse( $response ) {}
+
+}

src/Geocoders/Geocoder.php

@@ -0,0 +1,24 @@
+<?php
+
+namespace Maps\Geocoders;
+
+use DataValues\Geo\Values\LatLongValue;
+
+/**
+ * @since 3.8
+ *
+ * @licence GNU GPL v2+
+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ */
+interface Geocoder {
+
+	/**
+	 * @param string $address
+	 *
+	 * @return LatLongValue|null
+	 *
+	 * TODO: specify failure behaviour. Exception or null?
+	 */
+	public function geocode( $address );
+
+}

src/Geocoders/InMemoryGeocoder.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Maps\Geocoders;
+
+use DataValues\Geo\Values\LatLongValue;
+
+/**
+ * @since 3.8
+ *
+ * @licence GNU GPL v2+
+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ */
+class InMemoryGeocoder implements Geocoder {
+
+	private $locations;
+
+	/**
+	 * @param LatLongValue[] $locations
+	 */
+	public function __construct( array $locations ) {
+		$this->locations = $locations;
+	}
+
+	/**
+	 * @param string $address
+	 *
+	 * @return LatLongValue|null
+	 */
+	public function geocode( $address ) {
+		if ( array_key_exists( $address, $this->locations ) ) {
+			return $this->locations[$address];
+		}
+
+		return null;
+	}
+
+}

src/Geocoders/NominatimGeocoder.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace Maps\Geocoders;
+
+use DataValues\Geo\Values\LatLongValue;
+use FileFetcher\FileFetcher;
+
+/**
+ * Webservice documentation: http://wiki.openstreetmap.org/wiki/Nominatim
+ *
+ * @since 3.8
+ *
+ * @licence GNU GPL v2+
+ * @author Peter Grassberger < petertheone@gmail.com >
+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ */
+class NominatimGeocoder implements Geocoder {
+
+	private $fileFetcher;
+
+	public function __construct( FileFetcher $fileFetcher ) {
+		$this->fileFetcher = $fileFetcher;
+	}
+
+	/**
+	 * @param string $address
+	 *
+	 * @return LatLongValue|null
+	 */
+	public function geocode( $address ) {
+		$response = $this->fileFetcher->fetchFile( $this->getRequestUrl( $address ) );
+
+		$jsonResponse = json_decode( $response );
+
+		if ( !is_array( $jsonResponse ) || count( $jsonResponse ) < 1 ) {
+			return null;
+		}
+
+		$location  = $jsonResponse[0];
+
+		if ( !$location->lat || !$location->lon ) return null;
+
+		return new LatLongValue( (float)$location->lat, (float)$location->lon );
+	}
+
+	/**
+	 * @param string $address
+	 *
+	 * @return string
+	 */
+	private function getRequestUrl( $address ) {
+		return 'https://nominatim.openstreetmap.org/search?format=jsonv2&limit=1&q=' . urlencode( $address );
+	}
+
+}

tests/bootstrap.php

@@ -0,0 +1,14 @@
+<?php
+
+if ( PHP_SAPI !== 'cli' ) {
+	die( 'Not an entry point' );
+}
+
+error_reporting( -1 );
+ini_set( 'display_errors', 1 );
+
+if ( !is_readable( __DIR__ . '/../vendor/autoload.php' ) ) {
+	die( 'You need to install this package with Composer before you can run the tests' );
+}
+
+require_once __DIR__ . '/../vendor/autoload.php';