Skip to content

Commit

Permalink
MDL-55604 cache: Fix identifier and cacheable_object
Browse files Browse the repository at this point in the history 
When a cachable object is store in the static cache from
the backing store, it was incorrect serialised rather than
using the wake function.  This has been resolved and tests added.

During the investigation into cacheable_object, it was discovered
that set_identifiers never removes identifiers when you call it,
so set_identifiers(array('a')) and set_identifiers(array('b')) really
resulted in array('a','b') as the identifiers rather than 'b'.

The fix for this issue depends on the set_identifiers fix and
they have been coupled together as a result.
  • Loading branch information
@mr-russ
mr-russ committed Aug 24, 2016
1 parent 3ca3cc7 commit 7ff43e1
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 20 deletions.
6 changes: 2 additions & 4 deletions cache/classes/definition.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -808,10 +808,8 @@ public function set_identifiers(array $identifiers = array()) {
}
}

if ($this->identifiers === null) {
// Initialize identifiers if they have not been.
$this->identifiers = array();
}
$this->identifiers = array();

foreach ($identifiers as $name => $value) {
$this->identifiers[$name] = (string)$value;
}
Expand Down
29 changes: 18 additions & 11 deletions cache/classes/loaders.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,12 @@ public function get($key, $strictness = IGNORE_MISSING) {
$result = $result->data;
}
}
if ($result instanceof cache_cached_object) {
$result = $result->restore_object();
}
if ($usesstaticacceleration) {
$this->static_acceleration_set($key, $result);
}
if ($result instanceof cache_cached_object) {
$result = $result->restore_object();
}
}

