Enable GeoLocation service being replaced by other providers

Making GeoLocation an interface makes it easier to replace the underlying implementation
from the current Http backed method.

Change-Id: I2beb97772fd74ab08b2214c08d82dbc1ebfcdcd2

includes/Decisions.php

@@ -90,20 +90,19 @@ class Decisions {
 		}
 
 		wfDebugLog( 'CookieWarning', 'Try to locate the user\'s IP address.' );
-		$located = $this->geoLocation->locate( $currentIP );
-		if ( !$located ) {
+		$location = $this->geoLocation->locate( $currentIP );
+		if ( $location === null ) {
 			wfDebugLog( 'CookieWarning',
 				'Locating the user\'s IP address failed or is misconfigured.' );
 
 			return '';
 		}
 
-		$lookedUpCountryCode = $this->geoLocation->getCountryCode();
-		$this->cache->set( $cacheKey, $lookedUpCountryCode );
+		$this->cache->set( $cacheKey, $location );
 
-		wfDebugLog( 'CookieWarning', 'Locating the user was successful, located' . ' region: ' .
-			$lookedUpCountryCode );
+		wfDebugLog( 'CookieWarning',
+			'Locating the user was successful, located region: ' . $location );
 
-		return $lookedUpCountryCode;
+		return $location;
 	}
 }

includes/GeoLocation.php

@@ -2,73 +2,13 @@
 
 namespace CookieWarning;
 
