Merge branch 'master' into readabilityImprovements

Conflicts: core/Tracker/Db/Mysqli.php
matomo-org · Sep 30, 2014 · 019fc10 · 019fc10
2 parents a00487b + ed55938
commit 019fc10
Show file tree

Hide file tree

Showing 20 changed files with 93 additions and 84 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -75,12 +75,12 @@ matrix:
       env: TEST_SUITE=AngularJSTests MYSQL_ADAPTER=PDO_MYSQL
     - php: hhvm
       env: TEST_SUITE=AngularJSTests MYSQL_ADAPTER=PDO_MYSQL
-    # Only run Mysqli tests on PHP 5.3
-    - php: 5.4
+    # Only run Mysqli tests on PHP 5.4
+    - php: 5.3
       env: TEST_SUITE=IntegrationTests MYSQL_ADAPTER=MYSQLI
-    - php: 5.4
+    - php: 5.3
       env: TEST_SUITE=PluginTests MYSQL_ADAPTER=MYSQLI
-    - php: 5.4
+    - php: 5.3
       env: TEST_SUITE=CoreTests MYSQL_ADAPTER=MYSQLI
     - php: 5.5
       env: TEST_SUITE=IntegrationTests MYSQL_ADAPTER=MYSQLI

diff --git a/core/ArchiveProcessor/Rules.php b/core/ArchiveProcessor/Rules.php
@@ -33,8 +33,6 @@ class Rules
 
     const FLAG_TABLE_PURGED = 'lastPurge_';
 
-    public static $purgeOutdatedArchivesIsDisabled = false;
-
     /** Flag that will forcefully disable the archiving process (used in tests only) */
     public static $archivingDisabledByTests = false;
 
@@ -129,16 +127,6 @@ public static function getDoneFlags(array $plugins, Segment $segment, $isSkipAgg
         return $doneFlags;
     }
 
-    public static function disablePurgeOutdatedArchives()
-    {
-        self::$purgeOutdatedArchivesIsDisabled = true;
-    }
-
-    public static function enablePurgeOutdatedArchives()
-    {
-        self::$purgeOutdatedArchivesIsDisabled = false;
-    }
-
     /**
      * Given a monthly archive table, will delete all reports that are now outdated,
      * or reports that ended with an error
@@ -148,9 +136,6 @@ public static function enablePurgeOutdatedArchives()
      */
     public static function shouldPurgeOutdatedArchives(Date $date)
     {
-        if (self::$purgeOutdatedArchivesIsDisabled) {
-            return false;
-        }
         $key = self::FLAG_TABLE_PURGED . "blob_" . $date->toString('Y_m');
         $timestamp = Option::get($key);
 

diff --git a/core/Tracker/Db/Mysqli.php b/core/Tracker/Db/Mysqli.php
@@ -81,8 +81,6 @@ public function connect()
         // See #6296 why this is important in tracker
         $flags = MYSQLI_CLIENT_FOUND_ROWS;
         mysqli_real_connect($this->connection, $this->host, $this->username, $this->password, $this->dbname, $this->port, $this->socket, $flags);
-
-       // $this->connection = mysqli_connect($this->host, $this->username, $this->password, $this->dbname, $this->port, $this->socket);
         if (!$this->connection || mysqli_connect_errno()) {
             throw new DbException("Connect failed: " . mysqli_connect_error());
         }
@@ -214,8 +212,8 @@ public function query($query, $parameters = array())
             return $result;
         } catch (Exception $e) {
             throw new DbException("Error query: " . $e->getMessage() . "
-                                In query: $query
-                                Parameters: " . var_export($parameters, true));
+                                   In query: $query
+                                   Parameters: " . var_export($parameters, true));
         }
     }
 

diff --git a/core/Tracker/Db/Pdo/Mysql.php b/core/Tracker/Db/Pdo/Mysql.php
@@ -68,8 +68,17 @@ public function connect()
             $timer = $this->initProfiler();
         }
 
