Improve AttributeHistoricalDataWithLocationsTest, remove existing geolocation created by fixture in DB, so command will re-attribute, and test API output of UserCountry reports to ensure accuracy. Made the test a System test. Also includes fix for IntegrationTestCase base type.

7 files changed, 313 insertions, 1 deletions

diff --git a/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php b/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php
index ad5d89627b..bf1d2aabc2 100644
--- a/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php
+++ b/plugins/UserCountry/Commands/AttributeHistoricalDataWithLocations.php
@@ -338,4 +338,4 @@ class AttributeHistoricalDataWithLocations extends ConsoleCommand
 
         return false;
     }
-} 
+}
diff --git a/plugins/UserCountry/tests/Integration/AttributeHistoricalDataWithLocationsTest.php b/plugins/UserCountry/tests/System/AttributeHistoricalDataWithLocationsTest.php
index 6bfdc67005..844d42f9e5 100644
--- a/plugins/UserCountry/tests/Integration/AttributeHistoricalDataWithLocationsTest.php
+++ b/plugins/UserCountry/tests/System/AttributeHistoricalDataWithLocationsTest.php
@@ -8,6 +8,9 @@
  */
 namespace Piwik\Plugins\UserCountry\Test\Integration;
 
+use Piwik\Common;
+use Piwik\Db;
+use Piwik\Piwik;
 use Piwik\Plugins\UserCountry\Commands\AttributeHistoricalDataWithLocations;
 use Piwik\Tests\Fixtures\ManyVisitsWithGeoIP;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
@@ -27,6 +30,33 @@ class AttributeHistoricalDataWithLocationsTest extends IntegrationTestCase
      */
     public static $fixture = null;
 
