Fixes #6436, fix concurrency issue regarding duplicate actions by always using least idaction and deleting duplicates when they are found to be inserted. Since tracker process can potentially fail before duplicates are removed, added test to make sure reports work when duplicate actions exist in the DB.

2 files changed, 76 insertions, 1 deletions

diff --git a/tests/PHPUnit/System/DuplicateActionsTest.php b/tests/PHPUnit/System/DuplicateActionsTest.php
new file mode 100644
index 0000000000..a9e1c02803
--- /dev/null
+++ b/tests/PHPUnit/System/DuplicateActionsTest.php
@@ -0,0 +1,75 @@
+<?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\Tests\System;
+
+use Piwik\Common;
+use Piwik\Db;
+use Piwik\Tests\Fixtures\OneVisitorTwoVisits;
+use Piwik\Tests\Framework\TestCase\SystemTestCase;
+
+/**
+ * The tracker inserts actions in separate SQL queries which can cause
+ * duplicate actions to be in the DB (actions w/ the same name, hash + type, but
+ * different idaction). The tracker will delete duplicate actions, but
+ * if for some reason the tracker fails before the DELETE occurs, there can be
+ * stray duplicate actions. This test is there to ensure reports are not affected
+ * by duplicate action entries.
+ *
+ * @group Core
+ * @group DuplicateActionsTest
+ */
+class DuplicateActionsTest extends SystemTestCase
+{
+    /**
+     * @var OneVisitorTwoVisits
+     */
+    public static $fixture = null; // initialized below class
+
+    public static function setUpBeforeClass()
+    {
+        parent::setUpBeforeClass();
+
+        // add duplicates for every action
+        $table = Common::prefixTable('log_action');
+        foreach (Db::fetchAll("SELECT * FROM $table") as $row) {
+            $insertSql = "INSERT INTO $table (name, type, hash, url_prefix)
+                               VALUES (?, ?, CRC32(?), ?)";
+
+            Db::query($insertSql, array($row['name'], $row['type'], $row['name'], $row['url_prefix']));
+        }
+    }
+
+    /**
+     * @dataProvider getApiForTesting
+     */
+    public function testApi($api, $params)
+    {
+        $this->runApiTests($api, $params);
+    }
+
+    public function getApiForTesting()
+    {
+        $idSite = self::$fixture->idSite;
+        $dateTime = self::$fixture->dateTime;
+
+        $api = array('VisitsSummary', 'Actions', 'Contents', 'Events');
+        return array(
+            array($api, array('idSite' => $idSite,
+                              'periods' => 'day',
+                              'date' => $dateTime,
+                              'compareAgainst' => 'OneVisitorTwoVisits',
+                              'otherRequestParameters' => array(
+                                  'hideColumns' => 'nb_users',
+                              )
+            ))
+        );
+    }
+}
+
+DuplicateActionsTest::$fixture = new OneVisitorTwoVisits();
+DuplicateActionsTest::$fixture->excludeMozilla = true;
\ No newline at end of file
diff --git a/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php b/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php
index 287e6799f1..8ca8259846 100755
--- a/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php
+++ b/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php
@@ -29,7 +29,7 @@ use Exception;
 class OneVisitorTwoVisitsTest extends SystemTestCase
 {
     /**
-     * @var Test_Piwik_Fixture_OneVisitorTwoVisits
+     * @var OneVisitorTwoVisits
      */
     public static $fixture = null; // initialized below class