-        $this->connection = @new PDO($this->dsn, $this->username, $this->password, $config = array());
-        $this->connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+        // Make sure MySQL returns all matched rows on update queries including
+        // rows that actually didn't have to be updated because the values didn't
+        // change. This matches common behaviour among other database systems.
+        // See #6296 why this is important in tracker
+        $config = array(
+            PDO::MYSQL_ATTR_FOUND_ROWS => true,
+            PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
+        );
+
+        $this->connection = @new PDO($this->dsn, $this->username, $this->password, $config);
+
         // we may want to setAttribute(PDO::ATTR_TIMEOUT ) to a few seconds (default is 60) in case the DB is locked
         // the piwik.php would stay waiting for the database... bad!
         // we delete the password from this object "just in case" it could be printed

diff --git a/core/Url.php b/core/Url.php
@@ -227,18 +227,18 @@ public static function isValidHost($host = false)
 
         $trustedHosts = self::getTrustedHosts();
 
+        // Only punctuation we allow is '[', ']', ':', '.', '_' and '-'
+        $hostLength = strlen($host);
+        if ($hostLength !== strcspn($host, '`~!@#$%^&*()+={}\\|;"\'<>,?/ ')) {
+            return false;
+        }
+
         // if no trusted hosts, just assume it's valid
         if (empty($trustedHosts)) {
             self::saveTrustedHostnameInConfig($host);
             return true;
         }
 
-        // Only punctuation we allow is '[', ']', ':', '.' and '-'
-        $hostLength = strlen($host);
-        if ($hostLength !== strcspn($host, '`~!@#$%^&*()_+={}\\|;"\'<>,?/ ')) {
-            return false;
-        }
-
         foreach ($trustedHosts as &$trustedHost) {
             $trustedHost = preg_quote($trustedHost);
         }

