From 89182546a0ae93a2aef79383bac902c77c7801f8 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Mon, 19 Aug 2013 10:00:44 +1200 Subject: [PATCH] MDL-41106 cache: tidied up several elements --- cache/classes/loaders.php | 95 +++++++++++++++++++++++++++--------- cache/stores/session/lib.php | 41 ++++++++++------ cache/tests/cache_test.php | 93 +++++++++++++++++++++++++++++++++-- cache/tests/fixtures/lib.php | 18 +++++++ 4 files changed, 208 insertions(+), 39 deletions(-) diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index 69949c0b859..6de94e299b3 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -271,7 +271,7 @@ class cache implements cache_loader { * In advanced cases an array may be useful such as in situations requiring the multi-key functionality. * @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 moodle_exception + * @throws coding_exception */ public function get($key, $strictness = IGNORE_MISSING) { // 1. Parse the key. @@ -329,7 +329,7 @@ class cache implements cache_loader { } // 5. Validate strictness. if ($strictness === MUST_EXIST && $result === false) { - throw new moodle_exception('Requested key did not exist in any cache stores and could not be loaded.'); + 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. if ($setaftervalidation) { @@ -363,7 +363,7 @@ class cache implements cache_loader { * @return array An array of key value pairs for the items that could be retrieved from the cache. * If MUST_EXIST was used and not all keys existed within the cache then an exception will be thrown. * Otherwise any key that did not exist will have a data value of false within the results. - * @throws moodle_exception + * @throws coding_exception */ public function get_many(array $keys, $strictness = IGNORE_MISSING) { @@ -420,7 +420,7 @@ class cache implements cache_loader { $missingkeys = array(); foreach ($result as $key => $value) { if ($value === false) { - $missingkeys[] = ($usingloader) ? $key : $parsedkeys[$key]; + $missingkeys[] = $parsedkeys[$key]; } } if (!empty($missingkeys)) { @@ -430,11 +430,9 @@ class cache implements cache_loader { $resultmissing = $this->datasource->load_many_for_cache($missingkeys); } foreach ($resultmissing as $key => $value) { - $pkey = ($usingloader) ? $key : $keysparsed[$key]; - $realkey = ($usingloader) ? $parsedkeys[$key] : $key; - $result[$pkey] = $value; + $result[$keysparsed[$key]] = $value; if ($value !== false) { - $this->set($realkey, $value); + $this->set($key, $value); } } unset($resultmissing); @@ -453,7 +451,7 @@ class cache implements cache_loader { if ($strictness === MUST_EXIST) { foreach ($keys as $key) { if (!array_key_exists($key, $fullresult)) { - throw new moodle_exception('Not all the requested keys existed within the cache stores.'); + throw new coding_exception('Not all the requested keys existed within the cache stores.'); } } } @@ -483,6 +481,11 @@ class cache implements cache_loader { if ($this->perfdebug) { cache_helper::record_cache_set($this->storetype, $this->definition->get_id()); } + if ($this->loader !== false) { + // 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. + $this->loader->set($key, $data); + } if (is_object($data) && $data instanceof cacheable_object) { $data = new cache_cached_object($data); } else if (!is_scalar($data)) { @@ -506,6 +509,7 @@ class cache implements cache_loader { * Removes references where required. * * @param stdClass|array $data + * @return mixed What ever was put in but without any references. */ protected function unref($data) { if ($this->definition->uses_simple_data()) { @@ -593,6 +597,11 @@ class cache implements cache_loader { * ... if they care that is. */ public function set_many(array $keyvaluearray) { + if ($this->loader !== false) { + // 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. + $this->loader->set_many($keyvaluearray); + } $data = array(); $simulatettl = $this->has_a_ttl() && !$this->store_supports_native_ttl(); $usepersistcache = $this->is_using_persist_cache(); @@ -1340,7 +1349,6 @@ class cache_application extends cache implements cache_loader_with_locking { * @param string|int $key The key for the data being requested. * @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 moodle_exception */ public function get($key, $strictness = IGNORE_MISSING) { if ($this->requirelockingread && $this->check_lock_state($key) === false) { @@ -1364,7 +1372,7 @@ class cache_application extends cache implements cache_loader_with_locking { * @return array An array of key value pairs for the items that could be retrieved from the cache. * If MUST_EXIST was used and not all keys existed within the cache then an exception will be thrown. * Otherwise any key that did not exist will have a data value of false within the results. - * @throws moodle_exception + * @throws coding_exception */ public function get_many(array $keys, $strictness = IGNORE_MISSING) { if ($this->requirelockingread) { @@ -1498,7 +1506,7 @@ class cache_session extends cache { /** * This is the key used to track last access. */ - const LASTACCESS = 'lastaccess'; + const LASTACCESS = '__lastaccess__'; /** * Override the cache::construct method. @@ -1599,7 +1607,7 @@ class cache_session extends cache { protected function parse_key($key) { $prefix = $this->get_key_prefix(); if ($key === self::LASTACCESS) { - return '__lastaccess__'.$prefix; + return $key.$prefix; } return $prefix.'_'.parent::parse_key($key); } @@ -1651,7 +1659,7 @@ class cache_session extends cache { * In advanced cases an array may be useful such as in situations requiring the multi-key functionality. * @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 moodle_exception + * @throws coding_exception */ public function get($key, $strictness = IGNORE_MISSING) { // Check the tracked user. @@ -1695,7 +1703,7 @@ class cache_session extends cache { } // 5. Validate strictness. if ($strictness === MUST_EXIST && $result === false) { - throw new moodle_exception('Requested key did not exist in any cache stores and could not be loaded.'); + throw new coding_exception('Requested key did not exist in any cache stores and could not be loaded.'); } // 6. Make sure we don't pass back anything that could be a reference. // We don't want people modifying the data in the cache. @@ -1728,6 +1736,12 @@ class cache_session extends cache { */ public function set($key, $data) { $this->check_tracked_user(); + $loader = $this->get_loader(); + if ($loader !== false) { + // 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. + $loader->set($key, $data); + } if ($this->perfdebug) { cache_helper::record_cache_set($this->storetype, $this->get_definition()->get_id()); } @@ -1780,7 +1794,7 @@ class cache_session extends cache { * @return array An array of key value pairs for the items that could be retrieved from the cache. * If MUST_EXIST was used and not all keys existed within the cache then an exception will be thrown. * Otherwise any key that did not exist will have a data value of false within the results. - * @throws moodle_exception + * @throws coding_exception */ public function get_many(array $keys, $strictness = IGNORE_MISSING) { $this->check_tracked_user(); @@ -1788,17 +1802,19 @@ class cache_session extends cache { $keymap = array(); foreach ($keys as $key) { $parsedkey = $this->parse_key($key); - $parsedkeys[] = $parsedkey; + $parsedkeys[$key] = $parsedkey; $keymap[$parsedkey] = $key; } $result = $this->get_store()->get_many($parsedkeys); - $mustexist = $strictness === MUST_EXIST; $return = array(); + $missingkeys = array(); + $hasmissingkeys = false; foreach ($result as $parsedkey => $value) { + $key = $keymap[$parsedkey]; if ($value instanceof cache_ttl_wrapper) { /* @var cache_ttl_wrapper $value */ if ($value->has_expired()) { - $this->get_store()->delete($parsedkey); + $this->delete($keymap[$parsedkey]); $value = false; } else { $value = $value->data; @@ -1808,11 +1824,40 @@ class cache_session extends cache { /* @var cache_cached_object $value */ $value = $value->restore_object(); } - if ($value === false && $mustexist) { - throw new moodle_exception('Not all the requested keys existed within the cache stores.'); + $return[$key] = $value; + if ($value === false) { + $hasmissingkeys = true; + $missingkeys[$parsedkey] = $key; + } + } + if ($hasmissingkeys) { + // We've got missing keys - we've got to check any loaders or data sources. + $loader = $this->get_loader(); + $datasource = $this->get_datasource(); + if ($loader !== false) { + foreach ($loader->get_many($missingkeys) as $key => $value) { + if ($value !== false) { + $return[$key] = $value; + unset($missingkeys[$parsedkeys[$key]]); + } + } + } + $hasmissingkeys = count($missingkeys) > 0; + if ($datasource !== false && $hasmissingkeys) { + // We're still missing keys but we've got a datasource. + foreach ($datasource->load_many_for_cache($missingkeys) as $key => $value) { + if ($value !== false) { + $return[$key] = $value; + unset($missingkeys[$parsedkeys[$key]]); + } + } + $hasmissingkeys = count($missingkeys) > 0; } - $return[$keymap[$parsedkey]] = $value; } + if ($hasmissingkeys && $strictness === MUST_EXIST) { + throw new coding_exception('Requested key did not exist in any cache stores and could not be loaded.'); + } + return $return; } @@ -1859,6 +1904,12 @@ class cache_session extends cache { */ public function set_many(array $keyvaluearray) { $this->check_tracked_user(); + $loader = $this->get_loader(); + if ($loader !== false) { + // 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. + $loader->set_many($keyvaluearray); + } $data = array(); $definitionid = $this->get_definition()->get_ttl(); $simulatettl = $this->has_a_ttl() && !$this->store_supports_native_ttl(); diff --git a/cache/stores/session/lib.php b/cache/stores/session/lib.php index 99f6c52e3e1..6171add3618 100644 --- a/cache/stores/session/lib.php +++ b/cache/stores/session/lib.php @@ -202,7 +202,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar */ public function initialise(cache_definition $definition) { $this->storeid = $definition->generate_definition_hash(); - $this->store = &self::register_store_id($definition->get_id()); + $this->store = &self::register_store_id($this->name.'-'.$definition->get_id()); $this->ttl = $definition->get_ttl(); $maxsize = $definition->get_maxsize(); if ($maxsize !== null) { @@ -300,7 +300,8 @@ class cachestore_session extends session_data_store implements cache_is_key_awar * * @param string $key The key to use. * @param mixed $data The data to set. - * @param bool $testmaxsize If set to true then we test the maxsize arg and reduce if required. + * @param bool $testmaxsize If set to true then we test the maxsize arg and reduce if required. If this is set to false you will + * need to perform these checks yourself. This allows for bulk set's to be performed and maxsize tests performed once. * @return bool True if the operation was a success false otherwise. */ public function set($key, $data, $testmaxsize = true) { @@ -429,7 +430,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar */ public function delete($key) { if (!isset($this->store[$key])) { - return false; + return true; } unset($this->store[$key]); if ($this->maxsize !== false) { @@ -445,15 +446,19 @@ class cachestore_session extends session_data_store implements cache_is_key_awar * @return int The number of items successfully deleted. */ public function delete_many(array $keys) { + // The number of items that have been successfully deleted. $count = 0; + // The number of items that have actually being removed (how much to decrement maxsize by). + $reduction = 0; foreach ($keys as $key) { if (isset($this->store[$key])) { - $count++; + $reduction++; } + $count++; unset($this->store[$key]); } if ($this->maxsize !== false) { - $this->storecount -= $count; + $this->storecount -= $reduction; } return $count; } @@ -535,26 +540,26 @@ class cachestore_session extends session_data_store implements cache_is_key_awar * @return int number of removed elements */ protected function check_ttl() { - if ($this->ttl == 0) { + if ($this->ttl === 0) { return 0; } $maxtime = cache::now() - $this->ttl; - $c = 0; + $count = 0; for ($value = reset($this->store); $value !== false; $value = next($this->store)) { if ($value[1] >= $maxtime) { - // We know that elements are sorted by ttl so no need to continue; + // We know that elements are sorted by ttl so no need to continue. break; } - $c++; + $count++; } - if ($c) { - // Remove first $c elements as they are expired. - $this->store = array_slice($this->store, $c, null, true); + if ($count) { + // Remove first $count elements as they are expired. + $this->store = array_slice($this->store, $count, null, true); if ($this->maxsize !== false) { - $this->storecount -= $c; + $this->storecount -= $count; } } - return $c; + return $count; } /** @@ -582,4 +587,12 @@ class cachestore_session extends session_data_store implements cache_is_key_awar } return $return; } + + /** + * This store supports native TTL handling. + * @return bool + */ + public function store_supports_native_ttl() { + return true; + } } diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index e7f4cdc20b0..23df737f0e8 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -1126,9 +1126,9 @@ class core_cache_testcase extends advanced_testcase { } /** - * Test that multiple loaders work ok. + * Test that multiple application loaders work ok. */ - public function test_multiple_loaders() { + public function test_multiple_application_loaders() { $instance = cache_config_phpunittest::instance(true); $instance->phpunit_add_file_store('phpunittest1'); $instance->phpunit_add_file_store('phpunittest2'); @@ -1173,6 +1173,93 @@ class core_cache_testcase extends advanced_testcase { $this->assertFalse($result['a']); $this->assertEquals('B', $result['b']); $this->assertFalse($result['c']); + + // Test non-recursive deletes. + $this->assertTrue($cache->set('test', 'test')); + $this->assertSame('test', $cache->get('test')); + $this->assertTrue($cache->delete('test', false)); + // We should still have it on a deeper loader. + $this->assertSame('test', $cache->get('test')); + // Test non-recusive with many functions. + $this->assertSame(3, $cache->set_many(array( + 'one' => 'one', + 'two' => 'two', + 'three' => 'three' + ))); + $this->assertSame('one', $cache->get('one')); + $this->assertSame(array('two' => 'two', 'three' => 'three'), $cache->get_many(array('two', 'three'))); + $this->assertSame(3, $cache->delete_many(array('one', 'two', 'three'), false)); + $this->assertSame('one', $cache->get('one')); + $this->assertSame(array('two' => 'two', 'three' => 'three'), $cache->get_many(array('two', 'three'))); + } + + /** + * Test that multiple application loaders work ok. + */ + public function test_multiple_session_loaders() { + /* @var cache_config_phpunittest $instance */ + $instance = cache_config_phpunittest::instance(true); + $instance->phpunit_add_session_store('phpunittest1'); + $instance->phpunit_add_session_store('phpunittest2'); + $instance->phpunit_add_definition('phpunit/multi_loader', array( + 'mode' => cache_store::MODE_SESSION, + 'component' => 'phpunit', + 'area' => 'multi_loader' + )); + $instance->phpunit_add_definition_mapping('phpunit/multi_loader', 'phpunittest1', 3); + $instance->phpunit_add_definition_mapping('phpunit/multi_loader', 'phpunittest2', 2); + + $cache = cache::make('phpunit', 'multi_loader'); + $this->assertInstanceOf('cache_session', $cache); + $this->assertFalse($cache->get('test')); + $this->assertTrue($cache->set('test', 'test')); + $this->assertEquals('test', $cache->get('test')); + $this->assertTrue($cache->delete('test')); + $this->assertFalse($cache->get('test')); + $this->assertTrue($cache->set('test', 'test')); + $this->assertTrue($cache->purge()); + $this->assertFalse($cache->get('test')); + + // Test the many commands. + $this->assertEquals(3, $cache->set_many(array('a' => 'A', 'b' => 'B', 'c' => 'C'))); + $result = $cache->get_many(array('a', 'b', 'c')); + $this->assertInternalType('array', $result); + $this->assertCount(3, $result); + $this->assertArrayHasKey('a', $result); + $this->assertArrayHasKey('b', $result); + $this->assertArrayHasKey('c', $result); + $this->assertEquals('A', $result['a']); + $this->assertEquals('B', $result['b']); + $this->assertEquals('C', $result['c']); + $this->assertEquals($result, $cache->get_many(array('a', 'b', 'c'))); + $this->assertEquals(2, $cache->delete_many(array('a', 'c'))); + $result = $cache->get_many(array('a', 'b', 'c')); + $this->assertInternalType('array', $result); + $this->assertCount(3, $result); + $this->assertArrayHasKey('a', $result); + $this->assertArrayHasKey('b', $result); + $this->assertArrayHasKey('c', $result); + $this->assertFalse($result['a']); + $this->assertEquals('B', $result['b']); + $this->assertFalse($result['c']); + + // Test non-recursive deletes. + $this->assertTrue($cache->set('test', 'test')); + $this->assertSame('test', $cache->get('test')); + $this->assertTrue($cache->delete('test', false)); + // We should still have it on a deeper loader. + $this->assertSame('test', $cache->get('test')); + // Test non-recusive with many functions. + $this->assertSame(3, $cache->set_many(array( + 'one' => 'one', + 'two' => 'two', + 'three' => 'three' + ))); + $this->assertSame('one', $cache->get('one')); + $this->assertSame(array('two' => 'two', 'three' => 'three'), $cache->get_many(array('two', 'three'))); + $this->assertSame(3, $cache->delete_many(array('one', 'two', 'three'), false)); + $this->assertSame('one', $cache->get('one')); + $this->assertSame(array('two' => 'two', 'three' => 'three'), $cache->get_many(array('two', 'three'))); } /** @@ -1448,4 +1535,4 @@ class core_cache_testcase extends advanced_testcase { $this->assertInstanceOf('cache_request', $cache); $this->assertArrayHasKey('cache_is_searchable', $cache->phpunit_get_store_implements()); } -} +} \ No newline at end of file diff --git a/cache/tests/fixtures/lib.php b/cache/tests/fixtures/lib.php index f03a66cc8e4..b6d73584220 100644 --- a/cache/tests/fixtures/lib.php +++ b/cache/tests/fixtures/lib.php @@ -94,6 +94,24 @@ class cache_config_phpunittest extends cache_config_writer { ); } + /** + * Forcefully adds a session store. + * + * @param string $name + */ + public function phpunit_add_session_store($name) { + $this->configstores[$name] = array( + 'name' => $name, + 'plugin' => 'session', + 'configuration' => array(), + 'features' => 14, + 'modes' => 2, + 'default' => true, + 'class' => 'cachestore_session', + 'lock' => 'cachelock_file_default', + ); + } + /** * Forcefully injects a definition => store mapping. * -- 2.43.0