diff --git a/cache/classes/helper.php b/cache/classes/helper.php index 5f42b24909c..b3a5d083d21 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -861,4 +861,16 @@ class cache_helper { } return $warnings; } + + /** + * A helper to determine whether a result was found. + * + * This has been deemed required after people have been confused by the fact that [] == false. + * + * @param mixed $value + * @return bool + */ + public static function result_found($value): bool { + return $value !== false; + } } diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index 706a6e7ae31..a8ebef37503 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -470,7 +470,7 @@ class cache implements cache_loader { } } else { // If there's no result, obviously it doesn't meet the required version. - if ($result === false) { + if (!cache_helper::result_found($result)) { return false; } if (!($result instanceof \core_cache\version_wrapper)) { @@ -503,7 +503,7 @@ class cache implements cache_loader { if ($usesstaticacceleration) { $result = $this->static_acceleration_get($key); - if ($result !== false && self::check_version($result, $requiredversion)) { + if (cache_helper::result_found($result) && self::check_version($result, $requiredversion)) { if ($requiredversion === self::VERSION_NONE) { return $result; } else { @@ -518,7 +518,7 @@ class cache implements cache_loader { // 3. Get it from the store. Obviously wasn't in the static acceleration array. $result = $this->store->get($parsedkey); - if ($result !== false) { + if (cache_helper::result_found($result)) { // Check the result has at least the required version. try { $validversion = self::check_version($result, $requiredversion); @@ -548,7 +548,7 @@ class cache implements cache_loader { $this->store->delete($parsedkey); } } - if ($result !== false) { + if (cache_helper::result_found($result)) { // Look to see if there's a TTL wrapper. It might be inside a version wrapper. if ($requiredversion !== self::VERSION_NONE) { $ttlconsider = $result->data; @@ -582,7 +582,7 @@ class cache implements cache_loader { // 4. Load if from the loader/datasource if we don't already have it. $setaftervalidation = false; - if ($result === false) { + if (!cache_helper::result_found($result)) { if ($this->perfdebug) { cache_helper::record_cache_miss($this->store, $this->definition); } @@ -608,13 +608,13 @@ class cache implements cache_loader { } } } - $setaftervalidation = ($result !== false); + $setaftervalidation = (cache_helper::result_found($result)); } else if ($this->perfdebug) { $readbytes = $this->store->get_last_io_bytes(); cache_helper::record_cache_hit($this->store, $this->definition, 1, $readbytes); } // 5. Validate strictness. - if ($strictness === MUST_EXIST && $result === false) { + if ($strictness === MUST_EXIST && !cache_helper::result_found($result)) { 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. Only set to this direct @@ -1372,7 +1372,7 @@ class cache implements cache_loader { $result = $data; } } - if ($result !== false) { + if (cache_helper::result_found($result)) { if ($this->perfdebug) { cache_helper::record_cache_hit(cache_store::STATIC_ACCEL, $this->definition); } diff --git a/cache/tests/cache_helper_test.php b/cache/tests/cache_helper_test.php new file mode 100644 index 00000000000..fb8dc3b1f7e --- /dev/null +++ b/cache/tests/cache_helper_test.php @@ -0,0 +1,64 @@ +. + +namespace core_cache; + +/** + * PHPunit tests for the cache_helper class. + * + * @package core + * @category cache + * @copyright 2023 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversDefaultClass \cache_helper + */ +class cache_helper_test extends \advanced_testcase { + /** + * Test the result_found method. + * + * @param mixed $value + * @param bool $expected + * @dataProvider result_found_provider + * @covers ::result_found + */ + public function test_result_found($value, bool $expected): void { + $this->assertEquals($expected, \cache_helper::result_found($value)); + } + + /** + * Data provider for result_found tests. + * + * @return array + */ + public function result_found_provider(): array { + return [ + // Only false values are considered as not found. + [false, false], + + // The rest are considered valid values. + [null, true], + [0, true], + ['', true], + [[], true], + [new \stdClass(), true], + [true, true], + [1, true], + ['a', true], + [[1], true], + [new \stdClass(), true], + ]; + } +} diff --git a/cache/upgrade.txt b/cache/upgrade.txt index 3a63c137790..d2a072e3ffa 100644 --- a/cache/upgrade.txt +++ b/cache/upgrade.txt @@ -1,5 +1,9 @@ This files describes API changes in /cache/stores/* - cache store plugins. Information provided here is intended especially for developers. + +=== 4.2.2 === +* A new cache_helper::result_found() helper has been added to assist with cache value validation + === 4.2 === * The memcached cachestore has been removed. * The mongodb cachestore has been removed.