More refactoring and fixing VisitorGeolocatorTest (used to be LocationFetcherTest).

6 files changed, 16 insertions, 255 deletions

diff --git a/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php b/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php
index 43fb8042cf..ad5d89627b 100644
--- a/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php
+++ b/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php
@@ -247,24 +247,23 @@ class AttributeHistoricalDataWithLocations extends ConsoleCommand
      */
     protected function parseRowIntoValues(array $row, $ip)
     {
+        $location = $this->locationFetcher->getLocation(array('ip' => $ip));
+
         $values = array();
+        foreach ($this->logVisitFieldsToUpdate as $column => $locationKey) {
+            if (empty($location[$locationKey])) {
+                continue;
+            }
 
-        $location = $this->locationFetcher->getLocation(
-            array(
-                'ip' => $ip
-            )
-        );
+            $locationAttribute = $location[$locationKey];
 
-        foreach ($this->logVisitFieldsToUpdate as $column => $locationKey) {
-            if (!empty($location[$locationKey])) {
-                if ($locationKey === LocationProvider::COUNTRY_CODE_KEY) {
-                    if (strtolower($location[$locationKey]) != strtolower($row[$column])) {
-                        $values[$column] = strtolower($location[$locationKey]);
-                    }
-                } else {
-                    if ($location[$locationKey] != $row[$column]) {
-                        $values[$column] = $location[$locationKey];
-                    }
+            if ($locationKey === LocationProvider::COUNTRY_CODE_KEY) {
+                if (strtolower($locationAttribute) != strtolower($row[$column])) {
+                    $values[$column] = strtolower($locationAttribute);
+                }
+            } else {
+                if ($locationAttribute != $row[$column]) {
+                    $values[$column] = $locationAttribute;
                 }
             }
         }
diff --git a/plugins/UserCountry/VisitorGeolocator.php b/plugins/UserCountry/VisitorGeolocator.php
index 6111aeafa7..0a6ef43098 100644
--- a/plugins/UserCountry/VisitorGeolocator.php
+++ b/plugins/UserCountry/VisitorGeolocator.php
@@ -62,7 +62,7 @@ class VisitorGeolocator
     public function __construct(LocationProvider $provider = null, LocationProvider $backupProvider = null, Cache $locationCache = null)
     {
         if ($provider === null) {
-            // note: Common::getCurrentLocationProviderId() uses the tracker cache, so this should be used here instead
+            // note: Common::getCurrentLocationProviderId() uses the tracker cache, which is why it's used here instead
             // of accessing the option table
             $provider = LocationProvider::getProviderById(Common::getCurrentLocationProviderId());
 
diff --git a/plugins/UserCountry/tests/Integration/AttributeHistoricalDataWithLocationsTest.php b/plugins/UserCountry/tests/Integration/AttributeHistoricalDataWithLocationsTest.php
index 5fdf8febbf..6bfdc67005 100644
--- a/plugins/UserCountry/tests/Integration/AttributeHistoricalDataWithLocationsTest.php
+++ b/plugins/UserCountry/tests/Integration/AttributeHistoricalDataWithLocationsTest.php
@@ -89,15 +89,6 @@ class AttributeHistoricalDataWithLocationsTest extends IntegrationTestCase
 
         return $result;
     }
-
-    protected function getInputStream($input)
-    {
-        $stream = fopen('php://memory', 'r+', false);
-        fputs($stream, $input);
-        rewind($stream);
-
-        return $stream;
-    }
-} 
+}
 
 AttributeHistoricalDataWithLocationsTest::$fixture = new ManyVisitsWithGeoIP();
\ No newline at end of file
diff --git a/plugins/UserCountry/tests/Mock/ProviderGetterMock.php b/plugins/UserCountry/tests/Mock/ProviderGetterMock.php
deleted file mode 100644
index fe18299b06..0000000000
--- a/plugins/UserCountry/tests/Mock/ProviderGetterMock.php
+++ /dev/null
@@ -1,14 +0,0 @@
-<?php
-/**
- * Piwik - free/libre analytics platform
- *
- * @link http://piwik.org
- * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
- *
- */
-namespace Piwik\Plugins\UserCountry\tests\Mock;
-
-interface ProviderGetterMock
-{
-    public function getProviderById($id);
-} 
diff --git a/plugins/UserCountry/tests/Unit/LocationFetcherTest.php b/plugins/UserCountry/tests/Unit/LocationFetcherTest.php
deleted file mode 100644
index 2337309202..0000000000
--- a/plugins/UserCountry/tests/Unit/LocationFetcherTest.php
+++ /dev/null
@@ -1,188 +0,0 @@
-<?php
-/**
- * Piwik - free/libre analytics platform
- *
- * @link http://piwik.org
- * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
- *
- */
-namespace Piwik\Plugins\UserCountry\tests\Unit;
-
-use PHPUnit_Framework_MockObject_MockObject;
-use Piwik\Plugins\UserCountry\VisitorGeolocator;
-use Piwik\Plugins\UserCountry\LocationProvider;
-use Piwik\Tracker\Visit;
-
-/**
- * Class LocationFetcherTest
- * @package Piwik\Plugins\UserCountry\Test\Unit
- *
- * @group UserCountry
- */
-class LocationFetcherTest extends ProviderTest
-{
-    public function test_getLocation_shouldReturnLocationForProvider_IfLocationIsSetForCurrentProvider()
-    {
-        $location = array(
-            'city' => 'Wroclaw',
-            'country_code' => 'pl'
-        );
-
-        $provider = $this->getProviderMock();
-        $provider->expects($this->once())
-            ->method('getLocation')
-            ->will($this->returnValue($location));
-
-        $locationFetcher = $this->getLocationFetcherWithProviderMock($provider, 'mock_provider');
-
-        $this->assertEquals(
-            $location,
-            $locationFetcher->getLocation(array('ip' => '127.0.0.1'))
-        );
-    }
-
-    /**
-     *
-     */
-    public function test_getLocation_shouldReturnLocationForProvider_IfLocationCountryCodeIsNotSetShouldSetAsxx()
-    {
-        $location = array(
-            'city' => 'Wroclaw'
-        );
-
-        $provider = $this->getProviderMock();
-        $provider->expects($this->once())
-            ->method('getLocation')
-            ->will($this->returnValue($location));
-
-        $locationFetcher = $this->getLocationFetcherWithProviderMock($provider, 'mock_provider');
-
-        $this->assertEquals(
-            array_merge(
-                $location,
-                array(
-                    'country_code' => Visit::UNKNOWN_CODE
-                )
-            ),
-            $locationFetcher->getLocation(array('ip' => '127.0.0.2'))
-        );
-    }
-
-    public function test_getLocation_shouldReturnLocationForProviderAndReadFromCacheIfIPIsNotChanged()
-    {
-        $locations = array(
-            'pl' => array(
-                'city' => 'Wroclaw',
-                'country_code' => 'pl'
-            ),
-
-            'nz' => array(
-                'city' => 'Wellington',
-                'country_code' => 'nz'
-            ),
-        );
-
-        $poland = $this->getProviderMock();
-        $poland->expects($this->once())
-            ->method('getLocation')
-            ->will($this->returnValue($locations['pl']));
-
-        $locationFetcher = $this->getLocationFetcherWithProviderMock($poland, 'mock_provider');
-        $locationFetcher->getLocation(array('ip' => '10.0.0.1'));
-
-        $nz = $this->getProviderMock();
-        $nz->expects($this->once())
-            ->method('getLocation')
-            ->will($this->returnValue($locations['nz']));
-
-        $locationFetcher = $this->getLocationFetcherWithProviderMock($nz, 'mock_provider');
-        $locationFetcher->getLocation(array('ip' => '10.0.0.2'));
-
-        $this->assertEquals(
-            $locations,
-            array(
-                'pl' => $locationFetcher->getLocation(array('ip' => '10.0.0.1')),
-                'nz' => $locationFetcher->getLocation(array('ip' => '10.0.0.2'))
-            )
-        );
-    }
-
-    public function test_get_shouldReturnDefaultProvider_IfCurrentProviderReturnFalse()
-    {
-        $providerGetterMock = $this->getMockBuilder('\Piwik\Plugins\UserCountry\tests\Mock\ProviderGetterMock')
-            ->setMethods(array('getProviderById'))
-            ->getMock();
-
-        $providerGetterMock->expects($this->exactly(2))
-            ->method('getProviderById')
-            ->will($this->returnCallback(
-                function ($id)
-                {
-                    switch ($id) {
-                        case 'CurrentProviderId':
-                            return false;
-
-                        case 'DefaultProviderId':
-                            return 'DefaultProvider';
-                    }
-
-                    return false;
-                }
-            ));
-
-        $locationFetcher = new VisitorGeolocator(
-            'CurrentProviderId', array($providerGetterMock, 'getProviderById'), 'DefaultProviderId'
-        );
-
-        $this->assertEquals('DefaultProvider', $locationFetcher->getProvider('DefaultProviderId'));
-    }
-
-    public function test_get_shouldReturnCurrentProvider_IfCurrentProviderIsSet()
-    {
-        $providerGetterMock = $this->getMockBuilder('\Piwik\Plugins\UserCountry\tests\Mock\ProviderGetterMock')
-            ->setMethods(array('getProviderById'))
-            ->getMock();
-
-        $providerGetterMock->expects($this->once())
-            ->method('getProviderById')
-            ->will($this->returnCallback(
-                function ($id)
-                {
-                    switch ($id) {
-                        case 'CurrentProviderId':
-                            return 'CurrentProvider';
-
-                        case 'DefaultProviderId':
-                            return 'DefaultProvider';
-                    }
-
-                    return false;
-                }
-            ));
-
-        $locationFetcher = new VisitorGeolocator(
-            'CurrentProviderId', array($providerGetterMock, 'getProviderById')
-        );
-
-        $this->assertEquals('CurrentProvider', $locationFetcher->getProvider('DefaultProviderId'));
-    }
-
-    /**
-     * @param $provider
-     * @param string $currentLocationProviderId
-     * @return PHPUnit_Framework_MockObject_MockObject|VisitorGeolocator
-     */
-    protected function getLocationFetcherWithProviderMock($provider, $currentLocationProviderId = null)
-    {
-        $locationFetcher = $this->getMockBuilder('\Piwik\Plugins\UserCountry\LocationFetcher')
-            ->setConstructorArgs(array($currentLocationProviderId))
-            ->setMethods(array('getProvider'))
-            ->getMock();
-
-        $locationFetcher->expects($this->once())
-            ->method('getProvider')
-            ->willReturn($provider);
-
-        return $locationFetcher;
-    }
-}
diff --git a/plugins/UserCountry/tests/Unit/ProviderTest.php b/plugins/UserCountry/tests/Unit/ProviderTest.php
deleted file mode 100644
index 9252bb3403..0000000000
--- a/plugins/UserCountry/tests/Unit/ProviderTest.php
+++ /dev/null
@@ -1,27 +0,0 @@
-<?php
-/**
- * Piwik - free/libre analytics platform
- *
- * @link http://piwik.org
- * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
- *
- */
-namespace Piwik\Plugins\UserCountry\tests\Unit;
-
-use PHPUnit_Framework_MockObject_MockObject;
-use PHPUnit_Framework_TestCase;
-use Piwik\Plugins\UserCountry\LocationProvider;
-
-abstract class ProviderTest extends PHPUnit_Framework_TestCase
-{
-    /**
-     * @return PHPUnit_Framework_MockObject_MockObject|LocationProvider
-     */
-    protected function getProviderMock()
-    {
-        return $this->getMockBuilder('\Piwik\Plugins\UserCountry\LocationProvider')
-            ->setMethods(array('getId', 'getLocation', 'isAvailable', 'isWorking', 'getSupportedLocationInfo'))
-            ->disableOriginalConstructor()
-            ->getMock();
-    }
-}