From 3e390e5e38a6cbe60831352462d4b87b8bc2a451 Mon Sep 17 00:00:00 2001
From: diosmosis <benaka@piwik.pro>
Date: Sun, 1 Feb 2015 00:51:52 -0800
Subject: [PATCH] Add tests for action ID related methods in Tracker/Model.php
and fix group bys in Tracker/Model.php and TableLogAction.
---
core/Tracker/Model.php | 4 +-
core/Tracker/TableLogAction.php | 6 +-
.../PHPUnit/Integration/Tracker/ModelTest.php | 162 ++++++++++++++++++
3 files changed, 167 insertions(+), 5 deletions(-)
create mode 100644 tests/PHPUnit/Integration/Tracker/ModelTest.php
diff --git a/core/Tracker/Model.php b/core/Tracker/Model.php
index 89ee45b27d..e40f845e00 100644
--- a/core/Tracker/Model.php
+++ b/core/Tracker/Model.php
@@ -146,7 +146,7 @@ class Model
*
* @param string $name
* @param int $type
- * @param string $urlPrefix
+ * @param int $urlPrefix
* @return int The ID of the action (can be for an existing action or new action).
*/
public function createNewIdAction($name, $type, $urlPrefix)
@@ -230,7 +230,7 @@ class Model
$i++;
}
- $sql .= " GROUP BY type, name";
+ $sql .= " GROUP BY type, hash, name";
// Case URL & Title are empty
if (empty($bind)) {
diff --git a/core/Tracker/TableLogAction.php b/core/Tracker/TableLogAction.php
index 346407150f..4308f042f8 100644
--- a/core/Tracker/TableLogAction.php
+++ b/core/Tracker/TableLogAction.php
@@ -62,9 +62,9 @@ class TableLogAction
// now, we handle the cases =@ (contains) and !@ (does not contain)
// build the expression based on the match type
$sql = 'SELECT MIN(idaction) AS idaction FROM ' . Common::prefixTable('log_action') . ' WHERE %s AND type = ' . $actionType . ' )'
- . ' GROUP BY name, type'; // group by is to avoid possible case of duplicates in log_action table
- // (duplicates can exist if php tracker fails right after inserting a duplicate in
- // Tracker\Model::insertNewAction())
+ . ' GROUP BY name, hash, type'; // group by is to avoid possible case of duplicates in log_action table
+ // (duplicates can exist if php tracker fails right after inserting a duplicate in
+ // Tracker\Model::insertNewAction())
switch ($matchType) {
case '=@':
diff --git a/tests/PHPUnit/Integration/Tracker/ModelTest.php b/tests/PHPUnit/Integration/Tracker/ModelTest.php
new file mode 100644
index 0000000000..def08a3316
--- /dev/null
+++ b/tests/PHPUnit/Integration/Tracker/ModelTest.php
@@ -0,0 +1,162 @@
+<?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\Integration\Tracker;
+
+use Piwik\Common;
+use Piwik\Db;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+use Piwik\Tracker\Model;
+
+/**
+ * @group ModelTest
+ * @group Tracker
+ */
+class ModelTest extends IntegrationTestCase
+{
+ const TEST_ACTION_NAME = 'action_name';
+ const TEST_ACTION_TYPE = 1;
+ const TEST_ACTION_URL_PREFIX = 1;
+
+ /**
+ * @var Model
+ */
+ private $model;
+
+ public function setUp()
+ {
+ parent::setUp();
+
+ $this->model = new Model();
+ }
+
+ public function test_createNewIdAction_CreatesNewAction_WhenNoActionWithSameNameAndType()
+ {
+ $newIdAction = $this->model->createNewIdAction(self::TEST_ACTION_NAME, self::TEST_ACTION_TYPE, self::TEST_ACTION_URL_PREFIX);
+
+ $this->assertLogActionTableContainsTestAction($newIdAction);
+ }
+
+ public function test_createNewIdAction_DoesNotCreateDuplicateActions_AndReturnsExistingIdAction_IfActionAlreadyExists()
+ {
+ $this->insertSingleDuplicateAction();
+
+ $newIdAction = $this->model->createNewIdAction(self::TEST_ACTION_NAME, self::TEST_ACTION_TYPE, self::TEST_ACTION_URL_PREFIX);
+
+ $this->assertEquals(5, $newIdAction);
+ $this->assertLogActionTableContainsTestAction(5);
+ }
+
+ public function test_getIdsAction_CorrectlyReturnsRequestedActionIds()
+ {
+ $this->insertManyActions();
+
+ $result = $this->model->getIdsAction(array(
+ array('name' => 'action1', 'type' => 1),
+ array('name' => 'ACTION1', 'type' => 1),
+ array('name' => 'action1', 'type' => 2),
+ array('name' => 'action2', 'type' => 2)
+ ));
+
+ $expectedResult = array(
+ array(
+ 'idaction' => '3',
+ 'type' => '1',
+ 'name' => 'ACTION1'
+ ),
+ array(
+ 'idaction' => '2',
+ 'type' => '1',
+ 'name' => 'action1'
+ ),
+ array(
+ 'idaction' => '5',
+ 'type' => '2',
+ 'name' => 'action2'
+ ),
+ array(
+ 'idaction' => '4',
+ 'type' => '2',
+ 'name' => 'action1'
+ )
+ );
+ $this->assertEquals($expectedResult, $result);
+ }
+
+ public function test_getIdsAction_CorrectlyReturnsLowestIdActions_IfDuplicateIdActionsExistInDb()
+ {
+ $this->insertManyActionsWithDuplicates();
+
+ $result = $this->model->getIdsAction(array(
+ array('name' => 'action1', 'type' => 1),
+ array('name' => 'action2', 'type' => 2)
+ ));
+
+ $expectedResult = array(
+ array(
+ 'idaction' => '1',
+ 'type' => '1',
+ 'name' => 'action1'
+ ),
+ array(
+ 'idaction' => '4',
+ 'type' => '2',
+ 'name' => 'action2'
+ )
+ );
+ $this->assertEquals($expectedResult, $result);
+ }
+
+ private function assertLogActionTableContainsTestAction($idaction)
+ {
+ $expectedRows = array(
+ array(
+ 'idaction' => $idaction,
+ 'name' => self::TEST_ACTION_NAME,
+ 'type' => self::TEST_ACTION_TYPE,
+ 'url_prefix' => self::TEST_ACTION_URL_PREFIX
+ )
+ );
+ $this->assertEquals($expectedRows, Db::fetchAll("SELECT idaction, name, type, url_prefix FROM " . Common::prefixTable('log_action')));
+ }
+
+ private function insertSingleDuplicateAction()
+ {
+ $logActionTable = Common::prefixTable('log_action');
+ Db::query("INSERT INTO $logActionTable (idaction, name, type, url_prefix, hash) VALUES (?, ?, ?, ?, CRC32(?))",
+ array(5, self::TEST_ACTION_NAME, self::TEST_ACTION_TYPE, self::TEST_ACTION_URL_PREFIX,
+ self::TEST_ACTION_NAME));
+ }
+
+ private function insertManyActions()
+ {
+ $logActionTable = Common::prefixTable('log_action');
+ Db::query(
+ "INSERT INTO $logActionTable (idaction, name, type, hash)
+ VALUES (1, 'action0', 1, CRC32('action0')),
+ (2, 'action1', 1, CRC32('action1')),
+ (3, 'ACTION1', 1, CRC32('ACTION1')),
+ (4, 'action1', 2, CRC32('action1')),
+ (5, 'action2', 2, CRC32('action2')),
+ (6, 'action2', 3, CRC32('action2'))"
+ );
+ }
+
+ private function insertManyActionsWithDuplicates()
+ {
+ $logActionTable = Common::prefixTable('log_action');
+ Db::query(
+ "INSERT INTO $logActionTable (idaction, name, type, hash)
+ VALUES (1, 'action1', 1, CRC32('action1')),
+ (2, 'action1', 2, CRC32('action1')),
+ (3, 'action1', 3, CRC32('action1')),
+ (6, 'action2', 2, CRC32('action2')),
+ (5, 'action2', 2, CRC32('action2')),
+ (4, 'action2', 2, CRC32('action2'))"
+ );
+ }
+}
\ No newline at end of file
--
GitLab