From ac7a69a27e4f443d383077b53fc1f109dc702226 Mon Sep 17 00:00:00 2001
From: Stefan Giehl <stefan@piwik.org>
Date: Fri, 23 Sep 2016 06:22:34 +0200
Subject: [PATCH] Improves handling of Geolocation providers (3.x) (#10523)
* refactor code for including available location providers
* fix test
---
plugins/UserCountry/LocationProvider.php | 43 +++++++++++--------
.../UserCountry/LocationProvider/GeoIp.php | 18 +-------
.../Integration/VisitorGeolocatorTest.php | 5 +++
.../PHPUnit/Fixtures/ManyVisitsWithGeoIP.php | 3 +-
.../ManyVisitsWithMockLocationProvider.php | 2 +
tests/PHPUnit/System/TrackerTest.php | 2 +
6 files changed, 36 insertions(+), 37 deletions(-)
diff --git a/plugins/UserCountry/LocationProvider.php b/plugins/UserCountry/LocationProvider.php
index 50fd52b987..771984dbd4 100755
--- a/plugins/UserCountry/LocationProvider.php
+++ b/plugins/UserCountry/LocationProvider.php
@@ -13,25 +13,16 @@ use Piwik\Common;
use Piwik\IP;
use Piwik\Option;
use Piwik\Piwik;
+use Piwik\Plugin;
use Piwik\Plugins\UserCountry\LocationProvider\DefaultProvider;
+use Piwik\Plugin\Manager as PluginManager;
use Piwik\Tracker\Cache;
-use ReflectionClass;
/**
* @see plugins/UserCountry/functions.php
*/
require_once PIWIK_INCLUDE_PATH . '/plugins/UserCountry/functions.php';
-/**
- * @see plugins/UserCountry/LocationProvider/DefaultProvider.php
- */
-require_once PIWIK_INCLUDE_PATH . '/plugins/UserCountry/LocationProvider/DefaultProvider.php';
-
-/**
- * @see plugins/UserCountry/LocationProvider/GeoIp.php
- */
-require_once PIWIK_INCLUDE_PATH . '/plugins/UserCountry/LocationProvider/GeoIp.php';
-
/**
* The base class of all LocationProviders.
*
@@ -148,14 +139,10 @@ abstract class LocationProvider
{
if (is_null(self::$providers)) {
self::$providers = array();
- foreach (get_declared_classes() as $klass) {
- if (is_subclass_of($klass, 'Piwik\Plugins\UserCountry\LocationProvider')) {
- $klassInfo = new ReflectionClass($klass);
- if ($klassInfo->isAbstract()) {
- continue;
- }
-
- self::$providers[] = new $klass;
+ $plugins = PluginManager::getInstance()->getPluginsLoadedAndActivated();
+ foreach ($plugins as $plugin) {
+ foreach (self::getLocationProviders($plugin) as $instance) {
+ self::$providers[] = $instance;
}
}
}
@@ -163,6 +150,24 @@ abstract class LocationProvider
return self::$providers;
}
+ /**
+ * Get all lo that are defined by the given plugin.
+ *
+ * @param Plugin $plugin
+ * @return LocationProvider[]
+ */
+ protected static function getLocationProviders(Plugin $plugin)
+ {
+ $locationProviders = $plugin->findMultipleComponents('LocationProvider', 'Piwik\\Plugins\\UserCountry\\LocationProvider');
+ $instances = [];
+
+ foreach ($locationProviders as $locationProvider) {
+ $instances[] = new $locationProvider();
+ }
+
+ return $instances;
+ }
+
/**
* Returns all provider instances that are 'available'. An 'available' provider
* is one that is available for use. They may not necessarily be working.
diff --git a/plugins/UserCountry/LocationProvider/GeoIp.php b/plugins/UserCountry/LocationProvider/GeoIp.php
index b8e090ea74..b37c3df3c8 100755
--- a/plugins/UserCountry/LocationProvider/GeoIp.php
+++ b/plugins/UserCountry/LocationProvider/GeoIp.php
@@ -255,20 +255,4 @@ abstract class GeoIp extends LocationProvider
}
return false;
}
-}
-
-/**
- * @see plugins/UserCountry/LocationProvider/GeoIp/ServerBased.php
- */
-require_once PIWIK_INCLUDE_PATH . '/plugins/UserCountry/LocationProvider/GeoIp/ServerBased.php';
-
-/**
- * @see plugins/UserCountry/LocationProvider/GeoIp/Php.php
- */
-require_once PIWIK_INCLUDE_PATH . '/plugins/UserCountry/LocationProvider/GeoIp/Php.php';
-
-/**
- * @see plugins/UserCountry/LocationProvider/GeoIp/Pecl.php
- */
-require_once PIWIK_INCLUDE_PATH . '/plugins/UserCountry/LocationProvider/GeoIp/Pecl.php';
-
+}
\ No newline at end of file
diff --git a/plugins/UserCountry/tests/Integration/VisitorGeolocatorTest.php b/plugins/UserCountry/tests/Integration/VisitorGeolocatorTest.php
index ef1be64b1a..c9e6891f54 100644
--- a/plugins/UserCountry/tests/Integration/VisitorGeolocatorTest.php
+++ b/plugins/UserCountry/tests/Integration/VisitorGeolocatorTest.php
@@ -39,6 +39,11 @@ class VisitorGeolocatorTest extends IntegrationTestCase
parent::setUp();
$this->logInserter = new LogHelper();
+
+ // ensure all providers are loaded and add mock provider
+ LocationProvider::$providers = null;
+ $providers = LocationProvider::getAllProviders();
+ LocationProvider::$providers[] = new MockLocationProvider();
}
public function test_getLocation_shouldReturnLocationForProvider_IfLocationIsSetForCurrentProvider()
diff --git a/tests/PHPUnit/Fixtures/ManyVisitsWithGeoIP.php b/tests/PHPUnit/Fixtures/ManyVisitsWithGeoIP.php
index 8fb67f261a..7faeaee97a 100644
--- a/tests/PHPUnit/Fixtures/ManyVisitsWithGeoIP.php
+++ b/tests/PHPUnit/Fixtures/ManyVisitsWithGeoIP.php
@@ -245,7 +245,8 @@ class ManyVisitsWithGeoIP extends Fixture
private function setMockLocationProvider()
{
- LocationProvider::$providers = null;
+ LocationProvider::$providers = array();
+ LocationProvider::$providers[] = new MockLocationProvider();
LocationProvider::setCurrentProvider('mock_provider');
MockLocationProvider::$locations = array(
self::makeLocation('Stratford-upon-Avon', 'P3', 'gb', 123.456, 21.321), // template location
diff --git a/tests/PHPUnit/Fixtures/ManyVisitsWithMockLocationProvider.php b/tests/PHPUnit/Fixtures/ManyVisitsWithMockLocationProvider.php
index 9fbd6f986d..ae5b1ae4dd 100644
--- a/tests/PHPUnit/Fixtures/ManyVisitsWithMockLocationProvider.php
+++ b/tests/PHPUnit/Fixtures/ManyVisitsWithMockLocationProvider.php
@@ -212,6 +212,8 @@ class ManyVisitsWithMockLocationProvider extends Fixture
private function setMockLocationProvider()
{
+ LocationProvider::$providers = array();
+ LocationProvider::$providers[] = new MockLocationProvider();
LocationProvider::setCurrentProvider('mock_provider');
MockLocationProvider::$locations = array(
self::makeLocation('Toronto', 'ON', 'CA', $lat = null, $long = null, $isp = 'comcast.net'),
diff --git a/tests/PHPUnit/System/TrackerTest.php b/tests/PHPUnit/System/TrackerTest.php
index 417b40f72a..65ab91cc08 100644
--- a/tests/PHPUnit/System/TrackerTest.php
+++ b/tests/PHPUnit/System/TrackerTest.php
@@ -150,6 +150,8 @@ class TrackerTest extends IntegrationTestCase
public function test_trackingWithLangParameter_ForwardsLangParameter_ToDefaultLocationProvider()
{
+ LocationProvider::$providers = null;
+ LocationProvider::getAllProviders();
LocationProvider::setCurrentProvider(LocationProvider\DefaultProvider::ID);
$urlToTest = "?idsite=1&rec=1&action_name=test&lang=fr-be";
--
GitLab