+    public function setUp()
+    {
+        parent::setUp();
+
+        $tablesToUpdate = array('log_visit', 'log_conversion');
+        $columnsToUpdate = array(
+            'location_country' => '"xx"',
+            'location_region' => 'NULL',
+            'location_city' => 'NULL',
+            'location_latitude' => 'NULL',
+            'location_longitude' => 'NULL'
+        );
+
+        foreach ($tablesToUpdate as $table) {
+            $sql = "UPDATE `" . Common::prefixTable($table) . "` SET ";
+
+            $sets = array();
+            foreach ($columnsToUpdate as $column => $defaultValue) {
+                $sets[] = $column . ' = ' . $defaultValue;
+            }
+
+            $sql .= implode(', ', $sets);
+
+            Db::query($sql);
+        }
+    }
+
     /**
      * @expectedException \RuntimeException
      * @expectedExceptionMessage  Not enough arguments
@@ -61,6 +91,17 @@ class AttributeHistoricalDataWithLocationsTest extends IntegrationTestCase
         );
 
         $this->assertRegExp('/100% processed. \[in [(0-9)+] seconds\]/', $result);
+
+        $queryParams = array(
+            'idSite'  => self::$fixture->idSite,
+            'date'    => self::$fixture->dateTime,
+            'period'  => 'month'
+        );
+
+        $this->assertApiResponseEqualsExpected("UserCountry.getCountry", $queryParams);
+        $this->assertApiResponseEqualsExpected("UserCountry.getContinent", $queryParams);
+        $this->assertApiResponseEqualsExpected("UserCountry.getRegion", $queryParams);
+        $this->assertApiResponseEqualsExpected("UserCountry.getCity", $queryParams);
     }
 
     /**
@@ -89,6 +130,16 @@ class AttributeHistoricalDataWithLocationsTest extends IntegrationTestCase
 
         return $result;
     }
+
+    public static function configureFixture($fixture)
+    {
+        // empty (undo IntegrationTestCase configuring)
+    }
+
+    public static function getPathToTestDirectory()
+    {
+        return __DIR__;
+    }
 }
 
 AttributeHistoricalDataWithLocationsTest::$fixture = new ManyVisitsWithGeoIP();
\ No newline at end of file
diff --git a/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getCity_month.xml b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getCity_month.xml
new file mode 100644
index 0000000000..23c40113d8
--- /dev/null
+++ b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getCity_month.xml
@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<result>
+	<row>
+		<label>Unknown</label>
+		<nb_visits>35</nb_visits>
+		<nb_actions>61</nb_actions>
+		<max_actions>3</max_actions>
+		<sum_visit_length>21437</sum_visit_length>
+		<bounce_count>18</bounce_count>
+		<goals>
+			<row idgoal='1'>
+				<nb_conversions>35</nb_conversions>
+				<nb_visits_converted>35</nb_visits_converted>
+				<revenue>175</revenue>
+			</row>
+			<row idgoal='2'>
+				<nb_conversions>17</nb_conversions>
+				<nb_visits_converted>17</nb_visits_converted>
+				<revenue>85</revenue>
+			</row>
+		</goals>
+		<nb_conversions>52</nb_conversions>
+		<revenue>260</revenue>
+		<sum_daily_nb_uniq_visitors>18</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_users>1</sum_daily_nb_users>
+		<city_name>Unknown</city_name>
+		<city>xx</city>
+		<region>xx</region>
+		<country>xx</country>
+		<country_name>Unknown</country_name>
+		<region_name>Unknown</region_name>
+		<logo>plugins/UserCountry/images/flags/xx.png</logo>
+	</row>
+</result>
\ No newline at end of file
diff --git a/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getContinent_month.xml b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getContinent_month.xml
new file mode 100644
index 0000000000..5882ac249d
--- /dev/null
+++ b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getContinent_month.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<result>
+	<row>
+		<label>North America</label>
+		<nb_visits>35</nb_visits>
+		<nb_actions>61</nb_actions>
+		<max_actions>3</max_actions>
+		<sum_visit_length>21437</sum_visit_length>
+		<bounce_count>18</bounce_count>
+		<goals>
+			<row idgoal='1'>
+				<nb_conversions>35</nb_conversions>
+				<nb_visits_converted>35</nb_visits_converted>
+				<revenue>175</revenue>
+			</row>
+			<row idgoal='2'>
+				<nb_conversions>17</nb_conversions>
+				<nb_visits_converted>17</nb_visits_converted>
+				<revenue>85</revenue>
+			</row>
+		</goals>
+		<nb_conversions>52</nb_conversions>
+		<revenue>260</revenue>
+		<sum_daily_nb_uniq_visitors>18</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_users>1</sum_daily_nb_users>
+		<code>North America</code>
+	</row>
+</result>
\ No newline at end of file
diff --git a/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getCountry_month.xml b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getCountry_month.xml
new file mode 100644
index 0000000000..ee389b9ac8
--- /dev/null
+++ b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getCountry_month.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<result>
+	<row>
+		<label>United States</label>
+		<nb_visits>35</nb_visits>
+		<nb_actions>61</nb_actions>
+		<max_actions>3</max_actions>
+		<sum_visit_length>21437</sum_visit_length>
+		<bounce_count>18</bounce_count>
+		<goals>
+			<row idgoal='1'>
+				<nb_conversions>35</nb_conversions>
+				<nb_visits_converted>35</nb_visits_converted>
+				<revenue>175</revenue>
+			</row>
+			<row idgoal='2'>
+				<nb_conversions>17</nb_conversions>
+				<nb_visits_converted>17</nb_visits_converted>
+				<revenue>85</revenue>
+			</row>
+		</goals>
+		<nb_conversions>52</nb_conversions>
+		<revenue>260</revenue>
+		<sum_daily_nb_uniq_visitors>18</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_users>1</sum_daily_nb_users>
+		<code>us</code>
+		<logo>plugins/UserCountry/images/flags/us.png</logo>
+		<segment>countryCode==us</segment>
+		<logoWidth>16</logoWidth>
+		<logoHeight>11</logoHeight>
+	</row>
+</result>
\ No newline at end of file
diff --git a/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getRegion_month.xml b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getRegion_month.xml
new file mode 100644
index 0000000000..bb934e5ce6
--- /dev/null
+++ b/plugins/UserCountry/tests/System/expected/test_AttributeHistoricalDataWithLocationsTest_testExecute_ShouldReturnLogAfterWorkingWithSomeData_IfThereAreData__UserCountry.getRegion_month.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<result>
+	<row>
+		<label>Unknown</label>
+		<nb_visits>35</nb_visits>
+		<nb_actions>61</nb_actions>
+		<max_actions>3</max_actions>
+		<sum_visit_length>21437</sum_visit_length>
+		<bounce_count>18</bounce_count>
+		<goals>
+			<row idgoal='1'>
+				<nb_conversions>35</nb_conversions>
+				<nb_visits_converted>35</nb_visits_converted>
+				<revenue>175</revenue>
+			</row>
+			<row idgoal='2'>
+				<nb_conversions>17</nb_conversions>
+				<nb_visits_converted>17</nb_visits_converted>
+				<revenue>85</revenue>
+			</row>
+		</goals>
+		<nb_conversions>52</nb_conversions>
+		<revenue>260</revenue>
+		<sum_daily_nb_uniq_visitors>18</sum_daily_nb_uniq_visitors>
+		<sum_daily_nb_users>1</sum_daily_nb_users>
+		<region>xx</region>
+		<country>xx</country>
+		<country_name>Unknown</country_name>
+		<region_name>Unknown</region_name>
+		<logo>plugins/UserCountry/images/flags/xx.png</logo>
+	</row>
+</result>
\ No newline at end of file
diff --git a/plugins/UserCountry/tests/Unit/VisitorGeolocatorTest.php b/plugins/UserCountry/tests/Unit/VisitorGeolocatorTest.php
new file mode 100644
index 0000000000..b14cced3d9
--- /dev/null
+++ b/plugins/UserCountry/tests/Unit/VisitorGeolocatorTest.php
@@ -0,0 +1,135 @@
+<?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\VisitorGeolocator;
+use Piwik\Plugins\UserCountry\LocationProvider;
+use Piwik\Tracker\Cache;
+use Piwik\Tracker\Visit;
+
+/**
+ * @group UserCountry
+ */
+class VisitorGeolocatorTest extends PHPUnit_Framework_TestCase
+{
+    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));
+
+        $geolocator = new VisitorGeolocator($provider);
+
+        $this->assertEquals(
+            $location,
+            $geolocator->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));
+
+        $geolocator = new VisitorGeolocator($provider);
+
+        $this->assertEquals(
+            array_merge(
+                $location,
+                array(
+                    'country_code' => Visit::UNKNOWN_CODE
+                )
+            ),
+            $geolocator->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']));
+
+        $geolocator = new VisitorGeolocator($poland);
+        $geolocator->getLocation(array('ip' => '10.0.0.1'));
+
+        $nz = $this->getProviderMock();
+        $nz->expects($this->once())
+            ->method('getLocation')
+            ->will($this->returnValue($locations['nz']));
+
+        $geolocator = new VisitorGeolocator($nz);
+        $geolocator->getLocation(array('ip' => '10.0.0.2'));
+
+        $this->assertEquals(
+            $locations,
+            array(
+                'pl' => $geolocator->getLocation(array('ip' => '10.0.0.1')),
+                'nz' => $geolocator->getLocation(array('ip' => '10.0.0.2'))
+            )
+        );
+    }
+
+    public function test_get_shouldReturnDefaultProvider_IfCurrentProviderReturnFalse()
+    {
+        Cache::setCacheGeneral(array('currentLocationProviderId' => 'nonexistant'));
+        $geolocator = new VisitorGeolocator();
+
+        $this->assertEquals(LocationProvider\DefaultProvider::ID, $geolocator->getProvider()->getId());
+    }
+
+    public function test_get_shouldReturnCurrentProvider_IfCurrentProviderIsSet()
+    {
+        Cache::setCacheGeneral(array('currentLocationProviderId' => LocationProvider\GeoIp\Pecl::ID));
+        $geolocator = new VisitorGeolocator();
+
+        $this->assertEquals(LocationProvider\GeoIp\Pecl::ID, $geolocator->getProvider()->getId());
+    }
+
+    /**
+     * @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();
+    }
+}
\ No newline at end of file