MDL-78466 cache: Add new result_found helper

This helper is intended to prevent recurrences of this issue by moving
the guess-work to something well tested.

cache/classes/helper.php

@@ -858,4 +858,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;
+    }
 }

cache/classes/loaders.php

@@ -457,7 +457,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)) {
@@ -490,7 +490,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 {
@@ -505,7 +505,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);
@@ -535,7 +535,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;
@@ -569,7 +569,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);
             }
@@ -595,13 +595,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
@@ -1345,7 +1345,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);
             }

cache/tests/cache_helper_test.php

@@ -0,0 +1,64 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace core_cache;
+
+/**
+ * PHPunit tests for the cache_helper class.
+ *
+ * @package    core
+ * @category   cache
+ * @copyright  2023 Andrew Lyons <andrew@nicols.co.uk>
+ * @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],
+        ];
+    }
+}

cache/upgrade.txt

@@ -1,6 +1,9 @@
 This files describes API changes in /cache/stores/* - cache store plugins.
 Information provided here is intended especially for developers.
 
+=== 4.0.10 ===
+* A new cache_helper::result_found() helper has been added to assist with cache value validation
+
 === 4.0 ===
 * Cache stores may implement new optional function cache_store::get_last_io_bytes() to provide
   information about the size of data transferred (shown in footer if performance info enabled).