Make archiving process to respect process_new_segments_from settings (#17351)

* update canSkipThisArchive in Loader

* Update Loader.php

* Update Loader.php

* make cronarchive respect segment creation

* check for null

* do not use force-date-last-n in test

* fix some other tests

* fix other test

* add deprecated message

* Update UITestFixture.php

* wip

* reafactor and add tests

* take into account requested report

* refactor if

* refactoring

* fix bug

Co-authored-by: Thomas Steur <tsteur@users.noreply.github.com>

9 files changed, 302 insertions, 7 deletions

diff --git a/core/ArchiveProcessor/Loader.php b/core/ArchiveProcessor/Loader.php
index 026dda4a6b..703d55eda4 100644
--- a/core/ArchiveProcessor/Loader.php
+++ b/core/ArchiveProcessor/Loader.php
@@ -27,6 +27,7 @@ use Piwik\Piwik;
 use Piwik\SettingsServer;
 use Piwik\Site;
 use Psr\Log\LoggerInterface;
+use Piwik\CronArchive\SegmentArchiving;
 
 /**
  * This class uses PluginsArchiver class to trigger data aggregation and create archives.
@@ -393,12 +394,52 @@ class Loader
         $hasSiteVisitsBetweenTimeframe = $this->hasSiteVisitsBetweenTimeframe($idSite, $params->getPeriod());
         $hasChildArchivesInPeriod = $this->dataAccessModel->hasChildArchivesInPeriod($idSite, $params->getPeriod());
 
+        if ($this->canSkipArchiveForSegment()) {
+            return true;
+        }
+
         return $isWebsiteUsingTracker
             && !$isArchivingForcedWhenNoVisits
             && !$hasSiteVisitsBetweenTimeframe
             && !$hasChildArchivesInPeriod;
     }
 
+    public function canSkipArchiveForSegment()
+    {
+        $params = $this->params;
+
+        if ($params->getSegment()->isEmpty()) {
+            return false;
+        }
+
+        /** @var SegmentArchiving */
+        $segmentArchiving = StaticContainer::get(SegmentArchiving::class);
+        $segmentInfo = $segmentArchiving->findSegmentForHash($params->getSegment()->getHash(), $params->getSite()->getId());
+
+        if (!$segmentInfo) {
+            return false;
+        }
+
+        $segmentArchiveStartDate = $segmentArchiving->getReArchiveSegmentStartDate($segmentInfo);
+
+        if ($segmentArchiveStartDate !==null && $segmentArchiveStartDate->isLater($params->getPeriod()->getDateEnd()->getEndOfDay())) {
+            $doneFlag = Rules::getDoneStringFlagFor(
+                [$params->getSite()->getId()],
+                $params->getSegment(),
+                $params->getPeriod()->getLabel(),
+                $params->getRequestedPlugin()
+            );
+
+            // if there is no invalidation where the report is null, we can skip
+            // if we have invalidations for the period and name, but only for a specific reports, we can skip
+            // if the report is not null we only want to rearchive if we have invalidation for that report
+            // if we don't find invalidation for that report, we can skip
+            return !$this->dataAccessModel->hasInvalidationForPeriodAndName($params->getSite()->getId(), $params->getPeriod(), $doneFlag, $params->getArchiveOnlyReport());
+        }
+
+        return false;
+    }
+
     private function isWebsiteUsingTheTracker($idSite)
     {
         $idSitesNotUsingTracker = self::getSitesNotUsingTracker();
diff --git a/core/CronArchive.php b/core/CronArchive.php
index 85419be50f..ec52fe9a31 100644
--- a/core/CronArchive.php
+++ b/core/CronArchive.php
@@ -894,6 +894,23 @@ class CronArchive
                 if ($this->canWeSkipInvalidatingBecauseThereIsAUsablePeriod($params, $doNotIncludeTtlInExistingArchiveCheck)) {
                     $this->logger->debug('  Found usable archive for {archive}, skipping invalidation.', ['archive' => $params]);
                 } else {
+                    if (empty($this->segmentArchiving)) {
+                        // might not be initialised if init is not called
+                        $this->segmentArchiving = StaticContainer::get(SegmentArchiving::class);
+                    }
+
+                    $segmentInfo = $this->segmentArchiving->findSegmentForHash($params->getSegment()->getHash(), $idSite);
+
+                    if ($segmentInfo) {
+                        $segmentArchiveStartDate = $this->segmentArchiving->getReArchiveSegmentStartDate($segmentInfo);
+
+                        if ($segmentArchiveStartDate !== null && $segmentArchiveStartDate->isLater($params->getPeriod()->getDateEnd()->getEndOfDay())) {
+                            // the system is not allowed to invalidate reports for this period
+                            // automatically, only a user can specifically invalidate
+                            continue;
+                        }
+                    }
+
                     $this->getApiToInvalidateArchivedReport()->invalidateArchivedReports($idSite, $date, $period, $segmentDefinition,
                         $cascadeDown = false, $_forceInvalidateNonexistant);
                 }
diff --git a/core/DataAccess/Model.php b/core/DataAccess/Model.php
index ca4cdac9c2..1be2fea744 100644
--- a/core/DataAccess/Model.php
+++ b/core/DataAccess/Model.php
@@ -864,6 +864,48 @@ class Model
         return false;
     }
 