diff --git a/misc/proxy-hide-piwik-url/piwik.php b/misc/proxy-hide-piwik-url/piwik.php
@@ -76,7 +76,7 @@ function arrayValue($array, $key, $value = null)
 $url = sprintf("%spiwik.php?cip=%s&token_auth=%s&", $PIWIK_URL, getVisitIp(), $TOKEN_AUTH);
 
 foreach ($_GET as $key => $value) {
-    $url .= $key . '=' . urlencode($value) . '&';
+    $url .= urlencode($key ). '=' . urlencode($value) . '&';
 }
 sendHeader("Content-Type: image/gif");
 $stream_options = array('http' => array(

diff --git a/plugins/Contents/tests/expected/test_Contents__Actions.getPageUrls_day.xml b/plugins/Contents/tests/expected/test_Contents__Actions.getPageUrls_day.xml
@@ -5,18 +5,18 @@
 		<nb_visits>2</nb_visits>
 		<nb_uniq_visitors>2</nb_uniq_visitors>
 		<nb_hits>2</nb_hits>
-		<sum_time_spent>0</sum_time_spent>
+		<sum_time_spent>540</sum_time_spent>
 		<nb_hits_with_time_generation>2</nb_hits_with_time_generation>
 		<min_time_generation>0.333</min_time_generation>
 		<max_time_generation>0.333</max_time_generation>
 		<entry_nb_uniq_visitors>2</entry_nb_uniq_visitors>
 		<entry_nb_visits>2</entry_nb_visits>
 		<entry_nb_actions>2</entry_nb_actions>
-		<entry_sum_visit_length>2</entry_sum_visit_length>
+		<entry_sum_visit_length>542</entry_sum_visit_length>
 		<entry_bounce_count>2</entry_bounce_count>
 		<exit_nb_uniq_visitors>2</exit_nb_uniq_visitors>
 		<exit_nb_visits>2</exit_nb_visits>
-		<avg_time_on_page>0</avg_time_on_page>
+		<avg_time_on_page>270</avg_time_on_page>
 		<bounce_rate>100%</bounce_rate>
 		<exit_rate>100%</exit_rate>
 		<avg_time_generation>0.333</avg_time_generation>

diff --git a/plugins/Contents/tests/expected/test_Contents__Actions.getPageUrls_month.xml b/plugins/Contents/tests/expected/test_Contents__Actions.getPageUrls_month.xml
@@ -4,19 +4,19 @@
 		<label>/page</label>
 		<nb_visits>2</nb_visits>
 		<nb_hits>2</nb_hits>
-		<sum_time_spent>0</sum_time_spent>
+		<sum_time_spent>540</sum_time_spent>
 		<nb_hits_with_time_generation>2</nb_hits_with_time_generation>
 		<min_time_generation>0.333</min_time_generation>
 		<max_time_generation>0.333</max_time_generation>
 		<entry_nb_visits>2</entry_nb_visits>
 		<entry_nb_actions>2</entry_nb_actions>
-		<entry_sum_visit_length>2</entry_sum_visit_length>
+		<entry_sum_visit_length>542</entry_sum_visit_length>
 		<entry_bounce_count>2</entry_bounce_count>
 		<exit_nb_visits>2</exit_nb_visits>
 		<sum_daily_nb_uniq_visitors>2</sum_daily_nb_uniq_visitors>
 		<sum_daily_entry_nb_uniq_visitors>2</sum_daily_entry_nb_uniq_visitors>
 		<sum_daily_exit_nb_uniq_visitors>2</sum_daily_exit_nb_uniq_visitors>
-		<avg_time_on_page>0</avg_time_on_page>
+		<avg_time_on_page>270</avg_time_on_page>
 		<bounce_rate>100%</bounce_rate>
 		<exit_rate>100%</exit_rate>
 		<avg_time_generation>0.333</avg_time_generation>

diff --git a/plugins/CoreHome/templates/_topBarTopMenu.twig b/plugins/CoreHome/templates/_topBarTopMenu.twig
@@ -36,17 +36,8 @@
              menu-title="{{ helloAlias|trim }}"
              piwik-menudropdown>
 
-            {% if userLogin != 'anonymous' %}
-                {% if isAdminLayout is defined and currentModule == 'UsersManager' and currentAction == 'userSettings' %}
-                    <a class="item active" href="index.php?module=CoreAdminHome">{{ 'General_Settings'|translate }}</a>
-                {% else %}
-                    <a class="item" href="index.php?module=CoreAdminHome">{{ 'General_Settings'|translate }}</a>
-                {% endif %}
-            {% endif %}
-
             {% for lev1UserLabel,lev1UserMenu in userMenu if lev1UserLabel|slice(0,1) != '_' %}
-
-                {% if userLogin != 'anonymous' or not loop.first %}
+                {% if not loop.first %}
                     <hr class="item separator"/>
                 {% endif %}
 

diff --git a/plugins/Installation/Controller.php b/plugins/Installation/Controller.php
@@ -317,7 +317,7 @@ public function firstWebsiteSetup()
         $form = new FormFirstWebsiteSetup();
 
         if ($form->validate()) {
-            $name = Common::unsanitizeInputValue($form->getSubmitValue('siteName'));
+            $name = Common::sanitizeInputValue($form->getSubmitValue('siteName'));
             $url = Common::unsanitizeInputValue($form->getSubmitValue('url'));
             $ecommerce = (int)$form->getSubmitValue('ecommerce');
 

diff --git a/plugins/Installation/FormFirstWebsiteSetup.php b/plugins/Installation/FormFirstWebsiteSetup.php
@@ -12,6 +12,7 @@
 use HTML_QuickForm2_DataSource_Array;
 use HTML_QuickForm2_Factory;
 use HTML_QuickForm2_Rule;
+use Piwik\Log;
 use Piwik\Piwik;
 use Piwik\Plugins\SitesManager\API;
 use Piwik\QuickForm2;
@@ -82,6 +83,7 @@ function validateOwner()
                 API::getInstance()->setDefaultTimezone($timezone);
             }
         } catch (\Exception $e) {
+            Log::warning($e->getMessage());
             return false;
         }
         return true;

diff --git a/plugins/UsersManager/Menu.php b/plugins/UsersManager/Menu.php
@@ -9,6 +9,7 @@
 namespace Piwik\Plugins\UsersManager;
 
 use Piwik\Menu\MenuAdmin;
+use Piwik\Menu\MenuUser;
 use Piwik\Piwik;
 
 class Menu extends \Piwik\Plugin\Menu
@@ -20,4 +21,11 @@ public function configureAdminMenu(MenuAdmin $menu)
             $menu->addManageItem('UsersManager_MenuUserSettings', $this->urlForAction('userSettings'), $order = 3);
         }
     }
