Refactoring entire branch, moved most of logic to command class, turned...

Refactoring entire branch, moved most of logic to command class, turned DuplicateActionRemover into a DAO that only deals w/ duplicate actions, created two new DAO classes (one for log_action manipulation and one for getting relational table metadata), deprecated SHOW COLUMNS method in Db in favor of new table metadata DAO, and added & fixed tests.

core/DataAccess/Actions.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
+ */
+namespace Piwik\DataAccess;
+
+use Piwik\Db;
+use Piwik\Common;
+
+/**
+ * Data Access Object for operations dealing with the log_action table.
+ */
+class Actions
+{
+    /**
+     * Removes a list of actions from the log_action table by ID.
+     *
+     * @param int[] $idActions
+     */
+    public function delete($idActions)
+    {
+        foreach ($idActions as &$id) {
+            $id = (int)$id;
+        }
+
+        $table = Common::prefixTable('log_action');
+
+        $sql = "DELETE FROM $table WHERE idaction IN (" . implode(",", $idActions) . ")";
+        Db::query($sql);
+    }
+}
\ No newline at end of file

core/DataAccess/TableMetadata.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
+ */
+namespace Piwik\DataAccess;
+
+use Piwik\Common;
+use Piwik\Db;
+
+/**
+ * TODO
+ */
+class TableMetadata
+{
+    /**
+     * TODO
+     */
+    public function getColumns($table)
+    {
+        $table = str_replace("`", "", $table);
+
+        $columns = Db::fetchAll("SHOW COLUMNS FROM `" . $table . "`");
+
+        $columnNames = array();
+        foreach ($columns as $column) {
+            $columnNames[] = $column['Field'];
+        }
+
+        return $columnNames;
+    }
+
+    /**
+     * TODO
+     */
+    public function getIdActionColumnNames($table)
+    {
+        $columns = $this->getColumns($table);
+
+        $columns = array_filter($columns, function ($columnName) {
+            return strpos($columnName, 'idaction') !== false;
+        });
+
+        return array_values($columns);
+    }
+}
\ No newline at end of file

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 @@ class Db
      *
      * @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);
     }
 
     /**

plugins/CoreAdminHome/Commands/FixDuplicateLogActions.php

@@ -9,20 +9,93 @@
 namespace Piwik\Plugins\CoreAdminHome\Commands;
 
 use Piwik\Common;
+use Piwik\Container\StaticContainer;
+use Piwik\DataAccess\Actions;
 use Piwik\DataAccess\ArchiveInvalidator;
 use Piwik\Plugin\ConsoleCommand;
-use Piwik\Plugins\CoreAdminHome\Utility\DuplicateActionRemover;
+use Piwik\Plugins\CoreAdminHome\Model\DuplicateActionRemover;
 use Piwik\Timer;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 /**
- * Removes duplicate log_action rows and fixes references to these duplicate rows
- * in
+ * Finds duplicate actions rows in log_action and removes them. Fixes references to duplicate
+ * actions in the log_link_visit_action table, log_conversion table, and log_conversion_item
+ * table.
+ *
+ * Prior to version 2.11, there was a race condition in the tracker where it was possible for
+ * two or more actions with the same name and type to be inserted simultaneously. This resulted
+ * in inaccurate data. A Piwik database with this problem can be fixed using this class.
+ *
+ * With version 2.11 and above, it is still possible for duplicate actions to be inserted, but
+ * ONLY if the tracker's PHP process fails suddenly right after inserting an action. This is
+ * very rare, and even if it does happen, report data will not be affected, but the extra
+ * actions can be deleted w/ this class.
  */
 class FixDuplicateLogActions extends ConsoleCommand
 {
+    /**
+     * Used to invalidate archives. Only used if $shouldInvalidateArchives is true.
+     *
+     * @var ArchiveInvalidator
+     */
+    private $archiveInvalidator;
+
+    /**
+     * DAO used to find duplicate actions in log_action and fix references to them in other tables.
+     *
+     * @var DuplicateActionRemover
+     */
+    private $duplicateActionRemover;
+
+    /**
+     * DAO used to remove actions from the log_action table.
+     *
+     * @var Actions
+     */
+    private $actionsAccess;
+
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    /**
+     * Constructor.
+     *
+     * @param ArchiveInvalidator $invalidator
+     * @param DuplicateActionRemover $duplicateActionRemover
+     * @param Actions $actionsAccess
+     * @param LoggerInterface $logger
+     */
+    public function __construct(ArchiveInvalidator $invalidator = null, DuplicateActionRemover $duplicateActionRemover = null,
+                                Actions $actionsAccess = null, LoggerInterface $logger = null)
+    {
+        parent::__construct();
+
+        if ($invalidator === null) {
+            $invalidator = new ArchiveInvalidator();
+        }
+        $this->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');
@@ -38,23 +111,105 @@ class FixDuplicateLogActions extends ConsoleCommand
 
         $timer = new Timer();
 
-        $archiveInvalidator = $invalidateArchives ? new ArchiveInvalidator() : null;
-        $resolver = new DuplicateActionRemover($archiveInvalidator);
+        $duplicateActions = $this->duplicateActionRemover->getDuplicateIdActions();
+        if (empty($duplicateActions)) {
+            $output->writeln("Found no duplicate actions.");
+            return;
+        }
 
-        list($numberRemoved, $archivesAffected) = $resolver->removeDuplicateActionsFromDb();
+        $output->writeln("<info>Found " . count($duplicateActions) . " actions with duplicates.</info>");
 
-        if (!$invalidateArchives) {
-            $output->writeln("The following archives used duplicate actions and should be invalidated if you want correct reports:");
-            foreach ($archivesAffected as $archiveInfo) {
-                $output->writeln("\t[ idSite = {$archiveInfo['idsite']}, date = {$archiveInfo['server_time']} ]");
-            }
+        list($numberRemoved, $allArchivesAffected) = $this->fixDuplicateActionReferences($duplicateActions, $output);
+
+        $this->deleteDuplicatesFromLogAction($output, $duplicateActions);
+
+        if ($invalidateArchives) {
+            $this->invalidateArchivesUsingActionDuplicates($allArchivesAffected, $output);
+        } else {
+            $this->printAffectedArchives($allArchivesAffected, $output);
         }
 
-        $table = Common::prefixTable('log_action');
+        $logActionTable = Common::prefixTable('log_action');
         $this->writeSuccessMessage($output, array(
-            "Found and deleted $numberRemoved duplicate action entries in the $table table.",
+            "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("<info>[$index / $dupeCount]</info> 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) . ". <comment>" . $timer->__toString() . "</comment>.");
+
+        return $archivesAffected;
+    }
+
+    private function deleteDuplicatesFromLogAction(OutputInterface $output, $duplicateActions)
+    {
+        $logActionTable = Common::prefixTable('log_action');
+        $output->writeln("<info>Deleting duplicate actions from $logActionTable...</info>");
+
+        $idActions = array();
+        foreach ($duplicateActions as $dupeInfo) {
+            $idActions = array_merge($idActions, $dupeInfo['duplicateIdActions']);
+        }
+
+        $this->actionsAccess->delete($idActions);
+    }
+}
\ No newline at end of file

plugins/CoreAdminHome/Utility/DuplicateActionRemover.php → plugins/CoreAdminHome/Model/DuplicateActionRemover.php

@@ -5,27 +5,17 @@
  * @link http://piwik.org
  * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
  */
-namespace Piwik\Plugins\CoreAdminHome\Utility;
+namespace Piwik\Plugins\CoreAdminHome\Model;
 
 use Piwik\Common;
 use Piwik\Container\StaticContainer;
-use Piwik\DataAccess\ArchiveInvalidator;
+use Piwik\DataAccess\TableMetadata;
 use Piwik\Db;
 use Psr\Log\LoggerInterface;
 
 /**
- * Finds duplicate actions rows in log_action and removes them. Fixes references to duplicate
- * actions in the log_link_visit_action table, log_conversion table, and log_conversion_item
- * table.
- *
- * Prior to version 2.11, there was a race condition in the tracker where it was possible for
- * two or more actions with the same name and type to be inserted simultaneously. This resulted
- * in inaccurate data. A Piwik database with this problem can be fixed using this class.
- *
- * With version 2.11 and above, it is still possible for duplicate actions to be inserted, but
- * ONLY if the tracker's PHP process fails suddenly right after inserting an action. This is
- * very rare, and even if it does happen, report data will not be affected, but the extra
- * actions can be deleted w/ this class.
+ * Provides methods to find duplicate actions and fix duplicate action references in tables
+ * that reference log_action rows.
  */
 class DuplicateActionRemover
 {
@@ -34,15 +24,20 @@ class DuplicateActionRemover
      *
      * @var string[]
      */
-    private static $tablesWithIdActionColumns = array(
+    public static $tablesWithIdActionColumns = array(
         'log_link_visit_action',
         'log_conversion',
         'log_conversion_item'
     );
 
     /**
-     * The logger. Used to log status updates.
+     * DAO used to get idaction column names in tables that reference log_action rows.
      *
+     * @var TableMetadata
+     */
+    private $tableMetadataAccess;
+
+    /**
      * @var LoggerInterface
      */
     private $logger;
@@ -55,158 +50,81 @@ class DuplicateActionRemover
      */
     private $idactionColumns;
 
-    /**
-     * Used to invalidate archives. Only used if $shouldInvalidateArchives is true.
-     *
-     * @var ArchiveInvalidator
-     */
-    private $archiveInvalidator;
-
     /**
      * Constructor.
      *
-     * @param ArchiveInvalidator|null $archiveInvalidator If null, archives are not invalidated.
+     * @param TableMetadata $tableMetadataAccess
+     * @param LoggerInterface $logger
      */
-    public function __construct(ArchiveInvalidator $archiveInvalidator = null)
+    public function __construct($tableMetadataAccess = null, $logger = null)
     {
-        $this->logger = StaticContainer::get('Psr\Log\LoggerInterface');
-        $this->archiveInvalidator = $archiveInvalidator;
-    }
-
-    /**
-     * Performs the duplicate row removal & reference fixing.
-     *
-     * @return int The number of duplicates removed.
-     */
-    public function removeDuplicateActionsFromDb()
-    {
-        $this->getIdActionTableColumnsFromMetadata();
-
-        $duplicateActions = $this->getDuplicateIdActions();
-
-        $this->logger->info("<info>Found {count} actions with duplicates.</info>", array(
-            'count' => count($duplicateActions)
-        ));
-
-        $dupeCount = 0;
-
-        $archivesAffected = array();
-        if (!empty($duplicateActions)) {
-            foreach ($duplicateActions as $index => $dupeInfo) {
-                $name = $dupeInfo['name'];
-                $idactions = $dupeInfo['idactions'];
-
-                $this->logger->info("<info>[$index / $dupeCount]</info> Fixing duplicates for '{name}'", array(
-                    'name' => $name
-                ));
-                $this->logger->debug("  idactions = [ {idactions} ]", array('idactions' => $idactions));
-
-                $idactions = explode(',', $idactions);
-
-                $dupeCount += count($idactions) - 1; // -1, because the first idaction is the one that isn't removed
-
-                $this->fixDuplicateActions($idactions, $archivesAffected);
-            }
-
-            $this->deleteDuplicatesFromLogAction($duplicateActions);
+        if ($tableMetadataAccess === null) {
+            $tableMetadataAccess = new TableMetadata();
+        }
+        $this->tableMetadataAccess = $tableMetadataAccess;
 
-            $archivesAffected = array_values(array_unique($archivesAffected, SORT_REGULAR));
-            if (!empty($this->archiveInvalidator)) {
-                $this->invalidateArchivesUsingActionDuplicates($archivesAffected);
-            }
+        if ($logger === null) {
+            $logger = StaticContainer::get('Psr\Log\LoggerInterface');
         }
+        $this->logger = $logger;
 
-        return array($dupeCount, $archivesAffected);
+        $this->idactionColumns = $this->getIdActionTableColumnsFromMetadata();
     }
 
-    private function getDuplicateIdActions()
+    /**
+     * 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, hash, type, COUNT(*) AS count, GROUP_CONCAT(idaction ORDER BY idaction ASC SEPARATOR ',') as idactions
+        $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";
-        return Db::fetchAll($sql);
-    }
-
-    private function fixDuplicateActions($idactions, &$archivesAffected)
-    {
-        $toIdAction = array_shift($idactions);
-        $fromIdActions = $idactions;
-
-        foreach (self::$tablesWithIdActionColumns as $table) {
-            $this->fixDuplicateActionsInTable($table, $toIdAction, $fromIdActions, $archivesAffected);
-        }
-    }
-
-    private function fixDuplicateActionsInTable($table, $toIdAction, $fromIdActions, &$archivesAffected)
-    {
-        $startTime = microtime(true);
-
-        $sql = $this->getSitesAndDatesOfRowsUsingDuplicates($table, $fromIdActions);
 
+        $result = array();
         foreach (Db::fetchAll($sql) as $row) {
-            $archivesAffected[] = $row;
-        }
-
-        $sql = $this->getSqlToFixDuplicates($table, $toIdAction, $fromIdActions);
+            $dupeInfo = array('name' => $row['name']);
 
-        Db::query($sql);
+            $idActions = explode(",", $row['idactions']);
+            $dupeInfo['idaction'] = array_shift($idActions);
+            $dupeInfo['duplicateIdActions'] = $idActions;
 
-        $endTime = microtime(true);
-        $elapsed = $endTime - $startTime;
-
-        $this->logger->info("\tFixed duplicates in {table} in <comment>{elapsed}s</comment>.", array(
-            'table' => Common::prefixTable($table),
-            'elapsed' => $elapsed
-        ));
-    }
-
-    private function deleteDuplicatesFromLogAction($duplicateActions)
-    {
-        $table = Common::prefixTable('log_action');
-
-        $this->logger->info("<info>Deleting duplicate actions from {table}...</info>", array(
-            'table' => $table
-        ));
-
-        $sql = "DELETE FROM $table WHERE idaction IN (";
-        foreach ($duplicateActions as $index => $dupeInfos) {
-            if ($index != 0) {
-                $sql .= ",";
-            }
-
-            $restIdActions = $dupeInfos['idactions'];
-
-            $commaPos = strpos($restIdActions, ',');
-            if ($commaPos !== false) {
-                $restIdActions = substr($restIdActions, $commaPos + 1);
-            }
-
-            $sql .= $restIdActions;
+            $result[] = $dupeInfo;
         }
-        $sql .= ")";
-
-        Db::query($sql);
+        return $result;
     }
 
     /**
-     * Creates SQL that sets multiple columns in a table to a single value, if those
-     * columns are set to certain values.
+     * 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 ($fromIdActions)), $toIdAction, col1),
-     *         col2 = IF((col2 IN ($fromIdActions)), $toIdAction, col2),
+     *         col1 = IF((col1 IN ($duplicateIdActions)), $realIdAction, col1),
+     *         col2 = IF((col2 IN ($duplicateIdActions)), $realIdAction, col2),
      *         ...
-     *      WHERE col1 IN ($fromIdActions) OR col2 IN ($fromIdActions) OR ...
+     *      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.
      */
-    private function getSqlToFixDuplicates($table, $toIdAction, $fromIdActions)
+    public function fixDuplicateActionsInTable($table, $realIdAction, $duplicateIdActions)
     {
         $idactionColumns = array_values($this->idactionColumns[$table]);
         $table = Common::prefixTable($table);
 
-        $inFromIdsExpression = $this->getInFromIdsExpression($fromIdActions);
-        $setExpression = "%1\$s = IF(($inFromIdsExpression), $toIdAction, %1\$s)";
+        $inFromIdsExpression = $this->getInFromIdsExpression($duplicateIdActions);
+        $setExpression = "%1\$s = IF(($inFromIdsExpression), $realIdAction, %1\$s)";
 
         $sql = "UPDATE $table SET\n";
         foreach ($idactionColumns as $index => $column) {
@@ -215,39 +133,44 @@ class DuplicateActionRemover
             }
             $sql .= sprintf($setExpression, $column);
         }
-        $sql .= $this->getWhereToGetRowsUsingDuplicateActions($idactionColumns, $fromIdActions);
+        $sql .= $this->getWhereToGetRowsUsingDuplicateActions($idactionColumns, $duplicateIdActions);
 
-        return $sql;
+        Db::query($sql);
     }
 
-    private function getSitesAndDatesOfRowsUsingDuplicates($table, $fromIdActions)
+    /**
+     * 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, $fromIdActions);
-        return $sql;
+        $sql .= $this->getWhereToGetRowsUsingDuplicateActions($idactionColumns, $duplicateIdActions);
+        return Db::fetchAll($sql);
     }
 
     private function getIdActionTableColumnsFromMetadata()
     {
+        $result = array();
         foreach (self::$tablesWithIdActionColumns as $table) {
-            $columns = Db::fetchAll("SHOW COLUMNS FROM " . Common::prefixTable($table));
-
-            $columns = array_map(function ($row) { return $row['Field']; }, $columns);
-
-            $columns = array_filter($columns, function ($columnName) {
-                return strpos($columnName, 'idaction') !== false;
-            });
+            $columns = $this->tableMetadataAccess->getIdActionColumnNames(Common::prefixTable($table));
 
             $this->logger->debug("Found following idactions in {table}: {columns}", array(
                 'table' => $table,
                 'columns' => implode(',', $columns)
             ));
 
-            $this->idactionColumns[$table] = $columns;
+            $result[$table] = $columns;
         }
+        return $result;
     }
 
     private function getWhereToGetRowsUsingDuplicateActions($idactionColumns, $fromIdActions)
@@ -267,14 +190,4 @@ class DuplicateActionRemover
     {
         return "%1\$s IN (" . implode(',', $fromIdActions) . ")";
     }
-
-    private function invalidateArchivesUsingActionDuplicates($archivesAffected)
-    {
-        $this->logger->info("Invalidating archives affected by duplicates fixed...");
-        foreach ($archivesAffected as $archiveInfo) {
-            $this->archiveInvalidator->markArchivesAsInvalidated(
-                array($archiveInfo['idsite']), $archiveInfo['server_time'], $period = false);
-        }
-        $this->logger->info("...Done.");
-    }
 }
\ No newline at end of file

plugins/CoreAdminHome/tests/Integration/DuplicateActionRemoverTest.php → plugins/CoreAdminHome/tests/Fixture/DuplicateActions.php

@@ -5,19 +5,16 @@
  * @link http://piwik.org
  * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
  */
-namespace Piwik\Plugins\CoreAdminHome\tests\Integration;
+namespace Piwik\Plugins\CoreAdminHome\tests\Fixture;
 
 use Piwik\Common;
-use Piwik\DataAccess\ArchiveInvalidator;
 use Piwik\Db;
-use Piwik\Plugin\Manager as PluginManager;
-use Piwik\Plugins\CoreAdminHome\Utility\DuplicateActionRemover;
-use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+use Piwik\Tests\Framework\Fixture;
 
 /**
- * @group Core
+ * Fixture that adds log table rows that use duplicate actions.
  */
-class DuplicateActionRemoverTest extends IntegrationTestCase
+class DuplicateActions extends Fixture
 {
     const DUMMY_IDVISITOR = '008c5926ca861023c1d2a36653fd88e2';
 
@@ -143,11 +140,6 @@ class DuplicateActionRemoverTest extends IntegrationTestCase
         )
     );
 
-    /**
-     * @var DuplicateActionRemover
-     */
-    private $remover;
-
     public function setUp()
     {
         parent::setUp();
@@ -155,30 +147,6 @@ class DuplicateActionRemoverTest extends IntegrationTestCase
         foreach (self::$dataToInsert as $table => $rows) {
             self::insertRowData($table, $rows);
         }
-
-        $archiveInvalidator = new ArchiveInvalidator();
-        $this->remover = new DuplicateActionRemover($archiveInvalidator);
-    }
-
-    public function test_DuplicateActionRemover_CorrectlyRemovesDuplicates_AndFixesReferencesInOtherTables()
-    {
-        list($duplicatesCount, $affectedArchives) = $this->remover->removeDuplicateActionsFromDb();
-
-        $this->assertDuplicateActionsRemovedFromLogActionTable();
-        $this->assertDuplicatesFixedInLogLinkVisitActionTable();
-        $this->assertDuplicatesFixedInLogConversionTable();
-        $this->assertDuplicatesFixedInLogConversionItemTable();
-
-        $this->assertEquals(7, $duplicatesCount);
-
-        $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'),
-        );
-        $this->assertEquals($expectedAffectedArchives, $affectedArchives);
     }
 
     private static function insertRowData($unprefixedTable, $rows)
@@ -198,106 +166,4 @@ class DuplicateActionRemoverTest extends IntegrationTestCase
             Db::query($sql, array_values($row));
         }
     }