+    /**
+     * Returns true if any invalidations exists for the given
+     * $idsite and $doneFlag (name column) for the $period.
+     *
+     * @param mixed $idSite
+     * @param Period $period
+     * @param mixed $doneFlag
+     * @param mixed $report
+     * @return bool
+     * @throws Exception
+     */
+    public function hasInvalidationForPeriodAndName($idSite, Period $period, $doneFlag, $report = null)
+    {
+        $table = Common::prefixTable('archive_invalidations');
+
+        if (empty($report)) {
+            $sql = "SELECT idinvalidation FROM `$table` WHERE idsite = ? AND date1 = ? AND date2 = ? AND `period` = ? AND `name` = ?  AND `report` IS NULL LIMIT 1";
+        } else {
+            $sql = "SELECT idinvalidation FROM `$table` WHERE idsite = ? AND date1 = ? AND date2 = ? AND `period` = ? AND `name` = ? AND `report` = ? LIMIT 1";
+        }
+
+        $bind = [
+            $idSite,
+            $period->getDateStart()->toString(),
+            $period->getDateEnd()->toString(),
+            $period->getId(),
+            $doneFlag
+        ];
+
+        if (!empty($report)) {
+            $bind[] = $report;
+        }
+
+        $idInvalidation = Db::fetchOne($sql, $bind);
+
+        if (empty($idInvalidation)) {
+            return false;
+        }
+
+        return true;
+    }
+
     public function deleteInvalidationsForSites(array $idSites)
     {
         $idSites = array_map('intval', $idSites);
diff --git a/plugins/CoreConsole/Commands/CoreArchiver.php b/plugins/CoreConsole/Commands/CoreArchiver.php
index 1b78f5d5fb..ad16b543e4 100644
--- a/plugins/CoreConsole/Commands/CoreArchiver.php
+++ b/plugins/CoreConsole/Commands/CoreArchiver.php
@@ -23,6 +23,11 @@ class CoreArchiver extends ConsoleCommand
 
     protected function execute(InputInterface $input, OutputInterface $output)
     {
+        if($input->getOption('force-date-last-n')) {
+            $message = '"force-date-last-n" is deprecated. Please use the "process_new_segments_from" INI configuration option instead.';
+            $output->writeln('<comment>' . $message .'</comment>');
+        }
+
         $archiver = self::makeArchiver($input->getOption('url'), $input);
         $archiver->main();
     }
@@ -38,7 +43,6 @@ class CoreArchiver extends ConsoleCommand
         $archiver->shouldArchiveSpecifiedSites = self::getSitesListOption($input, "force-idsites");
         $archiver->shouldSkipSpecifiedSites = self::getSitesListOption($input, "skip-idsites");
         $archiver->phpCliConfigurationOptions = $input->getOption("php-cli-options");
-        $archiver->dateLastForced = $input->getOption('force-date-last-n');
         $archiver->concurrentRequestsPerWebsite = $input->getOption('concurrent-requests-per-website');
         $archiver->maxConcurrentArchivers = $input->getOption('concurrent-archivers');
         $archiver->shouldArchiveAllSites = $input->getOption('force-all-websites');
@@ -91,8 +95,8 @@ class CoreArchiver extends ConsoleCommand
             'If specified, segments will be only archived for yesterday, but not today. If the segment was created or changed recently, then it will still be archived for today and the setting will be ignored for this segment.');
         $command->addOption('force-periods', null, InputOption::VALUE_OPTIONAL,
             "If specified, archiving will be processed only for these Periods (comma separated eg. day,week,month,year,range)");
