Make sure not to clear the tracker cache so often when invalidating in core:archive (#17321)

* Make sure not to clear the tracker cache so often when invalidating in core:archive

* apply review feedback

* remove comment

* missed another withDelegatedCacheClears call

* fix test

* fix test

7 files changed, 162 insertions, 9 deletions

diff --git a/core/CronArchive.php b/core/CronArchive.php
index 0a27ab0640..8ed11d0445 100644
--- a/core/CronArchive.php
+++ b/core/CronArchive.php
@@ -343,7 +343,9 @@ class CronArchive
         }
 
         $this->logger->debug("Applying queued rearchiving...");
-        $this->invalidator->applyScheduledReArchiving();
+        \Piwik\Tracker\Cache::withDelegatedCacheClears(function () {
+            $this->invalidator->applyScheduledReArchiving();
+        });
 
         $failedJobs = $this->model->resetFailedArchivingJobs();
         if ($failedJobs) {
@@ -767,6 +769,13 @@ class CronArchive
 
     public function invalidateArchivedReportsForSitesThatNeedToBeArchivedAgain($idSiteToInvalidate)
     {
+        \Piwik\Tracker\Cache::withDelegatedCacheClears(function () use ($idSiteToInvalidate) {
+            $this->invalidateArchivedReportsForSitesThatNeedToBeArchivedAgainImpl($idSiteToInvalidate);
+        });
+    }
+
+    private function invalidateArchivedReportsForSitesThatNeedToBeArchivedAgainImpl($idSiteToInvalidate)
+    {
         if (empty($this->segmentArchiving)) {
             // might not be initialised if init is not called
             $this->segmentArchiving = StaticContainer::get(SegmentArchiving::class);
diff --git a/core/Tracker/Cache.php b/core/Tracker/Cache.php
index fef9f827c7..4cce6e4131 100644
--- a/core/Tracker/Cache.php
+++ b/core/Tracker/Cache.php
@@ -17,6 +17,7 @@ use Piwik\Option;
 use Piwik\Piwik;
 use Piwik\Tracker;
 use Psr\Log\LoggerInterface;
+use function DI\object;
 
 /**
  * Simple cache mechanism used in Tracker to avoid requesting settings from mysql on every request
@@ -24,6 +25,18 @@ use Psr\Log\LoggerInterface;
  */
 class Cache
 {
+    /**
+     * {@see self::withDelegatedCacheClears()}
+     * @var bool
+     */
+    private static $delegatingCacheClears;
+
+    /**
+     * {@see self::withDelegatedCacheClears()}
+     * @var array
+     */
+    private static $delegatedClears = [];
+
     private static $cacheIdGeneral = 'general';
 
     /**
@@ -61,7 +74,7 @@ class Cache
             return array();
         }
 
-        $idSite = (int) $idSite;
+        $idSite = (int)$idSite;
         if ($idSite <= 0) {
             return array();
         }
@@ -138,6 +151,11 @@ class Cache
      */
     public static function clearCacheGeneral()
     {
+        if (self::$delegatingCacheClears) {
+            self::$delegatedClears[__FUNCTION__] = [__FUNCTION__, []];
+            return;
+        }
+
         self::getCache()->delete(self::$cacheIdGeneral);
     }
 
@@ -169,7 +187,7 @@ class Cache
         Tracker::initCorePiwikInTrackerMode();
         $cacheContent = array(
             'isBrowserTriggerEnabled' => Rules::isBrowserTriggerEnabled(),
-            'lastTrackerCronRun'      => Option::get('lastTrackerCronRun'),
+            'lastTrackerCronRun' => Option::get('lastTrackerCronRun'),
         );
 
         /**
@@ -240,7 +258,12 @@ class Cache
      */
     public static function deleteCacheWebsiteAttributes($idSite)
     {
-        self::getCache()->delete((int) $idSite);
+        if (self::$delegatingCacheClears) {
+            self::$delegatedClears[__FUNCTION__ . $idSite] = [__FUNCTION__, func_get_args()];
+            return;
+        }
+
+        self::getCache()->delete((int)$idSite);
     }
 
     /**
@@ -248,6 +271,47 @@ class Cache
      */
     public static function deleteTrackerCache()
     {
+        if (self::$delegatingCacheClears) {
+            self::$delegatedClears[__FUNCTION__] = [__FUNCTION__, []];
+            return;
+        }
+
         self::getCache()->flushAll();
     }
+
+    /**
+     * Runs `$callback` without clearing any tracker cache, just collecting which delete methods were called.
+     * After `$callback` finishes, we clear caches, but just once per type of delete/clear method collected.
+     *
+     * Use this method if your code will create many cache clears in a short amount of time (eg, if you
+     * are invalidating a lot of archives at once).
+     *
+     * @param $callback
+     */
+    public static function withDelegatedCacheClears($callback)
+    {
+        try {
+            self::$delegatingCacheClears = true;
+            self::$delegatedClears = [];
+
+            return $callback();
+        } finally {
+            self::$delegatingCacheClears = false;
+
+            self::callAllDelegatedClears();
+
+            self::$delegatedClears = [];
+        }
+    }
+
+    private static function callAllDelegatedClears()
+    {
+        foreach (self::$delegatedClears as list($methodName, $params)) {
+            if (!method_exists(self::class, $methodName)) {
+                continue;
+            }
+
+            call_user_func_array([self::class, $methodName], $params);
+        }
+    }
 }
diff --git a/tests/PHPUnit/Framework/Fixture.php b/tests/PHPUnit/Framework/Fixture.php
index a74d04b8e6..9358b9d8c6 100644
--- a/tests/PHPUnit/Framework/Fixture.php
+++ b/tests/PHPUnit/Framework/Fixture.php
@@ -381,17 +381,18 @@ class Fixture extends \PHPUnit\Framework\Assert
             $this->dropDatabase();
         }
 
-        $this->clearInMemoryCaches();
+        self::clearInMemoryCaches();
 
         Log::unsetInstance();
 
         $this->destroyEnvironment();
     }
 
-    public function clearInMemoryCaches()
+    public static function clearInMemoryCaches($resetTranslations = true)
     {
         Date::$now = null;
         FrontController::$requestId = null;
+        Cache::$cache = null;
         Archive::clearStaticCache();
         DataTableManager::getInstance()->deleteAll();
         Option::clearCache();
@@ -408,7 +409,9 @@ class Fixture extends \PHPUnit\Framework\Assert
 
         Plugin\API::unsetAllInstances();
         $_GET = $_REQUEST = array();
-        self::resetTranslations();
+        if ($resetTranslations) {
+            self::resetTranslations();
+        }
 
         self::getConfig()->Plugins; // make sure Plugins exists in a config object for next tests that use Plugin\Manager
         // since Plugin\Manager uses getFromGlobalConfig which doesn't init the config object
diff --git a/tests/PHPUnit/Framework/TestCase/IntegrationTestCase.php b/tests/PHPUnit/Framework/TestCase/IntegrationTestCase.php
index 91b0fc8f61..ae26a06e27 100644
--- a/tests/PHPUnit/Framework/TestCase/IntegrationTestCase.php
+++ b/tests/PHPUnit/Framework/TestCase/IntegrationTestCase.php
@@ -100,7 +100,7 @@ abstract class IntegrationTestCase extends SystemTestCase
      */
     public function tearDown(): void
     {
-        static::$fixture->clearInMemoryCaches();
+        Fixture::clearInMemoryCaches();
         static::$fixture->destroyEnvironment();
 
         parent::tearDown();
diff --git a/tests/PHPUnit/Framework/TestCase/UnitTestCase.php b/tests/PHPUnit/Framework/TestCase/UnitTestCase.php
index 56428a92de..db29202b10 100644
--- a/tests/PHPUnit/Framework/TestCase/UnitTestCase.php
+++ b/tests/PHPUnit/Framework/TestCase/UnitTestCase.php
@@ -9,6 +9,8 @@
 namespace Piwik\Tests\Framework\TestCase;
 
 use Piwik\Application\Environment;
+use Piwik\Container\StaticContainer;
+use Piwik\Tests\Framework\Fixture;
 use Piwik\Tests\Framework\Mock\File;
 
 /**
@@ -45,12 +47,14 @@ abstract class UnitTestCase extends \PHPUnit\Framework\TestCase
 
         $this->initEnvironment();
 
+        Fixture::clearInMemoryCaches($resetTranslations = false);
         File::reset();
     }
 
     public function tearDown(): void
     {
         File::reset();
+        Fixture::clearInMemoryCaches($resetTranslations = false);
 
         // make sure the global container exists for the next test case that is executed (since logging can be done
         // before a test sets up an environment)
diff --git a/tests/PHPUnit/Integration/Settings/SettingTest.php b/tests/PHPUnit/Integration/Settings/SettingTest.php
index 2ddc816fcb..3491eea19b 100644
--- a/tests/PHPUnit/Integration/Settings/SettingTest.php
+++ b/tests/PHPUnit/Integration/Settings/SettingTest.php
@@ -33,8 +33,9 @@ class SettingTest extends \PHPUnit\Framework\TestCase
 
     public function tearDown(): void
     {
+        Fixture::clearInMemoryCaches();
+
         $fixutre = new Fixture();
-        $fixutre->clearInMemoryCaches();
         $fixutre->destroyEnvironment();
     }
 
diff --git a/tests/PHPUnit/Unit/Tracker/CacheTest.php b/tests/PHPUnit/Unit/Tracker/CacheTest.php
new file mode 100644
index 0000000000..145cd56004
--- /dev/null
+++ b/tests/PHPUnit/Unit/Tracker/CacheTest.php
@@ -0,0 +1,72 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+
+namespace PHPUnit\Unit\Tracker;
+
+use Matomo\Cache\Lazy;
+use Piwik\Tests\Framework\TestCase\UnitTestCase;
+use Piwik\Tracker\Cache;
+
+class CacheTest extends UnitTestCase
+{
+    private $methodsCalled;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+        $this->methodsCalled = [];
+    }
+
+    public function test_withDelegatedCacheClears_onlyCallsCacheClearsAtTheEndOnce()
+    {
+        $this->assertEmpty($this->methodsCalled);
+        Cache::withDelegatedCacheClears(function () {
+            Cache::clearCacheGeneral();
+            Cache::deleteTrackerCache();
+            Cache::clearCacheGeneral();
+            Cache::deleteCacheWebsiteAttributes(1);
+            Cache::clearCacheGeneral();
+            Cache::deleteCacheWebsiteAttributes(1);
+            Cache::deleteCacheWebsiteAttributes(1);
+            Cache::deleteCacheWebsiteAttributes(2);
+            Cache::deleteTrackerCache();
+            Cache::deleteCacheWebsiteAttributes(2);
+            Cache::deleteTrackerCache();
+
+            $this->assertEmpty($this->methodsCalled);
+        });
+
+        $expectedCalls = [
+            'delete.general',
+            'flushAll',
+            'delete.1',
+            'delete.2',
+        ];
+        $this->assertEquals($expectedCalls, $this->methodsCalled);
+    }
+
+    public function provideContainerConfig()
+    {
+        $mockLazyCache = $this->getMockBuilder(Lazy::class)
+            ->onlyMethods(['flushAll', 'delete'])
+            ->disableOriginalConstructor()
+            ->getMock();
+        $mockLazyCache
+            ->method('flushAll')->willReturnCallback(function () {
+                $this->methodsCalled[] = 'flushAll';
+            });
+        $mockLazyCache->method('delete')->willReturnCallback(function ($key) {
+                $this->methodsCalled[] = 'delete.' . $key;
+            });
+
+        return [
+            Lazy::class => $mockLazyCache,
+        ];
+    }
+}
\ No newline at end of file