// 4. Load if from the loader/datasource if we don't already have it.
Expand Down Expand Up @@ -410,12 +410,12 @@ public function get_many(array $keys, $strictness = IGNORE_MISSING) {
$value = $value->data;
}
}
if ($value instanceof cache_cached_object) {
$value = $value->restore_object();
}
if ($value !== false && $this->use_static_acceleration()) {
$this->static_acceleration_set($keystofind[$key], $value);
}
if ($value instanceof cache_cached_object) {
$value = $value->restore_object();
}
$resultstore[$key] = $value;
}
}
Expand Down Expand Up @@ -836,11 +836,7 @@ public function delete_many(array $keys, $recurse = true) {
*/
public function purge() {
// 1. Purge the static acceleration array.
$this->staticaccelerationarray = array();
if ($this->staticaccelerationsize !== false) {
$this->staticaccelerationkeys = array();
$this->staticaccelerationcount = 0;
}
$this->static_acceleration_purge();
// 2. Purge the store.
$this->store->purge();
// 3. Optionally pruge any stacked loaders.
Expand Down Expand Up @@ -1111,6 +1107,17 @@ protected function static_acceleration_delete($key) {
return true;
}

/**
* Purge the static acceleration cache.
*/
protected function static_acceleration_purge() {
$this->staticaccelerationarray = array();
if ($this->staticaccelerationsize !== false) {
$this->staticaccelerationkeys = array();
$this->staticaccelerationcount = 0;
}
}

/**
* Returns the timestamp from the first request for the time from the cache API.
*
Expand Down
74 changes: 72 additions & 2 deletions cache/tests/cache_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,35 @@ public function test_windows_nasty_keys() {
$this->assertEquals('test data 1', $cache->get('contest'));
}

/**
* Tests set_identifiers resets identifiers and static cache
*/
public function test_set_identifiers() {
$instance = cache_config_testing::instance();
$instance->phpunit_add_definition('phpunit/identifier', array(
'mode' => cache_store::MODE_APPLICATION,
'component' => 'phpunit',
'area' => 'identifier',
'simplekeys' => true,
'simpledata' => true,
'staticacceleration' => true
));
$cache = cache::make('phpunit', 'identifier', array('area'));
$this->assertTrue($cache->set('contest', 'test data 1'));
$this->assertEquals('test data 1', $cache->get('contest'));

$cache->set_identifiers(array());
$this->assertFalse($cache->get('contest'));
$this->assertTrue($cache->set('contest', 'empty ident'));
$this->assertEquals('empty ident', $cache->get('contest'));

$cache->set_identifiers(array('area'));
$this->assertEquals('test data 1', $cache->get('contest'));

$cache->set_identifiers(array());
$this->assertEquals('empty ident', $cache->get('contest'));
}

/**
* Tests the default application cache
*/
Expand Down Expand Up @@ -306,15 +335,17 @@ protected function run_on_cache(cache_loader $cache) {
$this->assertTrue($cache->set($key, $dataobject));
$this->assertEquals($dataobject, $cache->get($key));

$specobject = new cache_phpunit_dummy_object('red', 'blue');
$starttime = microtime(true);
$specobject = new cache_phpunit_dummy_object('red', 'blue', $starttime);
$this->assertTrue($cache->set($key, $specobject));
$result = $cache->get($key);
$this->assertInstanceOf('cache_phpunit_dummy_object', $result);
$this->assertEquals('red_ptc_wfc', $result->property1);
$this->assertEquals('blue_ptc_wfc', $result->property2);
$this->assertGreaterThan($starttime, $result->propertytime);

// Test array of objects.
$specobject = new cache_phpunit_dummy_object('red', 'blue');
$specobject = new cache_phpunit_dummy_object('red', 'blue', $starttime);
$data = new cacheable_object_array(array(
clone($specobject),
clone($specobject),
Expand All @@ -328,6 +359,8 @@ protected function run_on_cache(cache_loader $cache) {
$this->assertInstanceOf('cache_phpunit_dummy_object', $item);
$this->assertEquals('red_ptc_wfc', $item->property1);
$this->assertEquals('blue_ptc_wfc', $item->property2);
// Ensure that wake from cache is called in all cases.
$this->assertGreaterThan($starttime, $item->propertytime);
}

// Test set many.
Expand Down Expand Up @@ -1847,6 +1880,43 @@ public function test_static_acceleration() {
$this->assertEquals('D', $cache->phpunit_static_acceleration_get('d'));
$this->assertEquals('E', $cache->phpunit_static_acceleration_get('e'));

// Store a cacheable_object, get many times and ensure each time wake_for_cache is used.
// Both get and get_many are tested. Two cache entries are used to ensure the times aren't
// confused with multiple calls to get()/get_many().
$startmicrotime = microtime(true);
$cacheableobject = new cache_phpunit_dummy_object(1, 1, $startmicrotime);
$cacheableobject2 = new cache_phpunit_dummy_object(2, 2, $startmicrotime);
$this->assertTrue($cache->set('a', $cacheableobject));
$this->assertTrue($cache->set('b', $cacheableobject2));
$staticaccelerationreturntime = $cache->phpunit_static_acceleration_get('a')->propertytime;
$staticaccelerationreturntimeb = $cache->phpunit_static_acceleration_get('b')->propertytime;
$this->assertGreaterThan($startmicrotime, $staticaccelerationreturntime, 'Restore time of static must be newer.');

// Use set_identifiers to reset the static cache without resetting backing store.
$cache->phpunit_static_acceleration_purge();

// Get the value from the backend store, populating the static cache.
$cachevalue = $cache->get('a');
$this->assertInstanceOf('cache_phpunit_dummy_object', $cachevalue);
$this->assertGreaterThan($staticaccelerationreturntime, $cachevalue->propertytime);
$backingstorereturntime = $cachevalue->propertytime;

$results = $cache->get_many(array('b'));
$this->assertInstanceOf('cache_phpunit_dummy_object', $results['b']);
$this->assertGreaterThan($staticaccelerationreturntimeb, $results['b']->propertytime);
$backingstorereturntimeb = $results['b']->propertytime;

// Obtain the value again and confirm that static cache is using wake_from_cache.
// Upon failure, the times are not adjusted as wake_from_cache is skipped as the
// value is stored serialized in the static acceleration cache.
$cachevalue = $cache->phpunit_static_acceleration_get('a');
$this->assertInstanceOf('cache_phpunit_dummy_object', $cachevalue);
$this->assertGreaterThan($backingstorereturntime, $cachevalue->propertytime);

$results = $cache->get_many(array('b'));
$this->assertInstanceOf('cache_phpunit_dummy_object', $results['b']);
$this->assertGreaterThan($backingstorereturntimeb, $results['b']->propertytime);

/** @var cache_phpunit_application $cache */
$cache = cache::make('phpunit', 'accelerated2');
$this->assertInstanceOf('cache_phpunit_application', $cache);
Expand Down
34 changes: 31 additions & 3 deletions cache/tests/fixtures/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,9 @@ class cache_config_phpunittest extends cache_config_testing {
/**
* Dummy object for testing cacheable object interface and interaction
*
* Wake from cache needs specific testing at times to ensure that during multiple
* cache get() requests it's possible to verify that it's getting woken each time.
*
* @copyright 2012 Sam Hemelryk
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
Expand All @@ -323,29 +326,43 @@ class cache_phpunit_dummy_object extends stdClass implements cacheable_object {
* @var string
*/
public $property2;
/**
* Test property time for verifying wake is run at each get() call.
* @var float
*/
public $propertytime;
/**
* Constructor
* @param string $property1
* @param string $property2
*/
public function __construct($property1, $property2) {
public function __construct($property1, $property2, $propertytime = null) {
$this->property1 = $property1;
$this->property2 = $property2;
$this->propertytime = $propertytime === null ? microtime(true) : $propertytime;
}
/**
* Prepares this object for caching
* @return array
*/
public function prepare_to_cache() {
return array($this->property1.'_ptc', $this->property2.'_ptc');
return array($this->property1.'_ptc', $this->property2.'_ptc', $this->propertytime);
}
/**
* Returns this object from the cache
* @param array $data
* @return cache_phpunit_dummy_object
*/
public static function wake_from_cache($data) {
return new cache_phpunit_dummy_object(array_shift($data).'_wfc', array_shift($data).'_wfc');
$time = null;
if (!is_null($data[2])) {
// Windows 32bit microtime() resolution is 15ms, we ensure the time has moved forward.
do {
$time = microtime(true);
} while ($time == $data[2]);

}
return new cache_phpunit_dummy_object(array_shift($data).'_wfc', array_shift($data).'_wfc', $time);
}
}

Expand Down Expand Up @@ -426,6 +443,17 @@ public function phpunit_get_store_implements() {
public function phpunit_static_acceleration_get($key) {
return $this->static_acceleration_get($key);
}

/**
* Purges only the static acceleration while leaving the rest of the store in tack.
*
* Used for behaving like you have loaded 2 pages, and reset static while the backing store
* still contains all the same data.
*
*/
public function phpunit_static_acceleration_purge() {
$this->static_acceleration_purge();
}
}

/**
Expand Down

0 comments on commit 7ff43e1

Please sign in to comment.