-        $command->addOption('force-date-last-n', null, InputOption::VALUE_REQUIRED,
-            "This last N number of years of data to invalidate when a recently created or updated segment is found.", 7);
+        $command->addOption('force-date-last-n', null, InputOption::VALUE_OPTIONAL,
+            "Deprecated. Please use the \"process_new_segments_from\" INI configuration option instead.");
         $command->addOption('force-date-range', null, InputOption::VALUE_OPTIONAL,
             "If specified, archiving will be processed only for periods included in this date range. Format: YYYY-MM-DD,YYYY-MM-DD");
         $command->addOption('force-idsegments', null, InputOption::VALUE_REQUIRED,
diff --git a/plugins/CoreConsole/tests/System/ArchiveCronTest.php b/plugins/CoreConsole/tests/System/ArchiveCronTest.php
index bbc48d31c9..70839f54b3 100644
--- a/plugins/CoreConsole/tests/System/ArchiveCronTest.php
+++ b/plugins/CoreConsole/tests/System/ArchiveCronTest.php
@@ -432,7 +432,7 @@ class ArchiveCronTest extends SystemTestCase
         $urlToProxy = Fixture::getRootUrl() . 'tests/PHPUnit/proxy/index.php';
 
         // create the command
-        $cmd = "php \"$archivePhpScript\" --url=\"$urlToProxy\" --force-date-last-n=10";
+        $cmd = "php \"$archivePhpScript\" --url=\"$urlToProxy\"";
         foreach ($options as $name => $value) {
             $cmd .= " $name";
             if ($value !== null) {
@@ -464,6 +464,9 @@ class ArchiveCronTest extends SystemTestCase
             },
 
             'Tests.log.allowAllHandlers' => true,
+
+            CronArchive\SegmentArchiving::class => \DI\object()
+                ->constructorParameter('beginningOfTimeLastNInYears', 10)
         );
     }
 
