From 3353aecc8b633f7a54105586f45cbf31098a0e0f Mon Sep 17 00:00:00 2001 From: sam marshall Date: Wed, 4 Jan 2023 12:02:37 +0000 Subject: [PATCH] MDL-76791 core_cache: Locking breaks for multiple-identifiers store Cache locking fails if the cache store supports multiple identifiers (in core, the only two which do are cachestore_static and cachestore_mongodb, so this is unlikely to cause severe problems). --- cache/classes/loaders.php | 6 +++--- cache/tests/cache_test.php | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index 32ccdf16bae..e78697be15a 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -1679,7 +1679,7 @@ class cache_application extends cache implements cache_loader_with_locking { if ($this->get_loader() !== false) { $this->get_loader()->acquire_lock($key); } - $key = $this->parse_key($key); + $key = cache_helper::hash_key($key, $this->get_definition()); $before = microtime(true); if ($this->nativelocking) { $lock = $this->get_store()->acquire_lock($key, $this->get_identifier()); @@ -1706,7 +1706,7 @@ class cache_application extends cache implements cache_loader_with_locking { * someone else has the lock. */ public function check_lock_state($key) { - $key = $this->parse_key($key); + $key = cache_helper::hash_key($key, $this->get_definition()); if (!empty($this->locks[$key])) { return true; // Shortcut to save having to make a call to the cache store if the lock is held by this process. } @@ -1726,7 +1726,7 @@ class cache_application extends cache implements cache_loader_with_locking { */ public function release_lock($key) { $loaderkey = $key; - $key = $this->parse_key($key); + $key = cache_helper::hash_key($key, $this->get_definition()); if ($this->nativelocking) { $released = $this->get_store()->release_lock($key, $this->get_identifier()); } else { diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index 05242ef4f54..2ee1198f6fc 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -2111,6 +2111,30 @@ class cache_test extends \advanced_testcase { $this->assertFalse($cache->has('a')); } + /** + * The application locking feature should work with caches that support multiple identifiers + * (static cache and MongoDB with a specific setting). + * + * @covers \cache_application + */ + public function test_application_locking_multiple_identifier_cache() { + // Get an arbitrary definition (modinfo). + $instance = cache_config_testing::instance(true); + $definitions = $instance->get_definitions(); + $definition = \cache_definition::load('phpunit', $definitions['core/coursemodinfo']); + + // Set up a static cache using that definition, wrapped in cache_application so we can do + // locking. + $store = new \cachestore_static('test'); + $store->initialise($definition); + $cache = new cache_application($definition, $store); + + // Test the three locking functions. + $cache->acquire_lock('frog'); + $this->assertTrue($cache->check_lock_state('frog')); + $cache->release_lock('frog'); + } + /** * Test requiring a lock before attempting to set a key. *