Merge pull request #6323 from piwik/6296_alternative_fix

Mysql return rows count found in an UPDATE rather than rows modified

.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

core/Tracker/Db/Mysqli.php

@@ -73,7 +73,14 @@ class Mysqli extends Db
             $timer = $this->initProfiler();
         }
 
-        $this->connection = mysqli_connect($this->host, $this->username, $this->password, $this->dbname, $this->port, $this->socket);
+        $this->connection = mysqli_init();
+
+        // 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
+        $flags = MYSQLI_CLIENT_FOUND_ROWS;
+        mysqli_real_connect($this->connection, $this->host, $this->username, $this->password, $this->dbname, $this->port, $this->socket, $flags);
         if (!$this->connection || mysqli_connect_errno()) {
             throw new DbException("Connect failed: " . mysqli_connect_error());
         }

core/Tracker/Db/Pdo/Mysql.php

@@ -68,8 +68,17 @@ class Mysql extends Db
             $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

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>

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>

tests/PHPUnit/Integration/Core/Tracker/DbTest.php

+<?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));
+    }
+
+}
\ No newline at end of file

tests/PHPUnit/Integration/ManyVisitorsOneWebsiteTest.php

@@ -108,6 +108,7 @@ class ManyVisitorsOneWebsiteTest extends IntegrationTestCase
 
         // Randomly fails on 5.3
         if(!self::isPhpVersion53()) {
+
             $apiToTest[] = array('Live.getLastVisitsDetails', array(
                 'idSite'                 => $idSite,
                 'date'                   => $dateString,
@@ -140,7 +141,11 @@ class ManyVisitorsOneWebsiteTest extends IntegrationTestCase
                 '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
+                )
             ));
         }
 

tests/PHPUnit/Integration/expected/test_ManyVisitorsOneWebsiteTest_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>