diff --git a/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php b/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php
index 28881d034e..35cd5ff10f 100644
--- a/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php
+++ b/plugins/SegmentEditor/tests/System/UnprocessedSegmentsTest.php
@@ -17,6 +17,7 @@ use Piwik\Plugins\SegmentEditor\API;
 use Piwik\Plugins\VisitsSummary;
 use Piwik\Tests\Fixtures\OneVisitorTwoVisits;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+use Piwik\CronArchive\SegmentArchiving;
 
 /**
  * @group SegmentEditor
@@ -257,6 +258,9 @@ class UnprocessedSegmentsTest extends IntegrationTestCase
                 $previous->General['browser_archiving_disabled_enforce'] = 1;
                 return $previous;
             }),
+
+            SegmentArchiving::class => \DI\object()
+                ->constructorParameter('beginningOfTimeLastNInYears', 15)
         ];
     }
 
diff --git a/tests/PHPUnit/Fixtures/UITestFixture.php b/tests/PHPUnit/Fixtures/UITestFixture.php
index aea7359f66..64972ea701 100644
--- a/tests/PHPUnit/Fixtures/UITestFixture.php
+++ b/tests/PHPUnit/Fixtures/UITestFixture.php
@@ -43,6 +43,7 @@ use Piwik\ReportRenderer;
 use Piwik\Tests\Framework\XssTesting;
 use Piwik\Plugins\ScheduledReports\API as APIScheduledReports;
 use Psr\Container\ContainerInterface;
+use Piwik\CronArchive\SegmentArchiving;
 
 /**
  * Fixture for UI tests.
@@ -508,6 +509,9 @@ class UITestFixture extends SqlDump
                 $previous[] = $c->get(WebNotificationHandler::class);
                 return $previous;
             }),
+
+            SegmentArchiving::class => \DI\object()
+                ->constructorParameter('beginningOfTimeLastNInYears', 15)
         ];
     }
 
@@ -648,4 +652,4 @@ class CustomApiProxy extends Proxy
         }
         return parent::isExistingApiAction($pluginName, $apiAction);
     }
-}
\ No newline at end of file
+}
diff --git a/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php b/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php
index fb0986e4d6..7465261d28 100644
--- a/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php
+++ b/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php
@@ -29,6 +29,10 @@ use Piwik\Sequence;
 use Piwik\Site;
 use Piwik\Tests\Framework\Fixture;
 use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+use Piwik\Plugins\SegmentEditor\API as SegmentApi;
+use Piwik\Option;
+use Piwik\ArchiveProcessor\Rules;
+use ReflectionClass;
 
 class LoaderTest extends IntegrationTestCase
 {
@@ -1345,6 +1349,120 @@ class LoaderTest extends IntegrationTestCase
         $this->assertFalse($loader->canSkipThisArchive());
     }
 
+    public function test_canSkipArchiveForSegment_returnsFalseIfNoSegments()
+    {
+        $params = new Parameters(new Site(1), Factory::build('year', '2016-02-03'), new Segment('', []));
+        $loader = new Loader($params);
+
+        $this->assertFalse($loader->canSkipArchiveForSegment());
+    }
+
+    public function test_canSkipArchiveForSegment_returnsFalseIfPeriodEndLaterThanSegmentArchiveStartDate()
+    {
+        Rules::setBrowserTriggerArchiving(false);
+        $definition = 'browserCode==ch';
+
+        SegmentApi::getInstance()->add('segment', $definition, 1, true, true);
+        $params = new Parameters(new Site(1), Factory::build('year', '2021-04-23'), new Segment($definition, [1]));
+        $loader = new Loader($params);
+
+        $this->assertFalse($loader->canSkipArchiveForSegment());
+    }
+
+    public function test_canSkipArchiveForSegment_returnsTrueIfPeriodEndEarlierThanSegmentArchiveStartDate()
+    {
+        Rules::setBrowserTriggerArchiving(false);
+
+        $definition = 'browserCode==ch';
+        SegmentApi::getInstance()->add('segment', $definition, 1, true, true);
+        $params = new Parameters(new Site(1), Factory::build('year', '2010-04-23'), new Segment($definition, [1]));
+        $loader = new Loader($params);
+
+        $this->assertTrue($loader->canSkipArchiveForSegment());
+    }
+
+    public function test_canSkipArchiveForSegment_returnsFalseIfHasInvalidationForThePeriod()
+    {
+        Rules::setBrowserTriggerArchiving(false);
+
+        $date = '2010-04-23';
+        $definition = 'browserCode==ch';
+        $segment = new Segment($definition, [1]);
+        $doneFlag = Rules::getDoneStringFlagFor([1], $segment, 'day', null);
+
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => $doneFlag],
+        ]);
+
+        SegmentApi::getInstance()->add('segment', $definition, 1, true, true);
+        $params = new Parameters(new Site(1), Factory::build('day', $date), $segment);
+        $loader = new Loader($params);
+
+        $this->assertFalse($loader->canSkipArchiveForSegment());
+    }
+
+    public function test_canSkipArchiveForSegment_returnsTrueIfHasInvalidationForReportButWeDonSpecifyReport()
+    {
+        Rules::setBrowserTriggerArchiving(false);
+
+        $date = '2010-04-23';
+        $definition = 'browserCode==ch';
+        $segment = new Segment($definition, [1]);
+        $doneFlag = Rules::getDoneStringFlagFor([1], $segment, 'day', null);
+
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => $doneFlag, 'report' => 'myReport'],
+        ]);
+
+        SegmentApi::getInstance()->add('segment', $definition, 1, true, true);
+        $params = new Parameters(new Site(1), Factory::build('day', $date), $segment);
+        $loader = new Loader($params);
+
+        $this->assertTrue($loader->canSkipArchiveForSegment());
+    }
+
+    public function test_canSkipArchiveForSegment_returnsFalseIfHasInvalidationForReportWeAskedFor()
+    {
+        Rules::setBrowserTriggerArchiving(false);
+
+        $date = '2010-04-23';
+        $definition = 'browserCode==ch';
+        $segment = new Segment($definition, [1]);
+        $doneFlag = Rules::getDoneStringFlagFor([1], $segment, 'day', null);
+
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => $doneFlag, 'report' => 'myReport'],
+        ]);
+
+        SegmentApi::getInstance()->add('segment', $definition, 1, true, true);
+        $params = new Parameters(new Site(1), Factory::build('day', $date), $segment);
+        $params->setArchiveOnlyReport('myReport');
+        $loader = new Loader($params);
+
+        $this->assertFalse($loader->canSkipArchiveForSegment());
+    }
+
+    public function test_canSkipArchiveForSegment_returnsTrueIfHasNoInvalidationForReportWeAskedFor()
+    {
+        Rules::setBrowserTriggerArchiving(false);
+
+        $date = '2010-04-23';
+        $definition = 'browserCode==ch';
+        $segment = new Segment($definition, [1]);
+        $doneFlag = Rules::getDoneStringFlagFor([1], $segment, 'day', null);
+
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => $doneFlag, 'report' => 'myReport'],
+        ]);
+
+        SegmentApi::getInstance()->add('segment', $definition, 1, true, true);
+        $params = new Parameters(new Site(1), Factory::build('day', $date), $segment);
+        $params->setArchiveOnlyReport('otherReport');
+        $loader = new Loader($params);
+
+        $this->assertTrue($loader->canSkipArchiveForSegment());
+    }
+
     public function test_forcePluginArchiving_createsPluginSpecificArchive()
     {
         $_GET['trigger'] = 'archivephp';
@@ -1417,4 +1535,17 @@ class LoaderTest extends IntegrationTestCase
         }
         return $results;
     }
+
+    private function insertInvalidations(array $invalidations)
+    {
+        $table = Common::prefixTable('archive_invalidations');
+        $now = Date::now()->getDatetime();
+        foreach ($invalidations as $invalidation) {
+            $sql = "INSERT INTO `$table` (idsite, date1, date2, period, `name`, status, ts_invalidated, ts_started, report) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)";
+            Db::query($sql, [
+                $invalidation['idsite'] ?? 1, $invalidation['date1'], $invalidation['date2'], $invalidation['period'], $invalidation['name'],
+                $invalidation['status'] ?? 0, $invalidation['ts_invalidated'] ?? $now, $invalidation['ts_started'] ?? null, $invalidation['report'] ?? null
+            ]);
+        }
+    }
 }
diff --git a/tests/PHPUnit/Integration/DataAccess/ModelTest.php b/tests/PHPUnit/Integration/DataAccess/ModelTest.php
index 46cebc6d3a..5d01db8250 100644
--- a/tests/PHPUnit/Integration/DataAccess/ModelTest.php
+++ b/tests/PHPUnit/Integration/DataAccess/ModelTest.php
@@ -136,6 +136,55 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals($expected, $result);
     }
 
+    public function test_hasInvalidationForPeriodAndName_returnsTrueIfExists()
+    {
+        $date = '2021-03-23';
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => 'done'],
+        ]);
+
+        $periodObj = Factory::build('day', $date);
+        $result = $this->model->hasInvalidationForPeriodAndName(1, $periodObj, 'done');
+        $this->assertTrue($result);
+    }
+
+    public function test_hasInvalidationForPeriodAndName_returnsTrueIfExistsForReport()
+    {
+        $date = '2021-03-23';
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => 'done', 'report' => 'myReport'],
+        ]);
+
+        $periodObj = Factory::build('day', $date);
+        $result = $this->model->hasInvalidationForPeriodAndName(1, $periodObj, 'done', 'myReport');
+        $this->assertTrue($result);
+    }
+
+    public function test_hasInvalidationForPeriodAndName_returnsFalseIfNotExistsForReport()
+    {
+        $date = '2021-03-23';
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => 'done', 'report' => 'myReport'],
+        ]);
+
+        $periodObj = Factory::build('day', $date);
+        $result = $this->model->hasInvalidationForPeriodAndName(1, $periodObj, 'done', 'otherReport');
+        $this->assertFalse($result);
+    }
+
+    public function test_hasInvalidationForPeriodAndName_returnsFalseIfNotExists()
+    {
+        $date = '2021-03-23';
+        $date2 = '2021-03-22';
+        $this->insertInvalidations([
+            ['date1' => $date, 'date2' => $date, 'period' => 1, 'name' => 'done'],
+        ]);
+
+        $periodObj = Factory::build('day', $date2);
+        $result = $this->model->hasInvalidationForPeriodAndName(1, $periodObj, 'done');
+        $this->assertFalse($result);
+    }
+
     public function getTestDataForHasChildArchivesInPeriod()
     {
         return [
@@ -593,10 +642,10 @@ class ModelTest extends IntegrationTestCase
         $table = Common::prefixTable('archive_invalidations');
         $now = Date::now()->getDatetime();
         foreach ($invalidations as $invalidation) {
-            $sql = "INSERT INTO `$table` (idsite, date1, date2, period, `name`, status, ts_invalidated, ts_started) VALUES (?, ?, ?, ?, ?, ?, ?, ?)";
+            $sql = "INSERT INTO `$table` (idsite, date1, date2, period, `name`, status, ts_invalidated, ts_started, report) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)";
             Db::query($sql, [
                 $invalidation['idsite'] ?? 1, $invalidation['date1'], $invalidation['date2'], $invalidation['period'], $invalidation['name'],
-                $invalidation['status'] ?? 0, $invalidation['ts_invalidated'] ?? $now, $invalidation['ts_started'] ?? null,
+                $invalidation['status'] ?? 0, $invalidation['ts_invalidated'] ?? $now, $invalidation['ts_started'] ?? null, $invalidation['report'] ?? null,
             ]);
         }
     }