diff --git a/availability/condition/completion/tests/condition_test.php b/availability/condition/completion/tests/condition_test.php index c37940bbdd7..1793eaeb950 100644 --- a/availability/condition/completion/tests/condition_test.php +++ b/availability/condition/completion/tests/condition_test.php @@ -210,6 +210,8 @@ class availability_completion_condition_testcase extends advanced_testcase { 'completion' => COMPLETION_TRACKING_AUTOMATIC]); $DB->set_field('course_modules', 'completiongradeitemnumber', 0, ['id' => $assignrow->cmid]); + // As we manually set the field here, we are going to need to reset the modinfo cache. + rebuild_course_cache($course->id, true); $assign = new assign(context_module::instance($assignrow->cmid), false, false); // Get basic details. diff --git a/cache/classes/interfaces.php b/cache/classes/interfaces.php index 1ef4ebc8625..a8e394e9a62 100644 --- a/cache/classes/interfaces.php +++ b/cache/classes/interfaces.php @@ -49,22 +49,31 @@ interface cache_loader { public function get($key, $strictness = IGNORE_MISSING); /** - * Retrieves the value for the given key with the given version. + * Retrieves the value and actual version for the given key, with at least the required version. * - * This function must be used for localisable caches (where the cache could be stored on a local - * server as well as a shared cache). Specifying the version means that it will automatically - * retrieve the correct version if available, either from the local server or [if that has an - * older version] from the shared server. + * If there is no value for the key, or there is a value but it doesn't have the required + * version, then this function will return null (or throw an exception if you set strictness + * to MUST_EXIST). + * + * This function can be used to make it easier to support localisable caches (where the cache + * could be stored on a local server as well as a shared cache). Specifying the version means + * that it will automatically retrieve the correct version if available, either from the local + * server or [if that has an older version] from the shared server. + * + * If the cached version is newer than specified version, it will be returned regardless. For + * example, if you request version 4, but the locally cached version is 5, it will be returned. + * If you request version 6, and the locally cached version is 5, then the system will look in + * higher-level caches (if any); if there still isn't a version 6 or greater, it will return + * null. * * You must use this function if you use set_versioned. * * @param string|int $key The key for the data being requested. - * @param string $versiontag A unique identifier for the version of the data + * @param int $requiredversion Minimum required version of the data * @param int $strictness One of IGNORE_MISSING or MUST_EXIST. - * @return mixed The data retrieved from the cache, or false if the key did not exist within the cache. - * If MUST_EXIST was used then an exception will be thrown if the key does not exist within the cache. + * @return cache_version_wrapper|null Object containing data and version, or null */ - public function get_versioned($key, string $versiontag, int $strictness = IGNORE_MISSING); + public function get_versioned($key, int $requiredversion, int $strictness = IGNORE_MISSING): ?cache_version_wrapper; /** * Retrieves an array of values for an array of keys. @@ -107,21 +116,21 @@ interface cache_loader { * this one. This function should only be used if there is a known 'current version' (e.g. * stored in a database table). It only ensures that the cache does not return outdated data. * - * This function must be used for localisable caches (where the cache could be stored on a local - * server as well as a shared cache). The version will be recorded alongside the item and - * get_versioned will always return the correct version. + * This function can be used to help implement localisable caches (where the cache could be + * stored on a local server as well as a shared cache). The version will be recorded alongside + * the item and get_versioned will always return the correct version. * - * The version tag can be a version number or hash - the cache system doesn't interpret it, - * only checks it matches when you call get_versioned. + * The version number must be an integer that always increases. This could be based on the + * current time, or a stored value that increases by 1 each time it changes, etc. * * If you use this function you must use get_versioned to retrieve the data. * * @param string|int $key The key for the data being set. - * @param string $versiontag A unique identifier for the version of the data + * @param int $version Integer for the version of the data * @param mixed $data The data to set against the key. * @return bool True on success, false otherwise. */ - public function set_versioned($key, string $versiontag, $data): bool; + public function set_versioned($key, int $version, $data): bool; /** * Sends several key => value pairs to the cache. @@ -477,6 +486,28 @@ interface cache_data_source { public function load_many_for_cache(array $keys); } +/** + * Versionable cache data source. + * + * This interface extends the main cache data source interface to add an extra required method if + * the data source is to be used for a versioned cache. + * + * @package core_cache + */ +interface cache_data_source_versionable extends cache_data_source { + /** + * Loads the data for the key provided ready formatted for caching. + * + * If there is no data for that key, or if the data for the required key has an older version + * than the specified $requiredversion, then this returns null. + * + * @param string|int $key The key to load. + * @param int $requiredversion Minimum required version + * @return cache_version_wrapper|null Versioned data, or null if none + */ + public function load_for_cache_versioned($key, int $requiredversion): ?cache_version_wrapper; +} + /** * Cacheable object. * diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index 4092e888c34..a72ecaf2fb7 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -42,6 +42,11 @@ defined('MOODLE_INTERNAL') || die(); */ class cache implements cache_loader { + /** + * @var int Constant for cache entries that do not have a version number + */ + const NO_VERSION = -1; + /** * We need a timestamp to use within the cache API. * This stamp needs to be used for all ttl and time based operations to ensure that we don't end up with @@ -397,54 +402,73 @@ class cache implements cache_loader { * @throws coding_exception */ public function get($key, $strictness = IGNORE_MISSING) { - return $this->get_implementation($key, '', $strictness); + return $this->get_implementation($key, self::NO_VERSION, $strictness); } /** - * Retrieves the value for the given key with the given version. + * Retrieves the value and actual version for the given key, with at least the required version. * - * This function must be used for localisable caches (where the cache could be stored on a local - * server as well as a shared cache). Specifying the version means that it will automatically - * retrieve the correct version if available, either from the local server or [if that has an - * older version] from the shared server. + * If there is no value for the key, or there is a value but it doesn't have the required + * version, then this function will return null (or throw an exception if you set strictness + * to MUST_EXIST). + * + * This function can be used to make it easier to support localisable caches (where the cache + * could be stored on a local server as well as a shared cache). Specifying the version means + * that it will automatically retrieve the correct version if available, either from the local + * server or [if that has an older version] from the shared server. + * + * If the cached version is newer than specified version, it will be returned regardless. For + * example, if you request version 4, but the locally cached version is 5, it will be returned. + * If you request version 6, and the locally cached version is 5, then the system will look in + * higher-level caches (if any); if there still isn't a version 6 or greater, it will return + * null. * * You must use this function if you use set_versioned. * * @param string|int $key The key for the data being requested. - * @param string $versiontag A unique identifier for the version of the data + * @param int $requiredversion Minimum required version of the data * @param int $strictness One of IGNORE_MISSING or MUST_EXIST. - * @return mixed The data retrieved from the cache, or false if the key did not exist within the cache. - * If MUST_EXIST was used then an exception will be thrown if the key does not exist within the cache. + * @return cache_version_wrapper|null Object containing data and version, or null + * @throws \coding_exception If you call get_versioned on a non-versioned cache key */ - public function get_versioned($key, string $versiontag, int $strictness = IGNORE_MISSING) { - return $this->get_implementation($key, $versiontag, $strictness); + public function get_versioned($key, int $requiredversion, int $strictness = IGNORE_MISSING): ?cache_version_wrapper { + $result = $this->get_implementation($key, $requiredversion, $strictness); + // Return null not false if not found. + return $result ?: null; } /** - * Checks returned data to see if it matches the specified versiontag. + * Checks returned data to see if it matches the specified version number. + * + * For versioned data, this returns the cache_version_wrapper object (or false). For other + * data, it returns the actual data (or false). * * @param mixed $result Result data - * @param string $versiontag Required version tag or '' if there must be no version tag - * @return false|mixed False or the data (with version tag removed) + * @param int $requiredversion Required version number or NO_VERSION if there must be no version + * @return false|mixed False or the data + * @throws \coding_exception If unexpected type of data (versioned vs non-versioned) is found */ - protected static function check_version_tag($result, string $versiontag) { - if ($versiontag === '') { - if ($result instanceof cache_versiontag_wrapper) { - return false; + protected static function check_version($result, int $requiredversion) { + if ($requiredversion === self::NO_VERSION) { + if ($result instanceof cache_version_wrapper) { + throw new \coding_exception('Unexpectedly found versioned cache entry'); } else { return $result; } } else { // If result is false, not an object, or not a versiontag wrapper, return false. - if (!($result instanceof cache_versiontag_wrapper)) { + if (!$result) { return false; } + if (!($result instanceof cache_version_wrapper)) { + throw new \coding_exception('Unexpectedly found non-versioned cache entry'); + } // If the result doesn't match the required version tag, return false. - if ($result->versiontag !== $versiontag) { + if ($result->version < $requiredversion) { return false; } - // Return the actual data for the result. - return $result->data; + // Return the result. + return $result; } } @@ -454,17 +478,18 @@ class cache implements cache_loader { * @param string|int $key The key for the data being requested. * It can be any structure although using a scalar string or int is recommended in the interests of performance. * In advanced cases an array may be useful such as in situations requiring the multi-key functionality. - * @param string $versiontag A unique identifier for the version of the data or '' if none + * @param int $requiredversion Minimum required version of the data or cache::NO_VERSION * @param int $strictness One of IGNORE_MISSING | MUST_EXIST * @return mixed|false The data from the cache or false if the key did not exist within the cache. * @throws coding_exception */ - protected function get_implementation($key, string $versiontag, int $strictness) { + protected function get_implementation($key, int $requiredversion, int $strictness) { // 1. Get it from the static acceleration array if we can (only when it is enabled and it has already been requested/set). $usesstaticacceleration = $this->use_static_acceleration(); if ($usesstaticacceleration) { $result = $this->static_acceleration_get($key); + $result = self::check_version($result, $requiredversion); if ($result !== false) { return $result; } @@ -475,8 +500,30 @@ class cache implements cache_loader { // 3. Get it from the store. Obviously wasn't in the static acceleration array. $result = $this->store->get($parsedkey); - $result = self::check_version_tag($result, $versiontag); + if ($result) { + // Check the result has at least the required version. + try { + $result = self::check_version($result, $requiredversion); + } catch (\coding_exception $e) { + // If we get an exception because there is incorrect data in the cache (not + // versioned when it ought to be), delete it so this exception goes away next time. + // The exception should only happen if there is a code bug (which is why we still + // throw it) but there are unusual scenarios where it might happen and that would + // be annoying if it doesn't fix itself. + $this->store->delete($parsedkey); + throw $e; + } + + // If the result was too old, delete it immediately. This improves performance in the + // case when the cache item is large and there may be multiple clients simultaneously + // requesting it - they won't all have to do a megabyte of IO just in order to find + // that it's out of date. + if (!$result) { + $this->store->delete($parsedkey); + } + } if ($result !== false) { + if ($requiredversion === self::NO_VERSION) { if ($result instanceof cache_ttl_wrapper) { if ($result->has_expired()) { $this->store->delete($parsedkey); @@ -485,6 +532,16 @@ class cache implements cache_loader { $result = $result->data; } } + } else { + if ($result->data instanceof cache_ttl_wrapper) { + if ($result->data->has_expired()) { + $this->store->delete($parsedkey); + $result = false; + } else { + $result->data = $result->data->data; + } + } + } if ($usesstaticacceleration) { $this->static_acceleration_set($key, $result); } @@ -503,14 +560,27 @@ class cache implements cache_loader { // We must pass the original (unparsed) key to the next loader in the chain. // The next loader will parse the key as it sees fit. It may be parsed differently // depending upon the capabilities of the store associated with the loader. - if ($versiontag === '') { + if ($requiredversion === self::NO_VERSION) { $result = $this->loader->get($key); } else { - $result = $this->loader->get_versioned($key, $versiontag); + $result = $this->loader->get_versioned($key, $requiredversion); + // Within this function we use 'false' not null to indicate missing data. + $result = $result ?? false; } } else if ($this->datasource !== false) { + if ($requiredversion === self::NO_VERSION) { $result = $this->datasource->load_for_cache($key); - $result = self::check_version_tag($result, $versiontag); + } else { + if (!$this->datasource instanceof cache_data_source_versionable) { + throw new \coding_exception('Data source is not versionable'); + } + $result = $this->datasource->load_for_cache_versioned($key, $requiredversion); + if ($result && $result->version < $requiredversion) { + throw new \coding_exception('Data source returned outdated version'); + } + // Within this function we use 'false' not null to indicate missing data. + $result = $result ?? false; + } } $setaftervalidation = ($result !== false); } else if ($this->perfdebug) { @@ -521,12 +591,14 @@ class cache implements cache_loader { if ($strictness === MUST_EXIST && $result === false) { throw new coding_exception('Requested key did not exist in any cache stores and could not be loaded.'); } - // 6. Set it to the store if we got it from the loader/datasource. + // 6. Set it to the store if we got it from the loader/datasource. Only set to this direct + // store; parent method will have set it to all stores if needed. if ($setaftervalidation) { - if ($versiontag === '') { - $this->set($key, $result); + if ($requiredversion === self::NO_VERSION) { + $this->set_implementation($key, self::NO_VERSION, $result, false); } else { - $this->set_versioned($key, $versiontag, $result); + // Set it using the real version from the data, not the requested one. + $this->set_implementation($key, $result->version, $result->data, false); } } // 7. Make sure we don't pass back anything that could be a reference. @@ -703,43 +775,51 @@ class cache implements cache_loader { * @return bool True on success, false otherwise. */ public function set($key, $data) { - return $this->set_implementation($key, '', $data); + return $this->set_implementation($key, self::NO_VERSION, $data); } /** * Sets the value for the given key with the given version. * - * This function must be used for localisable caches (where the cache could be stored on a local - * server as well as a shared cache). The version will be recorded alongside the item and - * get_versioned will always return the correct version. + * The cache does not store multiple versions - any existing version will be overwritten with + * this one. This function should only be used if there is a known 'current version' (e.g. + * stored in a database table). It only ensures that the cache does not return outdated data. + * + * This function can be used to help implement localisable caches (where the cache could be + * stored on a local server as well as a shared cache). The version will be recorded alongside + * the item and get_versioned will always return the correct version. + * + * The version number must be an integer that always increases. This could be based on the + * current time, or a stored value that increases by 1 each time it changes, etc. * * If you use this function you must use get_versioned to retrieve the data. * * @param string|int $key The key for the data being set. - * @param string $versiontag A unique identifier for the version of the data + * @param int $version Integer for the version of the data * @param mixed $data The data to set against the key. * @return bool True on success, false otherwise. */ - public function set_versioned($key, string $versiontag, $data): bool { - return $this->set_implementation($key, $versiontag, $data); + public function set_versioned($key, int $version, $data): bool { + return $this->set_implementation($key, $version, $data); } /** * Sets the value for the given key, optionally with a version tag. * * @param string|int $key The key for the data being set. - * @param string $versiontag A unique identifier for the version of the data or '' if none + * @param int $version Version number for the data or cache::NO_VERSION if none * @param mixed $data The data to set against the key. + * @param bool $setparents If true, sets all parent loaders, otherwise only this one * @return bool True on success, false otherwise. */ - protected function set_implementation($key, string $versiontag, $data): bool { - if ($this->loader !== false) { + protected function set_implementation($key, int $version, $data, bool $setparents = true): bool { + if ($this->loader !== false && $setparents) { // We have a loader available set it there as well. // We have to let the loader do its own parsing of data as it may be unique. - if ($versiontag === '') { + if ($version === self::NO_VERSION) { $this->loader->set($key, $data); } else { - $this->loader->set_versioned($key, $versiontag, $data); + $this->loader->set_versioned($key, $version, $data); } } $usestaticacceleration = $this->use_static_acceleration(); @@ -755,7 +835,12 @@ class cache implements cache_loader { } if ($usestaticacceleration) { + // Static acceleration cache should include the cache version wrapper, but not TTL. + if ($version === self::NO_VERSION) { $this->static_acceleration_set($key, $data); + } else { + $this->static_acceleration_set($key, new cache_version_wrapper($data, $version)); + } } if ($this->has_a_ttl() && !$this->store_supports_native_ttl()) { @@ -763,8 +848,8 @@ class cache implements cache_loader { } $parsedkey = $this->parse_key($key); - if ($versiontag !== '') { - $data = new cache_versiontag_wrapper($data, $versiontag); + if ($version !== self::NO_VERSION) { + $data = new cache_version_wrapper($data, $version); } $success = $this->store->set($parsedkey, $data); @@ -1616,15 +1701,16 @@ class cache_application extends cache implements cache_loader_with_locking { * * * @param string|int $key The key for the data being requested. - * @param string $versiontag Version tag + * @param int $version Version number * @param mixed $data The data to set against the key. + * @param bool $setparents If true, sets all parent loaders, otherwise only this one * @return bool True on success, false otherwise. */ - protected function set_implementation($key, string $versiontag, $data): bool { + protected function set_implementation($key, int $version, $data, bool $setparents = true): bool { if ($this->requirelockingwrite && !$this->acquire_lock($key)) { return false; } - $result = parent::set_implementation($key, $versiontag, $data); + $result = parent::set_implementation($key, $version, $data, $setparents); if ($this->requirelockingwrite && !$this->release_lock($key)) { debugging('Failed to release cache lock on set operation... this should not happen.', DEBUG_DEVELOPER); } diff --git a/cache/disabledlib.php b/cache/disabledlib.php index 49248bda555..29b0d5868bd 100644 --- a/cache/disabledlib.php +++ b/cache/disabledlib.php @@ -91,11 +91,12 @@ class cache_disabled extends cache { * Sets a key value pair in the cache. * * @param int|string $key Unused. - * @param string $versiontag Unused. + * @param int $version Unused. * @param mixed $data Unused. + * @param bool $setparents Unused. * @return bool */ - protected function set_implementation($key, string $versiontag, $data): bool { + protected function set_implementation($key, int $version, $data, bool $setparents = true): bool { return false; } diff --git a/cache/lib.php b/cache/lib.php index 357b8c657b5..a711bd37c1d 100644 --- a/cache/lib.php +++ b/cache/lib.php @@ -127,9 +127,9 @@ class cache_ttl_wrapper { } /** - * Class wrapping information in the cache that is tagged with a version marker. + * Class wrapping information in the cache that is tagged with a version number. */ -class cache_versiontag_wrapper { +class cache_version_wrapper { /** * The data being stored. @@ -138,20 +138,20 @@ class cache_versiontag_wrapper { public $data; /** - * Version tag for the data - * @var string + * Version number for the data + * @var int */ - public $versiontag; + public $version; /** * Constructs a version tag wrapper. * * @param mixed $data - * @param string $versiontag Version tag + * @param int $version Version number */ - public function __construct($data, string $versiontag) { + public function __construct($data, int $version) { $this->data = $data; - $this->versiontag = $versiontag; + $this->version = $version; } } diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index c7fd8ff9e17..de7b41edcc6 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -14,32 +14,44 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * PHPunit tests for the cache API - * - * This file is part of Moodle's cache API, affectionately called MUC. - * It contains the components that are requried in order to use caching. - * - * @package core - * @category cache - * @copyright 2012 Sam Hemelryk - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); +namespace core_cache; -// Include the necessary evils. -global $CFG; -require_once($CFG->dirroot.'/cache/locallib.php'); -require_once($CFG->dirroot.'/cache/tests/fixtures/lib.php'); +use \advanced_testcase; +use \cache; +use \cache_config; +use \cache_config_testing; +use \cache_factory; +use \cache_helper; +use \cache_loader; +use \cache_phpunit_application; +use \cache_phpunit_cache; +use \cache_phpunit_dummy_object; +use \cache_phpunit_factory; +use \cache_phpunit_session; +use \cacheable_object_array; +use \cache_store; +use \coding_exception; +use \Exception; +use \stdClass; /** * PHPunit tests for the cache API * + * @package core_cache * @copyright 2012 Sam Hemelryk * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class core_cache_testcase extends advanced_testcase { +class cache_test extends advanced_testcase { + + /** + * Load required libraries and fixtures. + */ + public static function setUpBeforeClass(): void { + global $CFG; + + require_once($CFG->dirroot . '/cache/locallib.php'); + require_once($CFG->dirroot . '/cache/tests/fixtures/lib.php'); + } /** * Set things back to the default before each test. @@ -549,6 +561,58 @@ class core_cache_testcase extends advanced_testcase { $this->assertEquals('c has no value really.', $result['c']); } + /** + * Tests a definition using a data loader with versioned keys. + */ + public function test_definition_data_loader_versioned() { + // Create two definitions, one using a non-versionable data source and the other using + // a versionable one. + $instance = cache_config_testing::instance(true); + $instance->phpunit_add_definition('phpunit/datasourcetest1', array( + 'mode' => cache_store::MODE_APPLICATION, + 'component' => 'phpunit', + 'area' => 'datasourcetest1', + 'datasource' => 'cache_phpunit_dummy_datasource', + 'datasourcefile' => 'cache/tests/fixtures/lib.php' + )); + $instance->phpunit_add_definition('phpunit/datasourcetest2', array( + 'mode' => cache_store::MODE_APPLICATION, + 'component' => 'phpunit', + 'area' => 'datasourcetest2', + 'datasource' => 'cache_phpunit_dummy_datasource_versionable', + 'datasourcefile' => 'cache/tests/fixtures/lib.php' + )); + + // The first data source works for normal 'get'. + $cache1 = cache::make('phpunit', 'datasourcetest1'); + $this->assertEquals('Frog has no value really.', $cache1->get('Frog')); + + // But it doesn't work for get_versioned. + try { + $cache1->get_versioned('zombie', 1); + $this->fail(); + } catch (\coding_exception $e) { + $this->assertStringContainsString('Data source is not versionable', $e->getMessage()); + } + + // The second data source works for get_versioned. Set up the datasource first. + $cache2 = cache::make('phpunit', 'datasourcetest2'); + + $datasource = \cache_phpunit_dummy_datasource_versionable::get_last_instance(); + $datasource->has_value('frog', 3, 'Kermit'); + + // Check data with no value. + $this->assertNull($cache2->get_versioned('zombie', 1)); + + // Check data with value in datastore of required version. + $result = $cache2->get_versioned('frog', 3); + $this->assertEquals('Kermit', $result->data); + $this->assertEquals(3, $result->version); + + // Check when the datastore doesn't have required version. + $this->assertNull($cache2->get_versioned('frog', 4)); + } + /** * Tests a definition using an overridden loader */ @@ -1474,23 +1538,37 @@ class core_cache_testcase extends advanced_testcase { $multicache = cache::make('phpunit', 'multi_loader'); // Basic use of set_versioned and get_versioned. - $multicache->set_versioned('game', '001', 'Pooh-sticks'); - $this->assertEquals('Pooh-sticks', $multicache->get_versioned('game', '001')); + $multicache->set_versioned('game', 1, 'Pooh-sticks'); + $result = $multicache->get_versioned('game', 1); + $this->assertEquals('Pooh-sticks', $result->data); + $this->assertEquals(1, $result->version); // What if you ask for a version that doesn't exist? - $this->assertFalse($multicache->get_versioned('game', '002')); + $this->assertNull($multicache->get_versioned('game', 2)); // What if you use get on a get_version cache? - $this->assertFalse($multicache->get('game')); + $multicache->set_versioned('game', 1, 'Pooh-sticks'); + try { + $multicache->get('game'); + $this->fail(); + } catch (\coding_exception $e) { + $this->assertStringContainsString('Unexpectedly found versioned cache entry', $e->getMessage()); + } // Or get_version on a get cache? $multicache->set('toy', 'Train set'); - $this->assertFalse($multicache->get_versioned('toy', '001')); + try { + $multicache->get_versioned('toy', 1); + $this->fail(); + } catch (\coding_exception $e) { + $this->assertStringContainsString('Unexpectedly found non-versioned cache entry', $e->getMessage()); + } - // Setting a new version wipes out the old version. - $multicache->set_versioned('game', '002', 'Tag'); - $this->assertFalse($multicache->get_versioned('game', '001')); - $this->assertEquals('Tag', $multicache->get_versioned('game', '002')); + // Setting a new version wipes out the old version; if you request it, you get the new one. + $multicache->set_versioned('game', 2, 'Tag'); + $result = $multicache->get_versioned('game', 1); + $this->assertEquals('Tag', $result->data); + $this->assertEquals(2, $result->version); // Get the two separate cache stores for the multi-level cache. $factory = cache_factory::instance(); @@ -1502,28 +1580,139 @@ class core_cache_testcase extends advanced_testcase { $hashgame = cache_helper::hash_key('game', $definition); $data = 'British Bulldog'; if ($ttl) { - $data = new cache_ttl_wrapper($data, 600); + $data = new \cache_ttl_wrapper($data, 600); } - $storeb->set($hashgame, new cache_versiontag_wrapper($data, '003')); + $storeb->set($hashgame, new \cache_version_wrapper($data, 3)); // If we ask for the old one we'll get it straight off... - $this->assertEquals('Tag', $multicache->get_versioned('game', '002')); + $this->assertEquals('Tag', $multicache->get_versioned('game', 2)->data); // But if we ask for the new one it will still get it via the shared cache. - $this->assertEquals('British Bulldog', $multicache->get_versioned('game', '003')); + $this->assertEquals('British Bulldog', $multicache->get_versioned('game', 3)->data); // Also, now it will be updated in the local cache as well. $localvalue = $storea->get($hashgame); if ($ttl) { // In case the time has changed slightly since the first set, we can't do an exact // compare, so check it ignoring the time field. - $this->assertEquals('003', $localvalue->versiontag); + $this->assertEquals(3, $localvalue->version); $ttldata = $localvalue->data; $this->assertInstanceOf('cache_ttl_wrapper', $ttldata); $this->assertEquals('British Bulldog', $ttldata->data); } else { - $this->assertEquals(new cache_versiontag_wrapper('British Bulldog', '003'), $localvalue); + $this->assertEquals(new \cache_version_wrapper('British Bulldog', 3), $localvalue); } + + // If we ask for a newer version, then any older version should be deleted in each + // cache level. + $this->assertNull($multicache->get_versioned('game', 4)); + $this->assertFalse($storea->get($hashgame)); + $this->assertFalse($storeb->get($hashgame)); + } + + /** + * Tests a versioned cache when using static cache. + */ + public function test_versioned_cache_static(): void { + $instance = cache_config_testing::instance(true); + $instance->phpunit_add_file_store('a', false); + $defarray = [ + 'mode' => cache_store::MODE_APPLICATION, + 'component' => 'phpunit', + 'area' => 'versioned_static', + 'staticacceleration' => true, + 'staticaccelerationsize' => 10 + ]; + $instance->phpunit_add_definition('phpunit/versioned_static', $defarray, false); + $instance->phpunit_add_definition_mapping('phpunit/versioned_static', 'a', 1); + + $staticcache = cache::make('phpunit', 'versioned_static'); + + // Set a value in the cache, version 1. This will store it in static acceleration. + $staticcache->set_versioned('game', 1, 'Pooh-sticks'); + + // Get the two cache store. + $factory = cache_factory::instance(); + $definition = $factory->create_definition('phpunit', 'versioned_static'); + [0 => $storea] = $factory->get_store_instances_in_use($definition); + + // Hack a newer version into cache store without directly calling set (now the static + // has v1, store has v2). This simulates another client updating the cache. + $hashgame = cache_helper::hash_key('game', $definition); + $storea->set($hashgame, new \cache_version_wrapper('Tag', 2)); + + // Get the key from the cache, v1. This will use static acceleration. + $this->assertEquals('Pooh-sticks', $staticcache->get_versioned('game', 1)->data); + + // Now if we ask for a newer version, it should not use the static cached one. + $this->assertEquals('Tag', $staticcache->get_versioned('game', 2)->data); + + // This get should have updated static acceleration, so it will be used next time without + // a store request. + $storea->set($hashgame, new \cache_version_wrapper('British Bulldog', 3)); + $this->assertEquals('Tag', $staticcache->get_versioned('game', 2)->data); + + // Requesting the higher version will get rid of static acceleration again. + $this->assertEquals('British Bulldog', $staticcache->get_versioned('game', 3)->data); + + // Finally ask for a version that doesn't exist anywhere, just to confirm it returns null. + $this->assertNull($staticcache->get_versioned('game', 4)); + } + + /** + * Tests 3-layer versioned caches. + */ + public function test_versioned_cache_3_layers(): void { + $instance = cache_config_testing::instance(true); + $instance->phpunit_add_file_store('a', false); + $instance->phpunit_add_file_store('b', false); + $instance->phpunit_add_file_store('c', false); + $defarray = [ + 'mode' => cache_store::MODE_APPLICATION, + 'component' => 'phpunit', + 'area' => 'multi_loader' + ]; + $instance->phpunit_add_definition('phpunit/multi_loader', $defarray, false); + $instance->phpunit_add_definition_mapping('phpunit/multi_loader', 'a', 1); + $instance->phpunit_add_definition_mapping('phpunit/multi_loader', 'b', 2); + $instance->phpunit_add_definition_mapping('phpunit/multi_loader', 'c', 3); + + $multicache = cache::make('phpunit', 'multi_loader'); + + // Basic use of set_versioned and get_versioned. + $multicache->set_versioned('game', 1, 'Pooh-sticks'); + $this->assertEquals('Pooh-sticks', $multicache->get_versioned('game', 1)->data); + + // What if you ask for a version that doesn't exist? + $this->assertNull($multicache->get_versioned('game', 2)); + + // Setting a new version wipes out the old version; if you request it, you get the new one. + $multicache->set_versioned('game', 2, 'Tag'); + $this->assertEquals('Tag', $multicache->get_versioned('game', 1)->data); + + // Get the three separate cache stores for the multi-level cache. + $factory = cache_factory::instance(); + $definition = $factory->create_definition('phpunit', 'multi_loader'); + [0 => $storea, 1 => $storeb, 2 => $storec] = $factory->get_store_instances_in_use($definition); + + // Set up 3 different versions in each level. + $hashgame = cache_helper::hash_key('game', $definition); + $storeb->set($hashgame, new \cache_version_wrapper('British Bulldog', 3)); + $storec->set($hashgame, new \cache_version_wrapper('Hopscotch', 4)); + + // First request can be satisfied from A; second request requires B... + $this->assertEquals('Tag', $multicache->get_versioned('game', 2)->data); + $this->assertEquals('British Bulldog', $multicache->get_versioned('game', 3)->data); + + // And should update the data in A. + $this->assertEquals(new \cache_version_wrapper('British Bulldog', 3), $storea->get($hashgame)); + $this->assertEquals('British Bulldog', $multicache->get_versioned('game', 1)->data); + + // But newer data should still be in C. + $this->assertEquals('Hopscotch', $multicache->get_versioned('game', 4)->data); + // Now it's stored in A and B too. + $this->assertEquals(new \cache_version_wrapper('Hopscotch', 4), $storea->get($hashgame)); + $this->assertEquals(new \cache_version_wrapper('Hopscotch', 4), $storeb->get($hashgame)); } /** diff --git a/cache/tests/fixtures/lib.php b/cache/tests/fixtures/lib.php index 37ae3731b99..62f1a801549 100644 --- a/cache/tests/fixtures/lib.php +++ b/cache/tests/fixtures/lib.php @@ -413,6 +413,70 @@ class cache_phpunit_dummy_datasource implements cache_data_source { } } +/** + * A versionable dummy datasource. + * + * @package core_cache + */ +class cache_phpunit_dummy_datasource_versionable extends cache_phpunit_dummy_datasource + implements cache_data_source_versionable { + /** @var array Data in cache */ + protected $data = []; + + /** @var cache_phpunit_dummy_datasource_versionable Last created instance */ + protected static $lastinstance; + + /** + * Returns an instance of this object for use with the cache. + * + * @param cache_definition $definition + * @return cache_phpunit_dummy_datasource New object + */ + public static function get_instance_for_cache(cache_definition $definition): + cache_phpunit_dummy_datasource_versionable { + self::$lastinstance = new cache_phpunit_dummy_datasource_versionable(); + return self::$lastinstance; + } + + /** + * Gets the last instance that was created. + * + * @return cache_phpunit_dummy_datasource_versionable + */ + public static function get_last_instance(): cache_phpunit_dummy_datasource_versionable { + return self::$lastinstance; + } + + /** + * Sets up the datasource so that it has a value for a particular key. + * + * @param string $key Key + * @param int $version Version for key + * @param mixed $data + */ + public function has_value(string $key, int $version, $data): void { + $this->data[$key] = new cache_version_wrapper($data, $version); + } + + /** + * Loads versioned data. + * + * @param int|string $key Key + * @param int $requiredversion Minimum version number + * @return cache_version_wrapper|null Versioned data or null if not found + */ + public function load_for_cache_versioned($key, int $requiredversion): ?cache_version_wrapper { + if (!array_key_exists($key, $this->data)) { + return null; + } + $value = $this->data[$key]; + if ($value->version < $requiredversion) { + return null; + } + return $value; + } +} + /** * PHPUnit application cache loader. * diff --git a/cache/upgrade.txt b/cache/upgrade.txt index 1d30ed6bd4c..b42024bff76 100644 --- a/cache/upgrade.txt +++ b/cache/upgrade.txt @@ -9,7 +9,8 @@ Information provided here is intended especially for developers. store to provide better information for the new cache usage admin page. * New functions cache::set_versioned() and cache::get_versioned() can be used to ensure correct behaviour when using a multi-level cache with early cache levels stored locally. (Used when - rebuilding modinfo.) + rebuilding modinfo.) There is also a new interface cache_data_source_versionable which can + be implemented if you want to make a data source that supports versioning. === 3.10 === * The function supports_recursion() from the lock_factory interface has been deprecated including the related implementations. diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index c543168c113..1f125c5ea10 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -14,23 +14,61 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +namespace core_course; + +use \advanced_testcase; +use \backup_controller; +use \backup; +use \blog_entry; +use \cache; +use \calendar_event; +use \coding_exception; +use \comment; +use \completion_criteria_date; +use \completion_completion; +use \context_course; +use \context_module; +use \context_system; +use \context_coursecat; +use \core_completion_external; +use \core_external; +use \core_tag_index_builder; +use \core_tag_tag; +use \course_capability_assignment; +use \course_request; +use \core_course_category; +use \enrol_imsenterprise_testcase; +use \external_api; +use \grade_item; +use \grading_manager; +use \moodle_exception; +use \moodle_url; +use \phpunit_util; +use \rating_manager; +use \restore_controller; +use \stdClass; +use \testing_data_generator; + /** * Course related unit tests * - * @package core + * @package core_course * @category phpunit * @copyright 2012 Petr Skoda {@link http://skodak.org} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +class courselib_test extends advanced_testcase { -defined('MOODLE_INTERNAL') || die(); + /** + * Load required libraries and fixtures. + */ + public static function setUpBeforeClass(): void { + global $CFG; -global $CFG; -require_once($CFG->dirroot . '/course/lib.php'); -require_once($CFG->dirroot . '/course/tests/fixtures/course_capability_assignment.php'); -require_once($CFG->dirroot . '/enrol/imsenterprise/tests/imsenterprise_test.php'); - -class core_course_courselib_testcase extends advanced_testcase { + require_once($CFG->dirroot . '/course/lib.php'); + require_once($CFG->dirroot . '/course/tests/fixtures/course_capability_assignment.php'); + require_once($CFG->dirroot . '/enrol/imsenterprise/tests/imsenterprise_test.php'); + } /** * Set forum specific test values for calling create_module(). diff --git a/lib/modinfolib.php b/lib/modinfolib.php index 809f1702122..87972c5a714 100644 --- a/lib/modinfolib.php +++ b/lib/modinfolib.php @@ -475,20 +475,21 @@ class course_modinfo { $cachecoursemodinfo = cache::make('core', 'coursemodinfo'); // Retrieve modinfo from cache. If not present or cacherev mismatches, call rebuild and retrieve again. - $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); - if ($coursemodinfo === false) { + $coursemodinfowithversion = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); + if (!$coursemodinfowithversion) { $lock = self::get_course_cache_lock($course->id); try { // Only actually do the build if it's still needed after getting the lock (not if // somebody else, who might have been holding the lock, built it already). - $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); - if ($coursemodinfo === false) { - $coursemodinfo = self::inner_build_course_cache($course, $lock); + $coursemodinfowithversion = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev); + if (!$coursemodinfowithversion) { + $coursemodinfowithversion = self::inner_build_course_cache($course->id, $lock); } } finally { $lock->release(); } } + $coursemodinfo = $coursemodinfowithversion->data; // Set initial values $this->userid = $userid; @@ -664,7 +665,8 @@ class course_modinfo { $lock = self::get_course_cache_lock($course->id); try { - return self::inner_build_course_cache($course, $lock); + $versionwrapper = self::inner_build_course_cache($course->id, $lock); + return $versionwrapper->data; } finally { $lock->release(); } @@ -673,25 +675,20 @@ class course_modinfo { /** * Called to build course cache when there is already a lock obtained. * - * @param stdClass $course object from DB table course + * @param int $courseid Course id * @param \core\lock\lock $lock Lock object - not actually used, just there to indicate you have a lock - * @return stdClass Course object that has been stored in MUC + * @return cache_version_wrapper Course object and version that has been stored in MUC */ - protected static function inner_build_course_cache($course, \core\lock\lock $lock) { + protected static function inner_build_course_cache(int $courseid, \core\lock\lock $lock): cache_version_wrapper { global $DB, $CFG; require_once("{$CFG->dirroot}/course/lib.php"); - // Ensure object has all necessary fields. - foreach (self::$cachedfields as $key) { - if (!isset($course->$key)) { - $course = $DB->get_record('course', array('id' => $course->id), - implode(',', array_merge(array('id'), self::$cachedfields)), MUST_EXIST); - break; - } - } + // Always reload the course object from database to ensure we have the latest possible + // value for cacherev. + $course = $DB->get_record('course', ['id' => $courseid], + implode(',', array_merge(['id'], self::$cachedfields)), MUST_EXIST); + // Retrieve all information about activities and sections. - // This may take time on large courses and it is possible that another user modifies the same course during this process. - // Field cacherev stored in both DB and cache will ensure that cached data matches the current course state. $coursemodinfo = new stdClass(); $coursemodinfo->modinfo = get_array_of_activities($course->id); $coursemodinfo->sectioncache = self::build_course_section_cache($course); @@ -701,7 +698,7 @@ class course_modinfo { // Set the accumulated activities and sections information in cache, together with cacherev. $cachecoursemodinfo = cache::make('core', 'coursemodinfo'); $cachecoursemodinfo->set_versioned($course->id, $course->cacherev, $coursemodinfo); - return $coursemodinfo; + return new cache_version_wrapper($coursemodinfo, $course->cacherev); } } diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php index 4c1312891d2..79f7f760597 100644 --- a/lib/tests/modinfolib_test.php +++ b/lib/tests/modinfolib_test.php @@ -14,23 +14,25 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +namespace core; + +use \advanced_testcase; +use \cache; +use \cm_info; +use \coding_exception; +use \context_course; +use \context_module; +use \course_modinfo; +use \moodle_exception; +use \moodle_url; +use \Exception; + /** * Unit tests for lib/modinfolib.php. * * @package core * @category phpunit * @copyright 2012 Andrew Davis - */ - -defined('MOODLE_INTERNAL') || die(); - -global $CFG; -require_once($CFG->libdir . '/modinfolib.php'); - -/** - * Unit tests for modinfolib.php - * - * @copyright 2012 Andrew Davis * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class modinfolib_test extends advanced_testcase { @@ -265,7 +267,7 @@ class modinfolib_test extends advanced_testcase { $modinfo = get_fast_modinfo($course->id); $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id)); $this->assertEquals($prevcacherev, $cacherev); - $cachedvalue = $cache->get_versioned($course->id, $cacherev); + $cachedvalue = $cache->get_versioned($course->id, $cacherev)->data; $this->assertNotEmpty($cachedvalue); $this->assertEquals($cacherev, $cachedvalue->cacherev); $this->assertEquals($cacherev, $modinfo->get_course()->cacherev); @@ -279,7 +281,7 @@ class modinfolib_test extends advanced_testcase { $modinfo = get_fast_modinfo($course->id); $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id)); $this->assertEquals($prevcacherev, $cacherev); - $cachedvalue = $cache->get_versioned($course->id, $cacherev); + $cachedvalue = $cache->get_versioned($course->id, $cacherev)->data; $this->assertNotEmpty($cachedvalue); $this->assertEquals($cacherev, $cachedvalue->cacherev); $this->assertNotEmpty($cachedvalue->secretfield); @@ -290,7 +292,7 @@ class modinfolib_test extends advanced_testcase { rebuild_course_cache($course->id); $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id)); $this->assertGreaterThan($prevcacherev, $cacherev); - $cachedvalue = $cache->get_versioned($course->id, $cacherev); + $cachedvalue = $cache->get_versioned($course->id, $cacherev)->data; $this->assertNotEmpty($cachedvalue); $this->assertEquals($cacherev, $cachedvalue->cacherev); $modinfo = get_fast_modinfo($course->id); @@ -304,7 +306,7 @@ class modinfolib_test extends advanced_testcase { $modinfo = get_fast_modinfo($course->id); $cacherev = $DB->get_field('course', 'cacherev', array('id' => $course->id)); $this->assertGreaterThan($prevcacherev, $cacherev); - $cachedvalue = $cache->get_versioned($course->id, $cacherev); + $cachedvalue = $cache->get_versioned($course->id, $cacherev)->data; $this->assertNotEmpty($cachedvalue); $this->assertEquals($cacherev, $cachedvalue->cacherev); $this->assertEquals($cacherev, $modinfo->get_course()->cacherev); @@ -317,7 +319,7 @@ class modinfolib_test extends advanced_testcase { $this->assertEmpty($cache->get_versioned($course->id, $cacherev)); // Rebuild again. $modinfo = get_fast_modinfo($course->id); - $cachedvalue = $cache->get_versioned($course->id, $cacherev); + $cachedvalue = $cache->get_versioned($course->id, $cacherev)->data; $this->assertNotEmpty($cachedvalue); $this->assertEquals($cacherev, $cachedvalue->cacherev); $this->assertEquals($cacherev, $modinfo->get_course()->cacherev);