Use log tables to decide which tables can be purged (#10304)

* use log tables to decide which tables can be purged

* make sure to return the number of deleted visits

* fix tests

5 files changed, 37 insertions, 95 deletions

diff --git a/core/DataAccess/RawLogDao.php b/core/DataAccess/RawLogDao.php
index 7310267b44..8095963005 100644
--- a/core/DataAccess/RawLogDao.php
+++ b/core/DataAccess/RawLogDao.php
@@ -114,46 +114,15 @@ class RawLogDao
     }
 
     /**
-     * Deletes visits with the supplied IDs from log_visit. This method does not cascade, so rows in other tables w/
-     * the same visit ID will still exist.
-     *
-     * @param int[] $idVisits
-     * @return int The number of deleted rows.
-     */
-    public function deleteVisits($idVisits)
-    {
-        $sql = "DELETE FROM `" . Common::prefixTable('log_visit') . "` WHERE idvisit IN "
-             . $this->getInFieldExpressionWithInts($idVisits);
-
-        $statement = Db::query($sql);
-        return $statement->rowCount();
-    }
-
-    /**
-     * Deletes visit actions for the supplied visit IDs from log_link_visit_action.
-     *
-     * @param int[] $visitIds
-     * @return int The number of deleted rows.
-     */
-    public function deleteVisitActionsForVisits($visitIds)
-    {
-        $sql = "DELETE FROM `" . Common::prefixTable('log_link_visit_action') . "` WHERE idvisit IN "
-             . $this->getInFieldExpressionWithInts($visitIds);
-
-        $statement = Db::query($sql);
-        return $statement->rowCount();
-    }
-
-    /**
      * Deletes conversions for the supplied visit IDs from log_conversion. This method does not cascade, so
      * conversion items will not be deleted.
      *
      * @param int[] $visitIds
      * @return int The number of deleted rows.
      */
