Skip to content

Commit

Permalink
MDL-76784 core_cache: versioned cache (modinfo) can fail in install
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sammarshallou committed Feb 21, 2023
1 parent 8503f2c commit a3b1e3b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
9 changes: 7 additions & 2 deletions cache/disabledlib.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
34 changes: 34 additions & 0 deletions cache/tests/allow_temporary_caches_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

0 comments on commit a3b1e3b

Please sign in to comment.