diff --git a/core/ArchiveProcessor/Loader.php b/core/ArchiveProcessor/Loader.php index 3045325ca36..776b583ab60 100644 --- a/core/ArchiveProcessor/Loader.php +++ b/core/ArchiveProcessor/Loader.php @@ -36,6 +36,13 @@ class Loader { private static $archivingDepth = 0; + /** + * Tracks whether the current prepareArchive run reused an existing archive instead of processing. + * + * @var boolean + */ + private $didReuseArchive = false; + /** * @var Parameters */ @@ -103,6 +110,11 @@ public function prepareArchive($pluginName) return Context::changeIdSite($this->params->getSite()->getId(), function () use ($pluginName) { try { ++self::$archivingDepth; + + if (self::$archivingDepth === 1) { + $this->didReuseArchive = false; + } + return $this->prepareArchiveImpl($pluginName); } finally { --self::$archivingDepth; @@ -135,6 +147,7 @@ private function prepareArchiveImpl($pluginName) // load existing data from archive $data = $this->loadArchiveData(); if (sizeof($data) == 2) { + $this->didReuseArchive = true; return $data; } [$idArchives, $visits, $visitsConverted, $foundRecords] = $data; @@ -153,6 +166,7 @@ private function prepareArchiveImpl($pluginName) $data = $this->loadArchiveData(); if (sizeof($data) == 2) { + $this->didReuseArchive = true; return $data; } @@ -542,6 +556,11 @@ public function canSkipThisArchiveWithReason(): array ]; } + public function didReuseArchive(): bool + { + return $this->didReuseArchive; + } + private function hasChildArchivesInPeriod($idSite, Period $period): bool { $cacheKey = CacheId::siteAware('Archiving.hasChildArchivesInPeriod.' . $period->getRangeString(), [$idSite]); diff --git a/plugins/ArchivingMetrics/ArchivingMetrics.php b/plugins/ArchivingMetrics/ArchivingMetrics.php index 43f96d9880a..17530092aae 100644 --- a/plugins/ArchivingMetrics/ArchivingMetrics.php +++ b/plugins/ArchivingMetrics/ArchivingMetrics.php @@ -11,7 +11,55 @@ namespace Piwik\Plugins\ArchivingMetrics; +use Piwik\Period; +use Piwik\Segment; + class ArchivingMetrics extends \Piwik\Plugin { - // Tracking is performed manually via Tracker from CoreAdminHome API; no events registered here. + public function registerEvents() + { + return [ + 'CoreAdminHome.archiveReports.start' => 'onArchiveReportsStart', + 'CoreAdminHome.archiveReports.complete' => 'onArchiveReportsComplete', + ]; + } + + public function onArchiveReportsStart( + int $idSite, + Period $period, + Segment $segment, + string $plugin, + bool $isArchivePhpTriggered, + ?string $report = null + ): void + { + $timer = Timer::getInstance($isArchivePhpTriggered); + $context = $this->buildContext($idSite, $period, $segment, $plugin, $report); + + $timer->start($context); + } + + /** + * @param int[] $idArchives + */ + public function onArchiveReportsComplete( + int $idSite, + Period $period, + Segment $segment, + string $plugin, + bool $isArchivePhpTriggered, + array $idArchives, + bool $wasCached, + ?string $report = null + ): void { + $timer = Timer::getInstance($isArchivePhpTriggered); + $context = $this->buildContext($idSite, $period, $segment, $plugin, $report); + + $timer->complete($context, $idArchives, $wasCached); + } + + private function buildContext(int $idSite, Period $period, Segment $segment, string $plugin, ?string $report = null): Context + { + return new Context($idSite, $period, $segment, $plugin, $report); + } } diff --git a/plugins/ArchivingMetrics/Timer.php b/plugins/ArchivingMetrics/Timer.php index 82ce6158a0e..830cb4a9d12 100644 --- a/plugins/ArchivingMetrics/Timer.php +++ b/plugins/ArchivingMetrics/Timer.php @@ -40,7 +40,7 @@ final class Timer private $runs = []; /** - * @var Timer + * @var ?Timer */ private static $instance; @@ -69,6 +69,14 @@ public static function getInstance(bool $isArchivePhpTriggered, ?ClockInterface return self::$instance; } + /** + * @internal For tests only. + */ + public static function resetInstanceForTests(): void + { + self::$instance = null; + } + public function start(Context $context): void { if (false === $this->isApplicableForTiming($context)) { diff --git a/plugins/CoreAdminHome/API.php b/plugins/CoreAdminHome/API.php index ec377bb1fa6..8b94f45be0c 100644 --- a/plugins/CoreAdminHome/API.php +++ b/plugins/CoreAdminHome/API.php @@ -299,20 +299,38 @@ public function archiveReports($idSite, $period, $date, $segment = false, $plugi $period = Factory::build($period, $date); $site = new Site($idSite); + $segmentObj = new Segment( + $segment, + [$idSite], + $period->getDateTimeStart()->setTimezone($site->getTimezone()), + $period->getDateTimeEnd()->setTimezone($site->getTimezone()) + ); $parameters = new ArchiveProcessor\Parameters( $site, $period, - new Segment( - $segment, - [$idSite], - $period->getDateTimeStart()->setTimezone($site->getTimezone()), - $period->getDateTimeEnd()->setTimezone($site->getTimezone()) - ) + $segmentObj ); if ($report) { $parameters->setArchiveOnlyReport($report); } + /** + * Triggered before a full archiveReports run starts. + * + * Usage example: + * Piwik::addAction('CoreAdminHome.archiveReports.start', function ($idSite, $period, $segment, $plugin, $isArchivePhpTriggered) { ... }); + * + * @internal + */ + Piwik::postEvent('CoreAdminHome.archiveReports.start', [ + $idSite, + $period, + $segmentObj, + (string) $plugin, + $isArchivePhpTriggered, + $report + ]); + // TODO: need to test case when there are multiple plugin archives w/ only some data each. does purging remove some that we need? $archiveLoader = new ArchiveProcessor\Loader($parameters, $invalidateBeforeArchiving); @@ -323,6 +341,29 @@ public function archiveReports($idSite, $period, $date, $segment = false, $plugi 'nb_visits' => $result[1], ]; } + + $idArchives = isset($result['idarchives']) ? (array) $result['idarchives'] : []; + $wasCached = $archiveLoader->didReuseArchive(); + + /** + * Triggered after a full archiveReports run completes. + * + * Usage example: + * Piwik::addAction('CoreAdminHome.archiveReports.complete', function ($idSite, $period, $segment, $plugin, $isArchivePhpTriggered, $idArchives, $wasCached, $report) { ... }); + * + * @internal + */ + Piwik::postEvent('CoreAdminHome.archiveReports.complete', [ + $idSite, + $period, + $segmentObj, + (string) $plugin, + $isArchivePhpTriggered, + $idArchives, + $wasCached, + $report + ]); + return $result; } diff --git a/plugins/CoreAdminHome/tests/Integration/ArchiveReportsMetricsTimerTest.php b/plugins/CoreAdminHome/tests/Integration/ArchiveReportsMetricsTimerTest.php new file mode 100644 index 00000000000..b01ecb0d126 --- /dev/null +++ b/plugins/CoreAdminHome/tests/Integration/ArchiveReportsMetricsTimerTest.php @@ -0,0 +1,64 @@ +extraPluginsToLoad[] = 'ArchivingMetrics'; + + parent::setUpBeforeClass(); + } + + public function setUp(): void + { + parent::setUp(); + + Timer::resetInstanceForTests(); + } + + public function testArchiveReportsWritesMetricsOnceAndDoesNotWriteAgainWhenReusingDbArchive(): void + { + Fixture::createSuperUser(true); + $_GET['trigger'] = 'archivephp'; + + $idSite = Fixture::createWebsite('2024-01-01 00:00:00'); + + $t = Fixture::getTracker($idSite, '2024-01-01 12:00:00'); + $t->setUrl('http://example.com/'); + Fixture::checkResponse($t->doTrackPageView('test')); + + CoreAdminHomeAPI::getInstance()->archiveReports($idSite, 'day', '2024-01-01'); + $this->assertSame(1, $this->getMetricsCount(), 'Expected archiving_metrics to have 1 row after first archiveReports call.'); + + CoreAdminHomeAPI::getInstance()->archiveReports($idSite, 'day', '2024-01-01'); + $this->assertSame(1, $this->getMetricsCount(), 'Expected archiving_metrics to still have 1 row after reusing the same DB archive.'); + } + + private function getMetricsCount(): int + { + return (int) Db::fetchOne('SELECT COUNT(*) FROM ' . Common::prefixTable('archiving_metrics')); + } +} diff --git a/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php b/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php index 6bf6a6597bb..ae799f6d8b6 100644 --- a/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php +++ b/tests/PHPUnit/Integration/ArchiveProcessor/LoaderTest.php @@ -48,6 +48,41 @@ protected static function beforeTableDataCached() Fixture::createWebsite('2012-02-03 00:00:00'); } + public function testDidReuseArchiveFlagIsOnlySetWhenReusingArchiveFromDb() + { + $oldGet = $_GET; + Fixture::createSuperUser(true); + $_GET['trigger'] = 'archivephp'; + + $idSite = 1; + $dateTime = '2024-01-01 12:00:00'; + $date = '2024-01-01'; + + $t = Fixture::getTracker($idSite, $dateTime); + $t->setUrl('http://example.com/'); + Fixture::checkResponse($t->doTrackPageView('test')); + + $periodObj = Factory::build('day', $date); + + $params = new Parameters(new Site($idSite), $periodObj, new Segment('', [$idSite])); + $loader = new Loader($params); + $result = $loader->prepareArchive('VisitsSummary'); + + $this->assertFalse($loader->didReuseArchive(), 'Expected first archiving run to generate a new archive.'); + $this->assertNotEmpty($result); + $this->assertNotEmpty($result[0]); + + Cache::flushAll(); + + $params = new Parameters(new Site($idSite), $periodObj, new Segment('', [$idSite])); + $loader = new Loader($params); + $result = $loader->prepareArchive('VisitsSummary'); + + $this->assertTrue($loader->didReuseArchive(), 'Expected second archiving run to reuse the existing DB archive.'); + $this->assertSame(1, (int) $result[0][0], 'Expected second archiving run to return the same archive ids.'); + $_GET = $oldGet; + } + public function testPluginOnlyArchivingDoesNotRelaunchChildArchives() { $_GET['pluginOnly'] = 1;