diff --git a/core/DataAccess/Actions.php b/core/DataAccess/Actions.php new file mode 100644 index 00000000000..9efc4fc2a59 --- /dev/null +++ b/core/DataAccess/Actions.php @@ -0,0 +1,34 @@ +getColumns($table); + + $columns = array_filter($columns, function ($columnName) { + return strpos($columnName, 'idaction') !== false; + }); + + return array_values($columns); + } +} \ No newline at end of file diff --git a/core/Db.php b/core/Db.php index 67618c8f58d..a96786e1e39 100644 --- a/core/Db.php +++ b/core/Db.php @@ -9,6 +9,7 @@ namespace Piwik; use Exception; +use Piwik\DataAccess\TableMetadata; use Piwik\Db\Adapter; use Piwik\Tracker; @@ -387,17 +388,12 @@ public static function dropAllTables() * * @param string|array $table The name of the table you want to get the columns definition for. * @return \Zend_Db_Statement + * @deprecated since 2.11.0 */ public static function getColumnNamesFromTable($table) { - $columns = self::fetchAll("SHOW COLUMNS FROM `" . $table . "`"); - - $columnNames = array(); - foreach ($columns as $column) { - $columnNames[] = $column['Field']; - } - - return $columnNames; + $tableMetadataAccess = new TableMetadata(); + return $tableMetadataAccess->getColumns($table); } /** diff --git a/core/Tracker/Model.php b/core/Tracker/Model.php index 090cd8be596..e40f845e009 100644 --- a/core/Tracker/Model.php +++ b/core/Tracker/Model.php @@ -9,10 +9,8 @@ namespace Piwik\Tracker; use Exception; -use PDOStatement; use Piwik\Common; use Piwik\Tracker; -use Piwik\Tracker\Db\DbException; class Model { @@ -142,7 +140,31 @@ public function createEcommerceItems($ecommerceItems) Common::printDebug($bind); } + /** + * Inserts a new action into the log_action table. If there is an existing action that was inserted + * due to another request pre-empting this one, the newly inserted action is deleted. + * + * @param string $name + * @param int $type + * @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) + { + $newActionId = $this->insertNewAction($name, $type, $urlPrefix); + + $realFirstActionId = $this->getIdActionMatchingNameAndType($name, $type); + + // if the inserted action ID is not the same as the queried action ID, then that means we inserted + // a duplicate, so remove it now + if ($realFirstActionId != $newActionId) { + $this->deleteDuplicateAction($newActionId); + } + + return $realFirstActionId; + } + + private function insertNewAction($name, $type, $urlPrefix) { $table = Common::prefixTable('log_action'); $sql = "INSERT INTO $table (name, hash, type, url_prefix) VALUES (?,CRC32(?),?,?)"; @@ -157,8 +179,11 @@ public function createNewIdAction($name, $type, $urlPrefix) private function getSqlSelectActionId() { + // it is possible for multiple actions to exist in the DB (due to rare concurrency issues), so the ORDER BY and + // LIMIT are important $sql = "SELECT idaction, type, name FROM " . Common::prefixTable('log_action') - . " WHERE ( hash = CRC32(?) AND name = ? AND type = ? ) "; + . " WHERE " . $this->getSqlConditionToMatchSingleAction() . " " + . "ORDER BY idaction ASC LIMIT 1"; return $sql; } @@ -173,9 +198,16 @@ public function getIdActionMatchingNameAndType($name, $type) return $idAction; } + /** + * Returns the IDs for multiple actions based on name + type values. + * + * @param array $actionsNameAndType Array like `array( array('name' => '...', 'type' => 1), ... )` + * @return array|false Array of DB rows w/ columns: **idaction**, **type**, **name**. + */ public function getIdsAction($actionsNameAndType) { - $sql = $this->getSqlSelectActionId(); + $sql = "SELECT MIN(idaction) as idaction, type, name FROM " . Common::prefixTable('log_action') + . " WHERE"; $bind = array(); $i = 0; @@ -187,15 +219,19 @@ public function getIdsAction($actionsNameAndType) } if ($i > 0) { - $sql .= " OR ( hash = CRC32(?) AND name = ? AND type = ? ) "; + $sql .= " OR"; } + $sql .= " " . $this->getSqlConditionToMatchSingleAction() . " "; + $bind[] = $name; $bind[] = $name; $bind[] = $actionNameType['type']; $i++; } + $sql .= " GROUP BY type, hash, name"; + // Case URL & Title are empty if (empty($bind)) { return false; @@ -375,9 +411,21 @@ private function visitFieldsToQuery($valuesToUpdate) return array($updateParts, $sqlBind); } + private function deleteDuplicateAction($newActionId) + { + $sql = "DELETE FROM " . Common::prefixTable('log_action') . " WHERE idaction = ?"; + + $db = $this->getDb(); + $db->query($sql, array($newActionId)); + } + private function getDb() { return Tracker::getDatabase(); } + private function getSqlConditionToMatchSingleAction() + { + return "( hash = CRC32(?) AND name = ? AND type = ? )"; + } } diff --git a/core/Tracker/TableLogAction.php b/core/Tracker/TableLogAction.php index fe620035d72..4308f042f86 100644 --- a/core/Tracker/TableLogAction.php +++ b/core/Tracker/TableLogAction.php @@ -61,7 +61,10 @@ private static function getSelectQueryWhereNameContains($matchType, $actionType) { // now, we handle the cases =@ (contains) and !@ (does not contain) // build the expression based on the match type - $sql = 'SELECT idaction FROM ' . Common::prefixTable('log_action') . ' WHERE %s AND type = ' . $actionType . ' )'; + $sql = 'SELECT MIN(idaction) AS idaction FROM ' . Common::prefixTable('log_action') . ' WHERE %s AND type = ' . $actionType . ' )' + . ' 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/core/Updates/2.11.0-b2.php b/core/Updates/2.11.0-b2.php index b0984433abe..ce0c7b25335 100644 --- a/core/Updates/2.11.0-b2.php +++ b/core/Updates/2.11.0-b2.php @@ -6,7 +6,6 @@ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later * */ - namespace Piwik\Updates; use Piwik\Common; diff --git a/plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php b/plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php new file mode 100644 index 00000000000..54b53afabe9 --- /dev/null +++ b/plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php @@ -0,0 +1,215 @@ +archiveInvalidator = $invalidator; + + if ($duplicateActionRemover === null) { + $duplicateActionRemover = new DuplicateActionRemover(); + } + $this->duplicateActionRemover = $duplicateActionRemover; + + if ($actionsAccess === null) { + $actionsAccess = new Actions(); + } + $this->actionsAccess = $actionsAccess; + + if ($logger === null) { + $logger = StaticContainer::get('Psr\Log\LoggerInterface'); + } + $this->logger = $logger; + } + + protected function configure() + { + $this->setName('core:fix-duplicate-log-actions'); + $this->addOption('invalidate-archives', null, InputOption::VALUE_NONE, "If supplied, archives for logs that use duplicate actions will be invalidated." + . " On the next cron archive run, the reports for those dates will be re-processed."); + $this->setDescription('Removes duplicates in the log action table and fixes references to the duplicates in ' + . 'related tables. NOTE: This action can take a long time to run!'); + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + $invalidateArchives = $input->getOption('invalidate-archives'); + + $timer = new Timer(); + + $duplicateActions = $this->duplicateActionRemover->getDuplicateIdActions(); + if (empty($duplicateActions)) { + $output->writeln("Found no duplicate actions."); + return; + } + + $output->writeln("Found " . count($duplicateActions) . " actions with duplicates."); + + list($numberRemoved, $allArchivesAffected) = $this->fixDuplicateActionReferences($duplicateActions, $output); + + $this->deleteDuplicatesFromLogAction($output, $duplicateActions); + + if ($invalidateArchives) { + $this->invalidateArchivesUsingActionDuplicates($allArchivesAffected, $output); + } else { + $this->printAffectedArchives($allArchivesAffected, $output); + } + + $logActionTable = Common::prefixTable('log_action'); + $this->writeSuccessMessage($output, array( + "Found and deleted $numberRemoved duplicate action entries in the $logActionTable table.", + "References in log_link_visit_action, log_conversion and log_conversion_item were corrected.", + $timer->__toString() + )); + } + + private function invalidateArchivesUsingActionDuplicates($archivesAffected, OutputInterface $output) + { + $output->write("Invalidating archives affected by duplicates fixed..."); + foreach ($archivesAffected as $archiveInfo) { + $this->archiveInvalidator->markArchivesAsInvalidated( + array($archiveInfo['idsite']), $archiveInfo['server_time'], $period = false); + } + $output->writeln("Done."); + } + + private function printAffectedArchives($allArchivesAffected, OutputInterface $output) + { + $output->writeln("The following archives used duplicate actions and should be invalidated if you want correct reports:"); + foreach ($allArchivesAffected as $archiveInfo) { + $output->writeln("\t[ idSite = {$archiveInfo['idsite']}, date = {$archiveInfo['server_time']} ]"); + } + } + + private function fixDuplicateActionReferences($duplicateActions, OutputInterface $output) + { + $dupeCount = count($duplicateActions); + + $numberRemoved = 0; + $allArchivesAffected = array(); + + foreach ($duplicateActions as $index => $dupeInfo) { + $name = $dupeInfo['name']; + $toIdAction = $dupeInfo['idaction']; + $fromIdActions = $dupeInfo['duplicateIdActions']; + + $numberRemoved += count($fromIdActions); + + $output->writeln("[$index / $dupeCount] Fixing duplicates for '$name'"); + + $this->logger->debug(" idaction = {idaction}, duplicate idactions = {duplicateIdActions}", array( + 'idaction' => $toIdAction, + 'duplicateIdActions' => $fromIdActions + )); + + foreach (DuplicateActionRemover::$tablesWithIdActionColumns as $table) { + $archivesAffected = $this->fixDuplicateActionsInTable($output, $table, $toIdAction, $fromIdActions); + $allArchivesAffected = array_merge($allArchivesAffected, $archivesAffected); + } + } + + $allArchivesAffected = array_values(array_unique($allArchivesAffected, SORT_REGULAR)); + + return array($numberRemoved, $allArchivesAffected); + } + + private function fixDuplicateActionsInTable(OutputInterface $output, $table, $toIdAction, $fromIdActions) + { + $timer = new Timer(); + + $archivesAffected = $this->duplicateActionRemover->getSitesAndDatesOfRowsUsingDuplicates($table, $fromIdActions); + + $this->duplicateActionRemover->fixDuplicateActionsInTable($table, $toIdAction, $fromIdActions); + + $output->writeln("\tFixed duplicates in " . Common::prefixTable($table) . ". " . $timer->__toString() . "."); + + return $archivesAffected; + } + + private function deleteDuplicatesFromLogAction(OutputInterface $output, $duplicateActions) + { + $logActionTable = Common::prefixTable('log_action'); + $output->writeln("Deleting duplicate actions from $logActionTable..."); + + $idActions = array(); + foreach ($duplicateActions as $dupeInfo) { + $idActions = array_merge($idActions, $dupeInfo['duplicateIdActions']); + } + + $this->actionsAccess->delete($idActions); + } +} \ No newline at end of file diff --git a/plugins/CoreAdminHome/Model/DuplicateActionRemover.php b/plugins/CoreAdminHome/Model/DuplicateActionRemover.php new file mode 100644 index 00000000000..51a93bab472 --- /dev/null +++ b/plugins/CoreAdminHome/Model/DuplicateActionRemover.php @@ -0,0 +1,193 @@ +tableMetadataAccess = $tableMetadataAccess; + + if ($logger === null) { + $logger = StaticContainer::get('Psr\Log\LoggerInterface'); + } + $this->logger = $logger; + + $this->idactionColumns = $this->getIdActionTableColumnsFromMetadata(); + } + + /** + * Returns list of all duplicate actions in the log_action table by name and the lowest action ID. + * The duplicate actions are returned with each action. + * + * @return array Contains the following elements: + * + * * **name**: The action's name. + * * **idaction**: The action's ID. + * * **duplicateIdActions**: An array of duplicate action IDs. + */ + public function getDuplicateIdActions() + { + $sql = "SELECT name, COUNT(*) AS count, GROUP_CONCAT(idaction ORDER BY idaction ASC SEPARATOR ',') as idactions + FROM " . Common::prefixTable('log_action') . " + GROUP BY name, hash, type HAVING count > 1"; + + $result = array(); + foreach (Db::fetchAll($sql) as $row) { + $dupeInfo = array('name' => $row['name']); + + $idActions = explode(",", $row['idactions']); + $dupeInfo['idaction'] = array_shift($idActions); + $dupeInfo['duplicateIdActions'] = $idActions; + + $result[] = $dupeInfo; + } + return $result; + } + + /** + * Executes one SQL statement that sets all idaction columns in a table to a single value, if the + * values of those columns are in the specified set (`$duplicateIdActions`). + * + * Notes: + * + * The SQL will look like: + * + * UPDATE $table SET + * col1 = IF((col1 IN ($duplicateIdActions)), $realIdAction, col1), + * col2 = IF((col2 IN ($duplicateIdActions)), $realIdAction, col2), + * ... + * WHERE col1 IN ($duplicateIdActions) OR col2 IN ($duplicateIdActions) OR ... + * + * @param string $table + * @param int $realIdAction The idaction to set column values to. + * @param int[] $duplicateIdActions The idaction values that should be changed. + */ + public function fixDuplicateActionsInTable($table, $realIdAction, $duplicateIdActions) + { + $idactionColumns = array_values($this->idactionColumns[$table]); + $table = Common::prefixTable($table); + + $inFromIdsExpression = $this->getInFromIdsExpression($duplicateIdActions); + $setExpression = "%1\$s = IF(($inFromIdsExpression), $realIdAction, %1\$s)"; + + $sql = "UPDATE $table SET\n"; + foreach ($idactionColumns as $index => $column) { + if ($index != 0) { + $sql .= ",\n"; + } + $sql .= sprintf($setExpression, $column); + } + $sql .= $this->getWhereToGetRowsUsingDuplicateActions($idactionColumns, $duplicateIdActions); + + Db::query($sql); + } + + /** + * Returns the server time and idsite of rows in a log table that reference at least one action + * in a set. + * + * @param string $table + * @param int[] $duplicateIdActions + * @return array with two elements **idsite** and **server_time**. idsite is the site ID and server_time + * is the date of the log. + */ + public function getSitesAndDatesOfRowsUsingDuplicates($table, $duplicateIdActions) + { + $idactionColumns = array_values($this->idactionColumns[$table]); + $table = Common::prefixTable($table); + + $sql = "SELECT idsite, DATE(server_time) as server_time FROM $table "; + $sql .= $this->getWhereToGetRowsUsingDuplicateActions($idactionColumns, $duplicateIdActions); + return Db::fetchAll($sql); + } + + private function getIdActionTableColumnsFromMetadata() + { + $result = array(); + foreach (self::$tablesWithIdActionColumns as $table) { + $columns = $this->tableMetadataAccess->getIdActionColumnNames(Common::prefixTable($table)); + + $this->logger->debug("Found following idactions in {table}: {columns}", array( + 'table' => $table, + 'columns' => implode(',', $columns) + )); + + $result[$table] = $columns; + } + return $result; + } + + private function getWhereToGetRowsUsingDuplicateActions($idactionColumns, $fromIdActions) + { + $sql = "WHERE "; + foreach ($idactionColumns as $index => $column) { + if ($index != 0) { + $sql .= "OR "; + } + + $sql .= sprintf($this->getInFromIdsExpression($fromIdActions), $column) . " "; + } + return $sql; + } + + private function getInFromIdsExpression($fromIdActions) + { + return "%1\$s IN (" . implode(',', $fromIdActions) . ")"; + } +} \ No newline at end of file diff --git a/plugins/CoreAdminHome/tests/Fixture/DuplicateActions.php b/plugins/CoreAdminHome/tests/Fixture/DuplicateActions.php new file mode 100644 index 00000000000..7d98a57670e --- /dev/null +++ b/plugins/CoreAdminHome/tests/Fixture/DuplicateActions.php @@ -0,0 +1,171 @@ + array( + array('name' => 'action1', 'type' => 1), + array('name' => 'action1', 'type' => 1), + array('name' => 'action1', 'type' => 1), + + array('name' => 'action2', 'type' => 2), + array('name' => 'ACTION2', 'type' => 1), + array('name' => 'action4', 'type' => 3), + array('name' => 'ACTION2', 'type' => 1), + array('name' => 'action5', 'type' => 2), + + array('name' => 'action2', 'type' => 2), + array('name' => 'action4', 'type' => 3), + array('name' => 'ACTION2', 'type' => 1), + array('name' => 'action4', 'type' => 3), + ), + 'log_link_visit_action' => array( + array( + 'idsite' => 1, + 'idvisitor' => self::DUMMY_IDVISITOR, + 'idvisit' => 1, + 'server_time' => '2012-01-01 00:00:00', + 'time_spent_ref_action' => 100, + 'idaction_url_ref' => 1, + 'idaction_name_ref' => 2, + 'idaction_name' => 3, + 'idaction_url' => 4, + 'idaction_event_action' => 5, + 'idaction_event_category' => 6, + 'idaction_content_interaction' => 7, + 'idaction_content_name' => 8, + 'idaction_content_piece' => 9, + 'idaction_content_target' => 10, + ), + array( + 'idsite' => 2, + 'idvisitor' => self::DUMMY_IDVISITOR, + 'idvisit' => 2, + 'server_time' => '2013-01-01 00:00:00', + 'time_spent_ref_action' => 120, + 'idaction_url_ref' => 2, + 'idaction_name_ref' => 3, + 'idaction_name' => 5, + 'idaction_url' => 7, + 'idaction_event_action' => 9, + 'idaction_event_category' => 10, + 'idaction_content_interaction' => 11, + 'idaction_content_name' => 11, + 'idaction_content_piece' => 12, + 'idaction_content_target' => 12, + ), + ), + 'log_conversion' => array( + array( + 'idvisit' => 1, + 'idsite' => 1, + 'idvisitor' => self::DUMMY_IDVISITOR, + 'server_time' => '2012-02-01 00:00:00', + 'idgoal' => 1, + 'buster' => 1, + 'url' => 'http://example.com/', + 'location_country' => 'nz', + 'visitor_count_visits' => 1, + 'visitor_returning' => 1, + 'visitor_days_since_order' => 1, + 'visitor_days_since_first' => 1, + 'idaction_url' => 4, + ), + + array( + 'idvisit' => 2, + 'idsite' => 2, + 'idvisitor' => self::DUMMY_IDVISITOR, + 'server_time' => '2012-03-01 00:00:00', + 'idgoal' => 2, + 'buster' => 2, + 'url' => 'http://example.com/', + 'location_country' => 'nz', + 'visitor_count_visits' => 1, + 'visitor_returning' => 1, + 'visitor_days_since_order' => 1, + 'visitor_days_since_first' => 1, + 'idaction_url' => 7, + ) + ), + 'log_conversion_item' => array( + array( + 'idsite' => 1, + 'idvisitor' => self::DUMMY_IDVISITOR, + 'server_time' => '2012-02-01 00:00:00', + 'idvisit' => 1, + 'idorder' => 1, + 'price' => 10, + 'quantity' => 2, + 'deleted' => 0, + 'idaction_sku' => 1, + 'idaction_name' => 2, + 'idaction_category' => 3, + 'idaction_category2' => 4, + 'idaction_category3' => 5, + 'idaction_category4' => 6, + 'idaction_category5' => 7, + ), + array( + 'idsite' => 2, + 'idvisitor' => self::DUMMY_IDVISITOR, + 'server_time' => '2012-01-09 00:00:00', + 'idvisit' => 2, + 'idorder' => 2, + 'price' => 10, + 'quantity' => 1, + 'deleted' => 1, + 'idaction_sku' => 2, + 'idaction_name' => 3, + 'idaction_category' => 5, + 'idaction_category2' => 7, + 'idaction_category3' => 8, + 'idaction_category4' => 9, + 'idaction_category5' => 10, + ) + ) + ); + + public function setUp() + { + parent::setUp(); + + foreach (self::$dataToInsert as $table => $rows) { + self::insertRowData($table, $rows); + } + } + + private static function insertRowData($unprefixedTable, $rows) + { + $table = Common::prefixTable($unprefixedTable); + foreach ($rows as $row) { + if ($unprefixedTable == 'log_action') { + $row['hash'] = crc32($row['name']); + } + + if (isset($row['idvisitor'])) { + $row['idvisitor'] = pack("H*", $row['idvisitor']); + } + + $placeholders = array_map(function () { return "?"; }, $row); + $sql = "INSERT INTO $table (" . implode(',', array_keys($row)) . ") VALUES (" . implode(',', $placeholders) . ")"; + Db::query($sql, array_values($row)); + } + } +} \ No newline at end of file diff --git a/plugins/CoreAdminHome/tests/Integration/FixDuplicateActionsTest.php b/plugins/CoreAdminHome/tests/Integration/FixDuplicateActionsTest.php new file mode 100644 index 00000000000..ca56f3b026c --- /dev/null +++ b/plugins/CoreAdminHome/tests/Integration/FixDuplicateActionsTest.php @@ -0,0 +1,175 @@ +setAutoExit(false); + + $this->applicationTester = new ApplicationTester($application); + } + + public function test_FixDuplicateLogActions_CorrectlyRemovesDuplicates_AndFixesReferencesInOtherTables() + { + $result = $this->applicationTester->run(array( + 'command' => 'core:fix-duplicate-log-actions', + '--invalidate-archives' => 0, + '-vvv' => false + )); + + $this->assertEquals(0, $result, "Command failed: " . $this->applicationTester->getDisplay()); + + $this->assertDuplicateActionsRemovedFromLogActionTable(); + $this->assertDuplicatesFixedInLogLinkVisitActionTable(); + $this->assertDuplicatesFixedInLogConversionTable(); + $this->assertDuplicatesFixedInLogConversionItemTable(); + + $this->assertContains("Found and deleted 7 duplicate action entries", $this->applicationTester->getDisplay()); + + $expectedAffectedArchives = array( + array('idsite' => '1', 'server_time' => '2012-01-01'), + array('idsite' => '2', 'server_time' => '2013-01-01'), + array('idsite' => '1', 'server_time' => '2012-02-01'), + array('idsite' => '2', 'server_time' => '2012-01-09'), + array('idsite' => '2', 'server_time' => '2012-03-01'), + ); + foreach ($expectedAffectedArchives as $archive) { + $this->assertContains("[ idSite = {$archive['idsite']}, date = {$archive['server_time']} ]", $this->applicationTester->getDisplay()); + } + } + + private function assertDuplicateActionsRemovedFromLogActionTable() + { + $actions = Db::fetchAll("SELECT idaction, name FROM " . Common::prefixTable('log_action')); + $expectedActions = array( + array('idaction' => 1, 'name' => 'action1'), + array('idaction' => 4, 'name' => 'action2'), + array('idaction' => 5, 'name' => 'ACTION2'), + array('idaction' => 6, 'name' => 'action4'), + array('idaction' => 8, 'name' => 'action5'), + ); + $this->assertEquals($expectedActions, $actions); + } + + private function assertDuplicatesFixedInLogLinkVisitActionTable() + { + $columns = array( + 'idaction_url_ref', + 'idaction_name_ref', + 'idaction_name', + 'idaction_url', + 'idaction_event_action', + 'idaction_event_category', + 'idaction_content_interaction', + 'idaction_content_name', + 'idaction_content_piece', + 'idaction_content_target' + ); + $rows = Db::fetchAll("SELECT " . implode(',', $columns) . " FROM " . Common::prefixTable('log_link_visit_action')); + $expectedRows = array( + array( + 'idaction_url_ref' => '1', + 'idaction_name_ref' => '1', + 'idaction_name' => '1', + 'idaction_url' => '4', + 'idaction_event_action' => '5', + 'idaction_event_category' => '6', + 'idaction_content_interaction' => '5', + 'idaction_content_name' => '8', + 'idaction_content_piece' => '4', + 'idaction_content_target' => '6' + ), + array( + 'idaction_url_ref' => '1', + 'idaction_name_ref' => '1', + 'idaction_name' => '5', + 'idaction_url' => '5', + 'idaction_event_action' => '4', + 'idaction_event_category' => '6', + 'idaction_content_interaction' => '5', + 'idaction_content_name' => '5', + 'idaction_content_piece' => '6', + 'idaction_content_target' => '6' + ) + ); + $this->assertEquals($expectedRows, $rows); + } + + private function assertDuplicatesFixedInLogConversionTable() + { + $rows = Db::fetchAll("SELECT idaction_url FROM " . Common::prefixTable('log_conversion')); + $expectedRows = array( + array('idaction_url' => 4), + array('idaction_url' => 5) + ); + $this->assertEquals($expectedRows, $rows); + } + + private function assertDuplicatesFixedInLogConversionItemTable() + { + $columns = array( + 'idaction_sku', + 'idaction_name', + 'idaction_category', + 'idaction_category2', + 'idaction_category3', + 'idaction_category4', + 'idaction_category5' + ); + $rows = Db::fetchAll("SELECT " . implode(',', $columns) . " FROM " . Common::prefixTable('log_conversion_item')); + $expectedRows = array( + array( + 'idaction_sku' => '1', + 'idaction_name' => '1', + 'idaction_category' => '1', + 'idaction_category2' => '4', + 'idaction_category3' => '5', + 'idaction_category4' => '6', + 'idaction_category5' => '5' + ), + array( + 'idaction_sku' => '1', + 'idaction_name' => '1', + 'idaction_category' => '5', + 'idaction_category2' => '5', + 'idaction_category3' => '8', + 'idaction_category4' => '4', + 'idaction_category5' => '6' + ) + ); + $this->assertEquals($expectedRows, $rows); + } +} + +FixDuplicateActionsTest::$fixture = new DuplicateActions(); \ No newline at end of file diff --git a/plugins/CoreAdminHome/tests/Integration/Model/DuplicateActionRemoverTest.php b/plugins/CoreAdminHome/tests/Integration/Model/DuplicateActionRemoverTest.php new file mode 100644 index 00000000000..a898ef1616b --- /dev/null +++ b/plugins/CoreAdminHome/tests/Integration/Model/DuplicateActionRemoverTest.php @@ -0,0 +1,112 @@ +duplicateActionRemover = new DuplicateActionRemover(); + } + + public function test_getDuplicateIdActions_ReturnsDuplicateIdActions_AndTreatsLowestIdActionAsOriginal() + { + $expectedResult = array( + array('name' => 'action1', 'idaction' => 1, 'duplicateIdActions' => array(2, 3)), + array('name' => 'ACTION2', 'idaction' => 5, 'duplicateIdActions' => array(7, 11)), + array('name' => 'action2', 'idaction' => 4, 'duplicateIdActions' => array(9)), + array('name' => 'action4', 'idaction' => 6, 'duplicateIdActions' => array(10, 12)), + ); + $actualResult = $this->duplicateActionRemover->getDuplicateIdActions(); + $this->assertEquals($expectedResult, $actualResult); + } + + public function test_fixDuplicateActionsInTable_CorrectlyUpdatesIdActionColumns_InSpecifiedTable() + { + $this->duplicateActionRemover->fixDuplicateActionsInTable('log_conversion_item', 5, array(3, 6, 7, 10)); + + $columns = array('idaction_sku', 'idaction_name', 'idaction_category', 'idaction_category2', + 'idaction_category3', 'idaction_category4', 'idaction_category5'); + + $expectedResult = array( + array( + 'idaction_sku' => '1', + 'idaction_name' => '2', + 'idaction_category' => '5', + 'idaction_category2' => '4', + 'idaction_category3' => '5', + 'idaction_category4' => '5', + 'idaction_category5' => '5' + ), + array( + 'idaction_sku' => '2', + 'idaction_name' => '5', + 'idaction_category' => '5', + 'idaction_category2' => '5', + 'idaction_category3' => '8', + 'idaction_category4' => '9', + 'idaction_category5' => '5' + ), + ); + $actualResult = Db::fetchAll("SELECT " . implode(", ", $columns) . " FROM " . Common::prefixTable('log_conversion_item')); + $this->assertEquals($expectedResult, $actualResult); + } + + public function test_getSitesAndDatesOfRowsUsingDuplicates_ReturnsTheServerTimeAndIdSite_OfRowsUsingSpecifiedActionIds() + { + $row = array( + 'idsite' => 3, + 'idvisitor' => pack("H*", DuplicateActions::DUMMY_IDVISITOR), + 'server_time' => '2012-02-13 00:00:00', + 'idvisit' => 5, + 'idorder' => 6, + 'price' => 15, + 'quantity' => 21, + 'deleted' => 1, + 'idaction_sku' => 3, + 'idaction_name' => 3, + 'idaction_category' => 12, + 'idaction_category2' => 3, + 'idaction_category3' => 3, + 'idaction_category4' => 3, + 'idaction_category5' => 3, + ); + Db::query("INSERT INTO " . Common::prefixTable('log_conversion_item') . " (" . implode(", ", array_keys($row)) + . ") VALUES ('" . implode("', '", array_values($row)) . "')"); + + $expectedResult = array( + array('idsite' => 1, 'server_time' => '2012-02-01'), + array('idsite' => 3, 'server_time' => '2012-02-13') + ); + $actualResult = $this->duplicateActionRemover->getSitesAndDatesOfRowsUsingDuplicates('log_conversion_item', array(4, 6, 12)); + $this->assertEquals($expectedResult, $actualResult); + } +} + +DuplicateActionRemoverTest::$fixture = new DuplicateActions(); \ No newline at end of file diff --git a/tests/PHPUnit/Integration/DataAccess/ActionsTest.php b/tests/PHPUnit/Integration/DataAccess/ActionsTest.php new file mode 100644 index 00000000000..77ce6b08f15 --- /dev/null +++ b/tests/PHPUnit/Integration/DataAccess/ActionsTest.php @@ -0,0 +1,64 @@ +insertTestActions(); + + $this->actionsAccess = new Actions(); + } + + public function test_delete_DeletesSpecifiedActions() + { + $this->actionsAccess->delete(array(2,3,4,5)); + + $expectedActions = array( + array('name' => 'action1') + ); + $actualActions = Db::fetchAll("SELECT name FROM ".Common::prefixTable('log_action')); + $this->assertEquals($expectedActions, $actualActions); + } + + public function test_delete_ConvertsIdActionsToInt() + { + $this->actionsAccess->delete(array("2", "0, 1")); + + $expectedActions = array( + array('name' => 'action1'), + array('name' => 'action3') + ); + $actualActions = Db::fetchAll("SELECT name FROM ".Common::prefixTable('log_action')); + $this->assertEquals($expectedActions, $actualActions); + } + + private function insertTestActions() + { + Db::query("INSERT INTO " . Common::prefixTable('log_action') . " (name, type, hash) + VALUES ('action1', 1, CRC32('action1')), + ('action2', 1, CRC32('action2')), + ('action3', 1, CRC32('action3'))"); + } +} \ No newline at end of file diff --git a/tests/PHPUnit/Integration/DataAccess/TableMetadataTest.php b/tests/PHPUnit/Integration/DataAccess/TableMetadataTest.php new file mode 100644 index 00000000000..47c911bc2be --- /dev/null +++ b/tests/PHPUnit/Integration/DataAccess/TableMetadataTest.php @@ -0,0 +1,55 @@ +tableMetadataAccess = new TableMetadata(); + } + + public function test_getColumns_CorrectlyReturnsListOfColumnNames() + { + $expectedColumns = array('option_name', 'option_value', 'autoload'); + $columns = $this->tableMetadataAccess->getColumns(Common::prefixTable('option')); + $this->assertEquals($expectedColumns, $columns); + } + + /** + * @dataProvider getTablesWithIdActionColumnsToTest + */ + public function test_getIdActionColumnNames_CorrectlyReturnsColumnsWithIdActionName($table, $expectedColumns) + { + $columns = $this->tableMetadataAccess->getIdActionColumnNames(Common::prefixTable($table)); + $this->assertEquals($expectedColumns, $columns); + } + + public function getTablesWithIdActionColumnsToTest() + { + return array( + array('log_conversion', array('idaction_url')), + array('log_conversion_item', array('idaction_sku', 'idaction_name', 'idaction_category', 'idaction_category2', + 'idaction_category3', 'idaction_category4', 'idaction_category5')) + ); + } +} \ No newline at end of file diff --git a/tests/PHPUnit/Integration/Tracker/ModelTest.php b/tests/PHPUnit/Integration/Tracker/ModelTest.php new file mode 100644 index 00000000000..def08a3316c --- /dev/null +++ b/tests/PHPUnit/Integration/Tracker/ModelTest.php @@ -0,0 +1,162 @@ +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 diff --git a/tests/PHPUnit/System/DuplicateActionsTest.php b/tests/PHPUnit/System/DuplicateActionsTest.php new file mode 100644 index 00000000000..7d16329fa00 --- /dev/null +++ b/tests/PHPUnit/System/DuplicateActionsTest.php @@ -0,0 +1,75 @@ +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 287e6799f14..8ca82598466 100755 --- a/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php +++ b/tests/PHPUnit/System/OneVisitorTwoVisitsTest.php @@ -29,7 +29,7 @@ class OneVisitorTwoVisitsTest extends SystemTestCase { /** - * @var Test_Piwik_Fixture_OneVisitorTwoVisits + * @var OneVisitorTwoVisits */ public static $fixture = null; // initialized below class