-/**
- * GeoLocation implementation
- */
-
-use Config;
-use ConfigException;
-use Http;
-use InvalidArgumentException;
-use IP;
-
-/**
- * Implements the GeoLocation class, which allows to locate the user based on the IP address.
- */
-class GeoLocation {
-	private $config;
-	private $countryCode;
-
+interface GeoLocation {
 	/**
-	 * @param Config $config
-	 */
-	public function __construct( Config $config ) {
-		$this->config = $config;
-	}
-
-	/**
-	 * Returns the country code, if the last call to self::locate() returned true. Otherwise, NULL.
-	 *
-	 * @return null|string
-	 */
-	public function getCountryCode() {
-		return $this->countryCode;
-	}
-
-	/**
-	 * Tries to locate the IP address set with self::setIP() using the geolocation service
-	 * configured with the $wgCookieWarningGeoIPServiceURL configuration variable. If the config
-	 * isn't set, this function returns NULL. If the config is set, but the URL is invalid or an
-	 * other problem occures which resulted in a failed locating process, this function returns
-	 * false, otherwise it returns true.
+	 * Tries to locate the given IP address.
 	 *
 	 * @param string $ip The IP address to lookup
-	 * @return bool|null NULL if no geolocation service configured, false on error, true otherwise.
-	 * @throws ConfigException
+	 * @return null|string NULL on error or if locating the IP was not possible, the country
+	 * code otherwise
 	 */
-	public function locate( $ip ) {
-		$this->countryCode = null;
-		if ( !IP::isValid( $ip ) ) {
-			throw new InvalidArgumentException( "$ip is not a valid IP address." );
-		}
-		if ( !$this->config->get( 'CookieWarningGeoIPServiceURL' ) ) {
-			return null;
-		}
-		$requestUrl = $this->config->get( 'CookieWarningGeoIPServiceURL' );
-		if ( substr( $requestUrl, -1 ) !== '/' ) {
-			$requestUrl .= '/';
-		}
-		$json = Http::get( $requestUrl . $ip, [
-			'timeout' => '2'
-		] );
-		if ( !$json ) {
-			return false;
-		}
-		$returnObject = json_decode( $json );
-		if ( $returnObject === null || !property_exists( $returnObject, 'country_code' ) ) {
-			return false;
-		}
-		$this->countryCode = $returnObject->country_code;
-		return true;
-	}
+	public function locate( $ip );
 }

includes/HttpGeoLocation.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace CookieWarning;
+
+use Http;
+use InvalidArgumentException;
+use IP;
+
+/**
+ * Implements the GeoLocation class, which allows to locate the user based on the IP address.
+ */
+class HttpGeoLocation implements GeoLocation {
+	private $geoIPServiceURL;
+	private $locatedIPs = [];
+
+	/**
+	 * @param string $geoIPServiceURL
+	 */
+	public function __construct( $geoIPServiceURL ) {
+		if ( !is_string( $geoIPServiceURL ) || !$geoIPServiceURL ) {
+			throw new InvalidArgumentException( 'The geoIPServiceUL is invalid' );
+		}
+		$this->geoIPServiceURL = $geoIPServiceURL;
+	}
+
+	/**
+	 * {@inheritdoc}
+	 * @param string $ip The IP address to lookup
+	 * @return string|null
+	 */
+	public function locate( $ip ) {
+		if ( isset( $this->locatedIPs[$ip] ) ) {
+			return $this->locatedIPs[$ip];
+		}
+		if ( !IP::isValid( $ip ) ) {
+			throw new InvalidArgumentException( "$ip is not a valid IP address." );
+		}
+		if ( substr( $this->geoIPServiceURL, -1 ) !== '/' ) {
+			$this->geoIPServiceURL .= '/';
+		}
+		$json = Http::get( $this->geoIPServiceURL . $ip, [
+			'timeout' => '2',
+		] );
+		if ( !$json ) {
+			return null;
+		}
+		$returnObject = json_decode( $json );
+		if ( $returnObject === null || !property_exists( $returnObject, 'country_code' ) ) {
+			return null;
+		}
+		$this->locatedIPs[$ip] = $returnObject->country_code;
+
+		return $this->locatedIPs[$ip];
+	}
+}

includes/NoopGeoLocation.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace CookieWarning;
+
+class NoopGeoLocation implements GeoLocation {
+
+	/**
+	 * {@inheritdoc}
+	 * @param string $ip The IP address to lookup
+	 * @return bool|null NULL if no geolocation service configured, false on error, true otherwise.
+	 */
+	public function locate( $ip ) {
+		return null;
+	}
+}

includes/ServiceWiring.php

@@ -1,7 +1,8 @@
 <?php
 
 use CookieWarning\Decisions;
-use CookieWarning\GeoLocation;
+use CookieWarning\HttpGeoLocation;
+use CookieWarning\NoopGeoLocation;
 use MediaWiki\MediaWikiServices;
 
 return [
@@ -10,7 +11,14 @@ return [
 			->makeConfig( 'cookiewarning' );
 	},
 	'GeoLocation' => function ( MediaWikiServices $services ) {
-		return new GeoLocation( $services->getService( 'CookieWarning.Config' ) );
+		$geoIPServiceURL = $services
+			->getService( 'CookieWarning.Config' )
+			->get( 'CookieWarningGeoIPServiceURL' );
+
+		if ( !is_string( $geoIPServiceURL ) || !$geoIPServiceURL ) {
+			return new NoopGeoLocation();
+		}
+		return new HttpGeoLocation( $geoIPServiceURL );
 	},
 	'CookieWarning.Decisions' => function ( MediaWikiServices $services ) {
 		return new Decisions( $services->getService( 'CookieWarning.Config' ),

tests/phpunit/includes/DecisionsTest.php

@@ -28,8 +28,7 @@ class DecisionsTest extends MediaWikiTestCase {
 		$geoLocation = $this->getMockBuilder( GeoLocation::class )
 			->disableOriginalConstructor()
 			->getMock();
-		$geoLocation->method( 'locate' )->willReturn( true );
-		$geoLocation->method( 'getCountryCode' )->willReturn( 'EU' );
+		$geoLocation->method( 'locate' )->willReturn( 'EU' );
 
 		$geoLocation->expects( $this->once() )->method( 'locate' );
 		$cookieWarningDecisions = new Decisions(

tests/phpunit/includes/HooksTest.php

@@ -191,8 +191,7 @@ class HooksTest extends MediaWikiLangTestCase {
 		$geoLocation = $this->getMockBuilder( GeoLocation::class )
 			->disableOriginalConstructor()
 			->getMock();
-		$geoLocation->method( 'locate' )->willReturn( true );
-		$geoLocation->method( 'getCountryCode' )->willReturn( 'US' );
+		$geoLocation->method( 'locate' )->willReturn( 'US' );
 		$this->setService( 'GeoLocation', $geoLocation );
 	}
 }

tests/phpunit/includes/ServiceWiringTest.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace CookieWarning\Tests;
+
+use CookieWarning\HttpGeoLocation;
+use CookieWarning\NoopGeoLocation;
+use MediaWiki\MediaWikiServices;
+use MediaWikiTestCase;
+
+class ServiceWiringTest extends MediaWikiTestCase {
+
+	/**
+	 * @covers \CookieWarning\NoopGeoLocation
+	 */
+	public function testGeoLocationWithoutServiceURL() {
+		$this->setMwGlobals( [
+			'wgCookieWarningGeoIPServiceURL' => null
+		] );
+
+		$geoLocation = MediaWikiServices::getInstance()->getService( 'GeoLocation' );
+
+		$this->assertInstanceOf( NoopGeoLocation::class, $geoLocation );
+	}
+
+	/**
+	 * @covers \CookieWarning\HttpGeoLocation
+	 */
+	public function testGeoLocationWithServiceURL() {
+		$this->setMwGlobals( [
+			'wgCookieWarningGeoIPServiceURL' => 'http://localhost/'
+		] );
+
+		$geoLocation = MediaWikiServices::getInstance()->getService( 'GeoLocation' );
+
+		$this->assertInstanceOf( HttpGeoLocation::class, $geoLocation );
+	}
+}