From a3b1e3b4eecf455bbb3438b7d3f31c0175c2c7aa Mon Sep 17 00:00:00 2001 From: sam marshall Date: Tue, 3 Jan 2023 17:59:53 +0000 Subject: [PATCH] MDL-76784 core_cache: versioned cache (modinfo) can fail in install When cache is disabled but temporary in-memory caches are allowed, these were direct instances of cachestore_static with no loader, which meant that the get_versioned and set_versioned functions did not work. --- cache/disabledlib.php | 9 ++++-- cache/tests/allow_temporary_caches_test.php | 34 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/cache/disabledlib.php b/cache/disabledlib.php index 380a474804397..510d77a4fc468 100644 --- a/cache/disabledlib.php +++ b/cache/disabledlib.php @@ -294,8 +294,13 @@ public function create_cache_from_definition($component, $area, array $identifie $definition = $this->create_definition($component, $area); // The cachestore_static class returns true to all three 'SUPPORTS_' checks so it // can be used with all definitions. - $cache = new cachestore_static('TEMP:' . $component . '/' . $area); - $cache->initialise($definition); + $store = new cachestore_static('TEMP:' . $component . '/' . $area); + $store->initialise($definition); + // We need to use a cache loader wrapper rather than directly returning the store, + // or it wouldn't have support for versioning. The cache_application class is used + // (rather than cache_request which might make more sense logically) because it + // includes support for locking, which might be necessary for some caches. + $cache = new cache_application($definition, $store); self::$tempcaches[$key] = $cache; } return $cache; diff --git a/cache/tests/allow_temporary_caches_test.php b/cache/tests/allow_temporary_caches_test.php index 4fd950b953850..cd1879830f61d 100644 --- a/cache/tests/allow_temporary_caches_test.php +++ b/cache/tests/allow_temporary_caches_test.php @@ -62,4 +62,38 @@ protected function inner_is_allowed(): void { $gecko = new allow_temporary_caches(); $this->assertTrue(allow_temporary_caches::is_allowed()); } + + /** + * Tests that the temporary caches actually work, including normal and versioned get and set. + */ + public function test_temporary_cache(): void { + $this->resetAfterTest(); + + // Disable the cache. + \cache_phpunit_factory::phpunit_disable(); + try { + // Try using the cache now - it returns false/null for everything. + $cache = \cache::make('core', 'coursemodinfo'); + $cache->set('frog', 'ribbit'); + $this->assertFalse($cache->get('frog')); + $cache->set_versioned('toad', 2, 'croak'); + $this->assertFalse($cache->get_versioned('toad', 2)); + + // But when we allow temporary caches, it should work as normal. + $allow = new allow_temporary_caches(); + $cache = \cache::make('core', 'coursemodinfo'); + $cache->set('frog', 'ribbit'); + $this->assertEquals('ribbit', $cache->get('frog')); + $cache->set_versioned('toad', 2, 'croak'); + $this->assertEquals('croak', $cache->get_versioned('toad', 2)); + + // Let's actually use modinfo, to check it works with locking too. + $course = $this->getDataGenerator()->create_course(); + get_fast_modinfo($course); + } finally { + // You have to do this after phpunit_disable or it breaks later tests. + \cache_factory::reset(); + \cache_factory::instance(true); + } + } }