-    public function deleteConversions($visitIds)
+    public function deleteFromLogTable($tableName, $visitIds)
     {
-        $sql = "DELETE FROM `" . Common::prefixTable('log_conversion') . "` WHERE idvisit IN "
+        $sql = "DELETE FROM `" . Common::prefixTable($tableName) . "` WHERE idvisit IN "
              . $this->getInFieldExpressionWithInts($visitIds);
 
         $statement = Db::query($sql);
diff --git a/core/LogDeleter.php b/core/LogDeleter.php
index fc61ec9358..51d2dddef5 100644
--- a/core/LogDeleter.php
+++ b/core/LogDeleter.php
@@ -9,6 +9,7 @@
 namespace Piwik;
 
 use Piwik\DataAccess\RawLogDao;
+use Piwik\Plugin\LogTablesProvider;
 
 /**
  * Service that deletes log entries. Methods in this class cascade, so deleting visits will delete visit actions,
@@ -21,9 +22,15 @@ class LogDeleter
      */
     private $rawLogDao;
 
-    public function __construct(RawLogDao $rawLogDao)
+    /**
+     * @var LogTablesProvider
+     */
+    private $logTablesProvider;
+
+    public function __construct(RawLogDao $rawLogDao, LogTablesProvider $logTablesProvider)
     {
         $this->rawLogDao = $rawLogDao;
+        $this->logTablesProvider = $logTablesProvider;
     }
 
     /**
@@ -35,33 +42,18 @@ class LogDeleter
      */
     public function deleteVisits($visitIds)
     {
-        $this->deleteConversions($visitIds);
-        $this->rawLogDao->deleteVisitActionsForVisits($visitIds);
-
-        return $this->rawLogDao->deleteVisits($visitIds);
-    }
+        $numDeletedVisits = 0;
 
-    /**
-     * Deletes conversions by visit ID. This method cascades, so conversion items are also deleted.
-     *
-     * @param int[] $visitIds The list of visits to delete conversions for.
-     * @return int The number rows deleted.
-     */
-    public function deleteConversions($visitIds)
-    {
-        $this->deleteConversionItems($visitIds);
-        return $this->rawLogDao->deleteConversions($visitIds);
-    }
+        foreach ($this->logTablesProvider->getAllLogTables() as $logTable) {
+            if ($logTable->getColumnToJoinOnIdVisit()) {
+                $numVisits = $this->rawLogDao->deleteFromLogTable($logTable->getName(), $visitIds);
+                if ($logTable->getName() === 'log_visit') {
+                    $numDeletedVisits = $numVisits;
+                }
+            }
+        }
 
-    /**
-     * Deletes conversion items by visit ID.
-     *
-     * @param int[] $visitIds The list of visits to delete conversions for.
-     * @return int The number rows deleted.
-     */
-    public function deleteConversionItems($visitIds)
-    {
-        return $this->rawLogDao->deleteConversionItems($visitIds);
+        return $numDeletedVisits;
     }
 
     /**
diff --git a/plugins/PrivacyManager/LogDataPurger.php b/plugins/PrivacyManager/LogDataPurger.php
index 9f21d001d1..3026e7416b 100755
--- a/plugins/PrivacyManager/LogDataPurger.php
+++ b/plugins/PrivacyManager/LogDataPurger.php
@@ -9,6 +9,7 @@
 namespace Piwik\Plugins\PrivacyManager;
 
 use Piwik\Common;
+use Piwik\Container\StaticContainer;
 use Piwik\DataAccess\RawLogDao;
 use Piwik\Date;
 use Piwik\Db;
@@ -100,7 +101,7 @@ class LogDataPurger
         // deal w/ log tables that will be purged
         $maxIdVisit = $this->getDeleteIdVisitOffset($deleteLogsOlderThan);
         if (!empty($maxIdVisit)) {
-            foreach ($this->getDeleteTableLogTables() as $table) {
+            foreach (self::getDeleteTableLogTables() as $table) {
                 // getting an estimate for log_action is not supported since it can take too long
                 if ($table != Common::prefixTable('log_action')) {
                     $rowCount = $this->getLogTableDeleteCount($table, $maxIdVisit);
@@ -150,13 +151,20 @@ class LogDataPurger
     // let's hardcode, since these are not dynamically created tables
     public static function getDeleteTableLogTables()
     {
-        $result = Common::prefixTables('log_conversion',
-            'log_link_visit_action',
-            'log_visit',
-            'log_conversion_item');
+        $provider = StaticContainer::get('Piwik\Plugin\LogTablesProvider');
+
+        $result = array();
+        foreach ($provider->getAllLogTables() as $logTable) {
+
+            if ($logTable->getColumnToJoinOnIdVisit()) {
+                $result[] = Common::prefixTable($logTable->getName());
+            }
+        }
+
         if (Db::isLockPrivilegeGranted()) {
             $result[] = Common::prefixTable('log_action');
         }
+
         return $result;
     }
 }
diff --git a/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php b/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
index 1f79d34b82..f8ff592eb9 100644
--- a/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
+++ b/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
@@ -23,6 +23,7 @@ use Piwik\Plugins\PrivacyManager\LogDataPurger;
 use Piwik\Plugins\PrivacyManager\PrivacyManager;
 use Piwik\Plugins\PrivacyManager\ReportsPurger;
 use Piwik\Plugins\VisitorInterest\API as APIVisitorInterest;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
 use Piwik\Tracker\GoalManager;
 use Piwik\Tests\Framework\Fixture;
@@ -517,7 +518,7 @@ class DataPurgingTest extends IntegrationTestCase
     {
         $rawLogDao = new DataPurgingTest_RawLogDao(new DimensionMetadataProvider());
         $rawLogDao->insertActionsOlderThanCallback = array($this, 'addReferenceToUnusedAction');
-        $purger = new LogDataPurger(new LogDeleter($rawLogDao), $rawLogDao);
+        $purger = new LogDataPurger(new LogDeleter($rawLogDao, new LogTablesProvider()), $rawLogDao);
 
         $this->unusedIdAction = Db::fetchOne(
             "SELECT idaction FROM " . Common::prefixTable('log_action') . " WHERE name = ?",
diff --git a/tests/PHPUnit/Integration/LogDeleterTest.php b/tests/PHPUnit/Integration/LogDeleterTest.php
index a3f0a2f841..62138ec5c4 100644
--- a/tests/PHPUnit/Integration/LogDeleterTest.php
+++ b/tests/PHPUnit/Integration/LogDeleterTest.php
@@ -12,6 +12,7 @@ use Piwik\Common;
 use Piwik\DataAccess\RawLogDao;
 use Piwik\Db;
 use Piwik\LogDeleter;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
 use Piwik\Tests\Framework\TestDataHelper\LogHelper;
 
@@ -34,7 +35,7 @@ class LogDeleterTest extends IntegrationTestCase
     {
         parent::setUp();
 
-        $this->logDeleter = new LogDeleter(new RawLogDao());
+        $this->logDeleter = new LogDeleter(new RawLogDao(), new LogTablesProvider());
 
         $this->logInserter = new LogHelper();
         $this->insertTestData();
@@ -50,35 +51,6 @@ class LogDeleterTest extends IntegrationTestCase
         $this->assertVisitExists(4);
     }
 
-    public function test_deleteConversions_RemovesConversionsAndConversionItems()
-    {
-        $this->logDeleter->deleteConversions(array(2, 3));
-
-        $this->assertConversionsNotExists(2);
-        $this->assertConversionsNotExists(3);
-
-        $this->assertVisitExists(1);
-        $this->assertVisitExists(2, $checkAggregates = false);
-        $this->assertVisitExists(3, $checkAggregates = false);
-        $this->assertVisitExists(4);
-    }
-
-    public function test_deleteConversionItems_RemovesConversionItems()
-    {
-        $this->logDeleter->deleteConversionItems(array(2, 3));
-
-        $this->assertConversionItemsNotExist(2);
-        $this->assertConversionItemsNotExist(3);
-
-        $this->assertConversionsExists(2, $checkAggregates = false);
-        $this->assertConversionsExists(3, $checkAggregates = false);
-
-        $this->assertVisitExists(1);
-        $this->assertVisitExists(2, $checkAggregates = false);
-        $this->assertVisitExists(3, $checkAggregates = false);
-        $this->assertVisitExists(4);
-    }
-
     public function test_deleteVisitsFor_DeletesVisitsForSpecifiedRangeAndSites_AndInvokesCallbackAfterEveryChunkIsDeleted()
     {
         $iterationCount = 0;