+
+    public function configureUserMenu(MenuUser $menu)
+    {
+        if (!Piwik::isUserIsAnonymous()) {
+            $menu->addItem('', 'General_Settings', $this->urlForAction('index'), 0);
+        }
+    }
 }
diff --git a/tests/PHPUnit/Integration/Core/Tracker/DbTest.php b/tests/PHPUnit/Integration/Core/Tracker/DbTest.php
@@ -0,0 +1,42 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+use Piwik\Common;
+use Piwik\Db;
+
+/**
+ * Tracker DB test
+ *
+ * @group Core
+ * @group TrackerDbTest
+ */
+class Core_Tracker_DbTest extends DatabaseTestCase
+{
+    public function test_rowCount_whenUpdating_returnsAllMatchedRowsNotOnlyUpdatedRows()
+    {
+        $db = \Piwik\Tracker::getDatabase();
+        // insert one record
+        $db->query("INSERT INTO `" . Common::prefixTable('option') . "` VALUES ('rowid', '1', false)");
+
+        // We will now UPDATE this table and check rowCount() value
+        $sqlUpdate = "UPDATE `" . Common::prefixTable('option') . "` SET option_value = 2";
+
+        // when no record was updated, return 0
+        $result = $db->query($sqlUpdate . " WHERE option_name = 'NOT FOUND'");
+        $this->assertSame(0, $db->rowCount($result));
+
+        // when one record was found and updated, returns 1
+        $result = $db->query($sqlUpdate . " WHERE option_name = 'rowid'");
+        $this->assertSame(1, $db->rowCount($result));
+
+        // when one record was found but NOT actually updated (as values have not changed), we make sure to return 1
+        // testing for MYSQLI_CLIENT_FOUND_ROWS and MYSQL_ATTR_FOUND_ROWS
+        $result = $db->query($sqlUpdate . " WHERE option_name = 'rowid'");
+        $this->assertSame(1, $db->rowCount($result));
+    }
+
+}
diff --git a/tests/PHPUnit/Integration/ManyVisitorsOneWebsiteTest.php b/tests/PHPUnit/Integration/ManyVisitorsOneWebsiteTest.php
@@ -108,6 +108,7 @@ public function getApiForTesting()
 
         // Randomly fails on 5.3
         if(!self::isPhpVersion53()) {
+
             $apiToTest[] = array('Live.getLastVisitsDetails', array(
                 'idSite'                 => $idSite,
                 'date'                   => $dateString,
@@ -140,7 +141,11 @@ public function getApiForTesting()
                 'date'                   => $dateString,
                 'periods'                => 'month',
                 'testSuffix'             => '_Live.getLastVisitsDetails_sortByIdVisitAsc',
-                'otherRequestParameters' => array('filter_sort_order' => 'asc', 'filter_sort_column' => 'idVisit', 'filter_limit' => 7)
+                'otherRequestParameters' => array('filter_sort_order' => 'asc',
+                                                  'filter_sort_column' => 'idVisit',
+                                                  'filter_limit' => 7,
+                                                  'hideColumns' => 'latitude,longitude' // Mysqli has troubles with lat/long rounding
+                )
             ));
         }
 

diff --git a/tests/PHPUnit/Integration/PivotByQueryParamTest.php b/tests/PHPUnit/Integration/PivotByQueryParamTest.php
@@ -43,7 +43,7 @@ public function test_PivotBySubtableDimension_CreatesCorrectPivotTable()
         ));
     }
 
