Skip to content

Commit

Permalink
Merge branch 'MDL-78466-master' of https://github.com/andrewnicols/mo…
Browse files Browse the repository at this point in the history 
…odle
  • Loading branch information
@junpataleta
junpataleta committed Jun 14, 2023
2 parents 2f0585f + 5138f02 commit b1f0a24
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 8 deletions.
12 changes: 12 additions & 0 deletions cache/classes/helper.php
View file
Expand Up @@ -861,4 +861,16 @@ public static function warnings(array $stores = null) {
}
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;
}
}
16 changes: 8 additions & 8 deletions cache/classes/loaders.php
View file
Expand Up @@ -470,7 +470,7 @@ protected static function check_version($result, int $requiredversion): bool {
}
} else {
// If there's no result, obviously it doesn't meet the required version.
if (!$result) {
if (!cache_helper::result_found($result)) {
return false;
}
if (!($result instanceof \core_cache\version_wrapper)) {
Expand Down Expand Up @@ -503,7 +503,7 @@ protected function get_implementation($key, int $requiredversion, int $strictnes

if ($usesstaticacceleration) {
$result = $this->static_acceleration_get($key);
if ($result && self::check_version($result, $requiredversion)) {
if (cache_helper::result_found($result) && self::check_version($result, $requiredversion)) {
if ($requiredversion === self::VERSION_NONE) {
return $result;
} else {
Expand All @@ -518,7 +518,7 @@ protected function get_implementation($key, int $requiredversion, int $strictnes

// 3. Get it from the store. Obviously wasn't in the static acceleration array.
$result = $this->store->get($parsedkey);
if ($result) {
if (cache_helper::result_found($result)) {
// Check the result has at least the required version.
try {
$validversion = self::check_version($result, $requiredversion);
Expand Down Expand Up @@ -548,7 +548,7 @@ protected function get_implementation($key, int $requiredversion, int $strictnes
$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;
Expand Down Expand Up @@ -582,7 +582,7 @@ protected function get_implementation($key, int $requiredversion, int $strictnes

// 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);
}
Expand All @@ -608,13 +608,13 @@ protected function get_implementation($key, int $requiredversion, int $strictnes
}
}
}
$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
Expand Down Expand Up @@ -1372,7 +1372,7 @@ protected function static_acceleration_get($key) {
$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);
}
Expand Down
64 changes: 64 additions & 0 deletions cache/tests/cache_helper_test.php
View file
@@ -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],
];
}
}
157 changes: 157 additions & 0 deletions cache/tests/cache_test.php
View file
Expand Up @@ -50,6 +50,7 @@
* @copyright 2012 Sam Hemelryk
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @coversDefaultClass \cache
* @covers \cache
*/
class cache_test extends \advanced_testcase {

Expand Down Expand Up @@ -2827,6 +2828,162 @@ public function test_performance_debug() {
$startstats[$requestid]['stores']['default_request']['sets']);
}

/**
* Data provider for static acceleration performance tests.
*
* @return array
*/
public function static_acceleration_performance_provider(): array {
// Note: These are the delta values, not the absolute values.
// Also note that the set will actually store the valuein the static cache immediately.
$validfirst = [
'default_application' => [
'hits' => 1,
'misses' => 0,
],
cache_store::STATIC_ACCEL => [
'hits' => 0,
'misses' => 1,
],
];

$validsecond = [
'default_application' => [
'hits' => 0,
'misses' => 0,
],
cache_store::STATIC_ACCEL => [
'hits' => 1,
'misses' => 0,
],
];

$invalidfirst = [
'default_application' => [
'hits' => 0,
'misses' => 1,
],
cache_store::STATIC_ACCEL => [
'hits' => 0,
'misses' => 1,
],
];
$invalidsecond = [
'default_application' => [
'hits' => 0,
'misses' => 1,
],
cache_store::STATIC_ACCEL => [
'hits' => 0,
'misses' => 1,
],
];;

return [
'Truthy' => [
true,
$validfirst,
$validsecond,
],
'Null' => [
null,
$validfirst,
$validsecond,
],
'Empty Array' => [
[],
$validfirst,
$validsecond,
],
'Empty String' => [
'',
$validfirst,
$validsecond,
],
'False' => [
false,
$invalidfirst,
$invalidsecond,
],
];
}

/**
* Test performance of static acceleration caches with values which are frequently confused with missing values.
*
* @dataProvider static_acceleration_performance_provider
* @param mixed $value The value to test
* @param array $firstfetchstats The expected stats on the first fetch
* @param array $secondfetchstats The expected stats on the subsequent fetch
*/
public function test_static_acceleration_values_performance(
$value,
array $firstfetchstats,
array $secondfetchstats,
): void {
// Note: We need to modify perfdebug to test this.
global $CFG;
$this->resetAfterTest(true);
$CFG->perfdebug = 15;

$instance = cache_config_testing::instance();
$instance->phpunit_add_definition('phpunit/accelerated', [
'mode' => cache_store::MODE_APPLICATION,
'component' => 'phpunit',
'area' => 'accelerated',
'staticacceleration' => true,
'staticaccelerationsize' => 1,
]);

$cache = cache::make('phpunit', 'accelerated');
$this->assertInstanceOf(cache_phpunit_application::class, $cache);

$this->assertTrue($cache->set('value', $value));

$checkstats = function(
array $start,
array $expectedstats,
): array {
$applicationid = 'phpunit/accelerated';
$endstats = cache_helper::get_stats();

$start = $start[$applicationid]['stores'];
$end = $endstats[$applicationid]['stores'];

foreach ($expectedstats as $cachename => $expected) {
foreach ($expected as $type => $value) {
$startvalue = array_key_exists($cachename, $start) ? $start[$cachename][$type] : 0;
$endvalue = array_key_exists($cachename, $end) ? $end[$cachename][$type] : 0;
$diff = $endvalue - $startvalue;
$this->assertEquals(
$value,
$diff,
"Expected $cachename $type to be $value, got $diff",
);
}
}

return $endstats;
};

// Reset the cache factory so that we can get the stats from a fresh instance.
$factory = cache_factory::instance();
$factory->reset_cache_instances();
$cache = cache::make('phpunit', 'accelerated');

// Get the initial stats.
$startstats = cache_helper::get_stats();

// Fetching the value the first time should seed the static cache from the application cache.
$this->assertEquals($value, $cache->get('value'));
$startstats = $checkstats($startstats, $firstfetchstats);

// Fetching the value should only hit the static cache.
$this->assertEquals($value, $cache->get('value'));
$checkstats($startstats, $secondfetchstats);
}


public function test_static_cache() {
global $CFG;
$this->resetAfterTest(true);
Expand Down
4 changes: 4 additions & 0 deletions cache/upgrade.txt
View file
@@ -1,5 +1,9 @@
This files describes API changes in /cache/stores/* - cache store plugins.
Information provided here is intended especially for developers.

=== 4.3 ===
* 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.
Expand Down

0 comments on commit b1f0a24

Please sign in to comment.