Tweak behaviour of orphaned segment archive purge (#14857)

2 files changed, 8 insertions, 152 deletions

diff --git a/plugins/CoreAdminHome/Tasks.php b/plugins/CoreAdminHome/Tasks.php
index ef9b0bd215..d94979c9c3 100644
--- a/plugins/CoreAdminHome/Tasks.php
+++ b/plugins/CoreAdminHome/Tasks.php
@@ -23,9 +23,9 @@ use Piwik\Piwik;
 use Piwik\Plugins\CoreAdminHome\Emails\JsTrackingCodeMissingEmail;
 use Piwik\Plugins\CoreAdminHome\Emails\TrackingFailuresEmail;
 use Piwik\Plugins\CoreAdminHome\Tasks\ArchivesToPurgeDistributedList;
+use Piwik\Plugins\SegmentEditor\Model;
 use Piwik\Plugins\SitesManager\SitesManager;
 use Piwik\Scheduler\Schedule\SpecificTime;
-use Piwik\Segment;
 use Piwik\Settings\Storage\Backend\MeasurableSettingsTable;
 use Piwik\Tracker\Failures;
 use Piwik\Site;
@@ -275,7 +275,10 @@ class Tasks extends \Piwik\Plugin\Tasks
      */
     public function purgeOrphanedArchives()
     {
-        $segmentHashesByIdSite = $this->getSegmentHashesByIdSite();
+        $eightDaysAgo = Date::factory('now')->subDay(8);
+        $model = new Model();
+        $deletedSegments = $model->getSegmentsDeletedSince($eightDaysAgo);
+
         $archiveTables = ArchiveTableCreator::getTablesArchivesInstalled('numeric');
 
         $datesPurged = array();
@@ -286,35 +289,15 @@ class Tasks extends \Piwik\Plugin\Tasks
             $dateObj = Date::factory("$year-$month-15");
 
             $this->archivePurger->purgeDeletedSiteArchives($dateObj);
-            $this->archivePurger->purgeDeletedSegmentArchives($dateObj, $segmentHashesByIdSite);
+            if (count($deletedSegments)) {
+                $this->archivePurger->purgeDeletedSegmentArchives($dateObj, $deletedSegments);
+            }
 
             $datesPurged[$date] = true;
         }
     }
 
     /**
-     * Get a list of all segment hashes that currently exist, indexed by idSite.
-     * @return array
-     */
-    public function getSegmentHashesByIdSite()
-    {
-        //Get a list of hashes of all segments that exist now
-        $sql = "SELECT DISTINCT definition, enable_only_idsite FROM " . Common::prefixTable('segment')
-            . " WHERE deleted = 0";
-        $rows = Db::fetchAll($sql);
-        $segmentHashes = array();
-        foreach ($rows as $row) {
-            $idSite = (int)$row['enable_only_idsite'];
-            if (! isset($segmentHashes[$idSite])) {
-                $segmentHashes[$idSite] = array();
-            }
-            $segmentHashes[$idSite][] = Segment::getSegmentHash($row['definition']);
-        }
-
-        return $segmentHashes;
-    }
-
-    /**
      * we should only purge outdated & custom range archives if we know cron archiving has just run,
      * or if browser triggered archiving is enabled. if cron archiving has run, then we know the latest
      * archives are in the database, and we can remove temporary ones. if browser triggered archiving is
diff --git a/plugins/CoreAdminHome/tests/Integration/TasksTest.php b/plugins/CoreAdminHome/tests/Integration/TasksTest.php
index 830da7ef97..a23d368de4 100644
--- a/plugins/CoreAdminHome/tests/Integration/TasksTest.php
+++ b/plugins/CoreAdminHome/tests/Integration/TasksTest.php
@@ -219,133 +219,6 @@ class TasksTest extends IntegrationTestCase
         $this->assertEquals(2, $mail->getNumFailures());
     }
 
-    public function test_getSegmentHashesByIdSite_emptyWhenNoSegments()
-    {
-        $segmentsByIdSite = $this->tasks->getSegmentHashesByIdSite();
-        $this->assertEquals(array(), $segmentsByIdSite);
-    }
-
-    public function test_getSegmentHashesByIdSite_allWebsiteAndSiteSpecificSegments()
-    {
-        $model = new Model();
-        $model->createSegment(array(
-            'name' => 'Test Segment 1',
-            'definition' => 'continentCode==eur',
-            'enable_only_idsite' => 0,
-            'deleted' => 0
-        ));
-        $model->createSegment(array(
-            'name' => 'Test Segment 2',
-            'definition' => 'countryCode==nz',
-            'enable_only_idsite' => 0,
-            'deleted' => 0
-        ));
-        $model->createSegment(array(
-            'name' => 'Test Segment 3',
-            'definition' => 'countryCode==au',
-            'enable_only_idsite' => 2,
-            'deleted' => 0
-        ));
-
-        $segmentsByIdSite = $this->tasks->getSegmentHashesByIdSite();
-        $expected = array(
-            0 => array('be90051048558489e1d62f4245a6dc65', 'b92fbb3009b32cf632965802de2fb760'),
-            2 => array('cffd4336c22c6782211f853495076b1a')
-        );
-        $this->assertEquals($expected, $segmentsByIdSite);
-    }
-
-    public function test_getSegmentHashesByIdSite_deletedSegment()
-    {
-        $model = new Model();
-        $model->createSegment(array(
-            'name' => 'Test Segment 1',
-            'definition' => 'continentCode==eur',
-            'enable_only_idsite' => 0,
-            'deleted' => 0
-        ));
-        $model->createSegment(array(
-            'name' => 'Test Segment 2',
-            'definition' => 'countryCode==nz',
-            'enable_only_idsite' => 0,
-            'deleted' => 1
-        ));
-        $model->createSegment(array(
-            'name' => 'Test Segment 3',
-            'definition' => 'countryCode==au',
-            'enable_only_idsite' => 2,
-            'deleted' => 0
-        ));
-
-        $segmentsByIdSite = $this->tasks->getSegmentHashesByIdSite();
-        $expected = array(
-            0 => array('be90051048558489e1d62f4245a6dc65'),
-            2 => array('cffd4336c22c6782211f853495076b1a')
-        );
-        $this->assertEquals($expected, $segmentsByIdSite);
-    }
-
-    public function test_getSegmentHashesByIdSite_invalidSegment()
-    {
-        $model = new Model();
-        $model->createSegment(array(
-            'name' => 'Test Segment 4',
-            'definition' => 'countryCode=nz',   //The single "=" is invalid - we should generate a hash anyway
-            'enable_only_idsite' => 0,
-            'deleted' => 0
-        ));
-        $model->createSegment(array(
-            'name' => 'Test Segment 5',
-            'definition' => 'countryCode==au',
-            'enable_only_idsite' => 0,
-            'deleted' => 0
-        ));
-
-        $expected = array(
-            0 => array('5ffe7e116fae7576c047b1fb811584a5', 'cffd4336c22c6782211f853495076b1a'),
-        );
-
-        $segmentsByIdSite = $this->tasks->getSegmentHashesByIdSite();
-        $this->assertEquals($expected, $segmentsByIdSite);
-    }
-
-    public function test_getSegmentHashesByIdSite_siteSpecificCustomDimension()
-    {
-        // Insert a custom dimension for idsite = 1
-        $configuration = new Configuration();
-        $configuration->configureNewDimension(
-            1, 
-            'mydimension', 
-            CustomDimensions::SCOPE_VISIT, 
-            1, 
-            1, 
-            array(), 
-            true
-        );
-
-        $model = new Model();
-        $model->createSegment(array(
-            'name' => 'Test Segment 6',
-            'definition' => 'mydimension==red',
-            'enable_only_idsite' => 1,
-            'deleted' => 0
-        ));
-        $model->createSegment(array(
-            'name' => 'Test Segment 7',
-            'definition' => 'countryCode==au',
-            'enable_only_idsite' => 2,
-            'deleted' => 0
-        ));
-
-        $expected = array(
-            1 => array('240d2a84a309debd26bdbaa8eb3d363c'),
-            2 => array('cffd4336c22c6782211f853495076b1a')
-        );
-
-        $segmentsByIdSite = $this->tasks->getSegmentHashesByIdSite();
-        $this->assertEquals($expected, $segmentsByIdSite);
-    }
-
     /**
      * @param Date[] $dates
      */