-
-    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);
-    }
 }
\ No newline at end of file

plugins/CoreAdminHome/tests/Integration/FixDuplicateActionsTest.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
+ */
+namespace Piwik\Plugins\CoreAdminHome\tests\Integration;
+
+use Piwik\Common;
+use Piwik\Console;
+use Piwik\Db;
+use Piwik\Plugin\Manager as PluginManager;
+use Piwik\Plugins\CoreAdminHome\tests\Fixture\DuplicateActions;
+use Piwik\Plugins\QueuedTracking\tests\Framework\TestCase\IntegrationTestCase;
+use Symfony\Component\Console\Tester\ApplicationTester;
+
+/**
+ * @group Core
+ */
+class FixDuplicateActionsTest extends IntegrationTestCase
+{
+    /**
+     * @var DuplicateActions
+     */
+    public static $fixture = null;
+
+    /**
+     * @var ApplicationTester
+     */
+    protected $applicationTester = null;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $application = new Console();
+        $application->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

plugins/CoreAdminHome/tests/Integration/Model/DuplicateActionRemoverTest.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
+ */
+namespace Piwik\Plugins\CoreAdminHome\tests\Integration\Model;
+
+use Piwik\Common;
+use Piwik\Db;
+use Piwik\Plugins\CoreAdminHome\Model\DuplicateActionRemover;
+use Piwik\Plugins\CoreAdminHome\tests\Fixture\DuplicateActions;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+/**
+ * @group Core
+ */
+class DuplicateActionRemoverTest extends IntegrationTestCase
+{
+    /**
+     * @var DuplicateActions
+     */
+    public static $fixture = null;
+
+    /**
+     * @var DuplicateActionRemover
+     */
+    private $duplicateActionRemover;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->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' => 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

tests/PHPUnit/Integration/DataAccess/ActionsTest.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
+ */
+namespace Piwik\Tests\Integration\DataAccess;
+
+use Piwik\Common;
+use Piwik\DataAccess\Actions;
+use Piwik\Db;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+/**
+ * @group Core
+ */
+class ActionsTest extends IntegrationTestCase
+{
+    /**
+     * @var Actions
+     */
+    private $actionsAccess;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->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

tests/PHPUnit/Integration/DataAccess/TableMetadataTest.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
+ */
+namespace Piwik\Tests\Integration\DataAccess;
+
+use Piwik\Common;
+use Piwik\DataAccess\TableMetadata;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+/**
+ * @group Core
+ */
+class TableMetadataTest extends IntegrationTestCase
+{
+    /**
+     * @var TableMetadata
+     */
+    private $tableMetadataAccess;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->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