Add possibility to configure different interval for deleting unused actions (#13822)

* add possibility to configure different interval for deleting unused actions

* fix tests

6 files changed, 24 insertions, 10 deletions

diff --git a/plugins/PrivacyManager/Controller.php b/plugins/PrivacyManager/Controller.php
index 606270c1a8..165297fab0 100644
--- a/plugins/PrivacyManager/Controller.php
+++ b/plugins/PrivacyManager/Controller.php
@@ -233,7 +233,7 @@ class Controller extends \Piwik\Plugin\ControllerAdmin
         if ($settings['delete_logs_enable']) {
             /** @var LogDataPurger $logDataPurger */
             $logDataPurger = StaticContainer::get('Piwik\Plugins\PrivacyManager\LogDataPurger');
-            $logDataPurger->purgeData($settings['delete_logs_older_than']);
+            $logDataPurger->purgeData($settings['delete_logs_older_than'], true);
         }
         if ($settings['delete_reports_enable']) {
             $reportsPurger = ReportsPurger::make($settings, PrivacyManager::getAllMetricsToKeep());
diff --git a/plugins/PrivacyManager/LogDataPurger.php b/plugins/PrivacyManager/LogDataPurger.php
index a0d2d82e94..5d524a7fa7 100644
--- a/plugins/PrivacyManager/LogDataPurger.php
+++ b/plugins/PrivacyManager/LogDataPurger.php
@@ -61,8 +61,9 @@ class LogDataPurger
      * @param int $deleteLogsOlderThan The number of days after which log entires are considered old.
      *                                 Visits and related data whose age is greater than this number
      *                                 will be purged.
+     * @param bool $deleteUnusedLogActions Whether to delete unused log actions or not
      */
-    public function purgeData($deleteLogsOlderThan)
+    public function purgeData($deleteLogsOlderThan, $deleteUnusedLogActions)
     {
         $dateUpperLimit = Date::factory("today")->subDay($deleteLogsOlderThan);
         $this->logDeleter->deleteVisitsFor($start = null, $dateUpperLimit->getDatetime());
@@ -70,7 +71,7 @@ class LogDataPurger
         $logTables = self::getDeleteTableLogTables();
 
         // delete unused actions from the log_action table (but only if we can lock tables)
-        if (Db::isLockPrivilegeGranted()) {
+        if ($deleteUnusedLogActions && Db::isLockPrivilegeGranted()) {
             $this->rawLogDao->deleteUnusedLogActions();
         } else {
             $logMessage = get_class($this) . ": LOCK TABLES privilege not granted; skipping unused actions purge";
diff --git a/plugins/PrivacyManager/PrivacyManager.php b/plugins/PrivacyManager/PrivacyManager.php
index 73d12fa84e..8d4704203a 100644
--- a/plugins/PrivacyManager/PrivacyManager.php
+++ b/plugins/PrivacyManager/PrivacyManager.php
@@ -39,6 +39,7 @@ require_once PIWIK_INCLUDE_PATH . '/plugins/PrivacyManager/IPAnonymizer.php';
 class PrivacyManager extends Plugin
 {
     const OPTION_LAST_DELETE_PIWIK_LOGS = "lastDelete_piwik_logs";
+    const OPTION_LAST_DELETE_UNUSED_LOG_ACTIONS = "lastDelete_piwik_unused_log_actions";
     const OPTION_LAST_DELETE_PIWIK_REPORTS = 'lastDelete_piwik_reports';
     const OPTION_LAST_DELETE_PIWIK_LOGS_INITIAL = "lastDelete_piwik_logs_initial";
     const OPTION_USERID_SALT = 'useridsalt';
@@ -50,6 +51,7 @@ class PrivacyManager extends Plugin
         'delete_logs_schedule_lowest_interval' => 'Deletelogs',
         'delete_logs_older_than'               => 'Deletelogs',
         'delete_logs_max_rows_per_query'       => 'Deletelogs',
+        'delete_logs_unused_actions_schedule_lowest_interval' => 'Deletelogs',
         'enable_auto_database_size_estimate'   => 'Deletelogs',
         'enable_database_size_estimate'        => 'Deletelogs',
         'delete_reports_enable'                => 'Deletereports',
@@ -347,7 +349,7 @@ class PrivacyManager extends Plugin
         }
 
         // make sure purging should run at this time (unless this is a forced purge)
-        if (!$this->shouldPurgeData($settings, self::OPTION_LAST_DELETE_PIWIK_REPORTS)) {
+        if (!$this->shouldPurgeData($settings, self::OPTION_LAST_DELETE_PIWIK_REPORTS, 'delete_logs_schedule_lowest_interval')) {
             return false;
         }
 
@@ -381,7 +383,7 @@ class PrivacyManager extends Plugin
         }
 
         // make sure purging should run at this time
-        if (!$this->shouldPurgeData($settings, self::OPTION_LAST_DELETE_PIWIK_LOGS)) {
+        if (!$this->shouldPurgeData($settings, self::OPTION_LAST_DELETE_PIWIK_LOGS, 'delete_logs_schedule_lowest_interval')) {
             return false;
         }
 
@@ -393,10 +395,15 @@ class PrivacyManager extends Plugin
         $lastDeleteDate = Date::factory("today")->getTimestamp();
         Option::set(self::OPTION_LAST_DELETE_PIWIK_LOGS, $lastDeleteDate);
 
+        $shouldDeleteUnusedLogActions = $this->shouldPurgeData($settings, self::OPTION_LAST_DELETE_UNUSED_LOG_ACTIONS, 'delete_logs_unused_actions_schedule_lowest_interval');
+        if ($shouldDeleteUnusedLogActions) {
+            Option::set(self::OPTION_LAST_DELETE_UNUSED_LOG_ACTIONS, $lastDeleteDate);
+        }
+
         // execute the purge
         /** @var LogDataPurger $logDataPurger */
         $logDataPurger = StaticContainer::get('Piwik\Plugins\PrivacyManager\LogDataPurger');
-        $logDataPurger->purgeData($settings['delete_logs_older_than']);
+        $logDataPurger->purgeData($settings['delete_logs_older_than'], $shouldDeleteUnusedLogActions);
 
         return true;
     }
@@ -555,7 +562,7 @@ class PrivacyManager extends Plugin
     /**
      * Returns true if one of the purge data tasks should run now, false if it shouldn't.
      */
-    private function shouldPurgeData($settings, $lastRanOption)
+    private function shouldPurgeData($settings, $lastRanOption, $setting)
     {
         // Log deletion may not run until it is once rescheduled (initial run). This is the
         // only way to guarantee the calculated next scheduled deletion time.
@@ -567,11 +574,12 @@ class PrivacyManager extends Plugin
 
         // Make sure, log purging is allowed to run now
         $lastDelete = Option::get($lastRanOption);
-        $deleteIntervalDays = $settings['delete_logs_schedule_lowest_interval'];
+        $deleteIntervalDays = $settings[$setting];
         $deleteIntervalSeconds = $this->getDeleteIntervalInSeconds($deleteIntervalDays);
 
         if ($lastDelete === false ||
-            ($lastDelete !== false && ((int)$lastDelete + $deleteIntervalSeconds) <= time())
+            $lastDelete === '' ||
+            ((int)$lastDelete + $deleteIntervalSeconds) <= time()
         ) {
             return true;
         } else // not time to run data purge
diff --git a/plugins/PrivacyManager/Tasks.php b/plugins/PrivacyManager/Tasks.php
index 0ebdb93531..3257aac65a 100644
--- a/plugins/PrivacyManager/Tasks.php
+++ b/plugins/PrivacyManager/Tasks.php
@@ -64,6 +64,10 @@ class Tasks extends \Piwik\Plugin\Tasks
         $privacyManager->deleteReportData();
     }
 
+    /**
+     * To test execute the following command:
+     * `./console core:run-scheduled-tasks "Piwik\Plugins\PrivacyManager\Tasks.deleteLogData"`
+     */
     public function deleteLogData()
     {
         $privacyManager = new PrivacyManager();
diff --git a/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php b/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
index d2964f35e6..7ec0c07b64 100644
--- a/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
+++ b/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
@@ -526,7 +526,7 @@ class DataPurgingTest extends IntegrationTestCase
         $this->assertTrue($this->unusedIdAction > 0);
 
         // purge data
-        $purger->purgeData($this->settings['delete_logs_older_than'], $this->settings['delete_logs_max_rows_per_query']);
+        $purger->purgeData($this->settings['delete_logs_older_than'], true);
 
         // check that actions were purged
         $contentsNotPurged = 3;
diff --git a/plugins/PrivacyManager/tests/Integration/PrivacyManagerTest.php b/plugins/PrivacyManager/tests/Integration/PrivacyManagerTest.php
index cccb2c96e1..edd2906cff 100644
--- a/plugins/PrivacyManager/tests/Integration/PrivacyManagerTest.php
+++ b/plugins/PrivacyManager/tests/Integration/PrivacyManagerTest.php
@@ -149,6 +149,7 @@ class PrivacyManagerTest extends IntegrationTestCase
             'delete_logs_schedule_lowest_interval' => 7,
             'delete_logs_older_than' => 180,
             'delete_logs_max_rows_per_query' => 100000,
+            'delete_logs_unused_actions_schedule_lowest_interval' => 30,
             'enable_auto_database_size_estimate' => 1,
             'enable_database_size_estimate' => 1,
             'delete_reports_enable' => 0,