-    public function test_PivotBySubtableDimension_CreatesCorrectPivotTable_WhenEntireHirearchyIsNotLoaded()
+    public function test_PivotBySubtableDimension_WhenEntireHirearchyIsNotLoaded()
     {
         $this->assertApiResponseEqualsExpected("Referrers.getKeywords", array(
             'idSite' => self::$fixture->idSite,
@@ -186,4 +186,4 @@ public function assertApiResponseEqualsExpected($apiMethod, $queryParams)
     }
 }
 
-PivotByQueryParamTest::$fixture = new ManyVisitsWithMockLocationProvider();
+PivotByQueryParamTest::$fixture = new ManyVisitsWithMockLocationProvider();
diff --git a/tests/PHPUnit/Integration/PrivacyManagerTest.php b/tests/PHPUnit/Integration/PrivacyManagerTest.php
@@ -71,15 +71,9 @@ public static function setUpBeforeClass()
     {
         parent::setUpBeforeClass();
 
-        // Temporarily disable the purge of old archives so that getNumeric('nb_visits')
-        // in _addReportData does not trigger the data purge of data we've just imported
-        \Piwik\ArchiveProcessor\Rules::disablePurgeOutdatedArchives();
-
         self::_addLogData();
         self::_addReportData();
 
-        \Piwik\ArchiveProcessor\Rules::enablePurgeOutdatedArchives();
-
         self::$dbData = self::getDbTablesWithData();
     }
 

diff --git a/...bsiteTest_Live.getLastVisitsDetails_sortByIdVisitAsc__Live.getLastVisitsDetails_month.xml b/...bsiteTest_Live.getLastVisitsDetails_sortByIdVisitAsc__Live.getLastVisitsDetails_month.xml
@@ -101,8 +101,6 @@
 		<regionCode>P3</regionCode>
 		<city>Stratford-upon-Avon</city>
 		<location>Stratford-upon-Avon, Warwickshire, United Kingdom</location>
-		<latitude>123.456001</latitude>
-		<longitude>21.320999</longitude>
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
 		<daysSinceLastVisit>0</daysSinceLastVisit>
@@ -257,8 +255,6 @@
 		<regionCode>P3</regionCode>
 		<city>Stratford-upon-Avon</city>
 		<location>Stratford-upon-Avon, Warwickshire, United Kingdom</location>
-		<latitude>123.456001</latitude>
-		<longitude>21.320999</longitude>
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
 		<daysSinceLastVisit>10</daysSinceLastVisit>
@@ -382,8 +378,6 @@
 		<regionCode>A6</regionCode>
 		<city>Besançon</city>
 		<location>Besançon, Franche-Comte, France</location>
-		<latitude>47.249001</latitude>
-		<longitude>6.018000</longitude>
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
 		<daysSinceLastVisit>0</daysSinceLastVisit>
@@ -538,8 +532,6 @@
 		<regionCode>A6</regionCode>
 		<city>Besançon</city>
 		<location>Besançon, Franche-Comte, France</location>
-		<latitude>47.249001</latitude>
-		<longitude>6.018000</longitude>
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
 		<daysSinceLastVisit>10</daysSinceLastVisit>
@@ -663,8 +655,6 @@
 		<regionCode>BC</regionCode>
 		<city>Vancouver</city>
 		<location>Vancouver, British Columbia, Canada</location>
-		<latitude>49.250000</latitude>
-		<longitude>-123.133003</longitude>
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
 		<daysSinceLastVisit>0</daysSinceLastVisit>
@@ -819,8 +809,6 @@
 		<regionCode>BC</regionCode>
 		<city>Vancouver</city>
 		<location>Vancouver, British Columbia, Canada</location>
-		<latitude>49.250000</latitude>
-		<longitude>-123.133003</longitude>
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
 		<daysSinceLastVisit>10</daysSinceLastVisit>
@@ -936,8 +924,6 @@
 		<regionCode />
 		<city />
 		<location>Italy</location>
-		<latitude />
-		<longitude />
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
 		<daysSinceLastVisit>0</daysSinceLastVisit>