From 47834bcd41e791729fbcf26dd225a5dc61a53b58 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Fri, 2 Nov 2012 14:43:26 +1300 Subject: [PATCH] MDL-36120 cache: now uses sha1 and has an option for simple keys --- cache/README.md | 2 ++ cache/classes/definition.php | 24 ++++++++++++++++++++ cache/classes/factory.php | 2 -- cache/classes/helper.php | 13 +++++++---- cache/classes/loaders.php | 4 +++- cache/tests/cache_test.php | 43 ++++++++++++++++++++++++++++++++++-- lib/db/caches.php | 17 ++++++++++++++ version.php | 2 +- 8 files changed, 97 insertions(+), 10 deletions(-) diff --git a/cache/README.md b/cache/README.md index 40a723d582542..408e014a9f61a 100644 --- a/cache/README.md +++ b/cache/README.md @@ -9,6 +9,7 @@ A definition: $definitions = array( 'string' => array( // Required, unique to the component 'mode' => cache_store::MODE_APPLICATION, // Required + 'simplekeys' => false, // Optional 'simpledata' => false, // Optional 'requireidentifiers' => array( // Optional 'lang' @@ -105,6 +106,7 @@ The following settings are required for a definition: * mode - Application, session or request. The following optional settings can also be defined: +* simplekeys - Set to true if items will always and only have simple keys. Simple keys may contain a-zA-Z0-9_. If set to true we use the keys as they are without hashing them. Good for performance and possible because we know the keys are safe. * simpledata - Set to true if you know that you will only be storing scalar values or arrays of scalar values. Avoids costly investigation of data types. * requireidentifiers - Any identifiers the definition requires. Must be provided when creating the loader. * requiredataguarantee - If set to true then only stores that support data guarantee will be used. diff --git a/cache/classes/definition.php b/cache/classes/definition.php index dcc219d805a67..35494d6429936 100644 --- a/cache/classes/definition.php +++ b/cache/classes/definition.php @@ -39,6 +39,11 @@ * [int] Sets the mode for the definition. Must be one of cache_store::MODE_* * * Optional settings: + * + simplekeys + * [bool] Set to true if your cache will only use simple keys for its items. + * Simple keys consist of digits, underscores and the 26 chars of the english language. a-zA-Z0-9_ + * If true the keys won't be hashed before being passed to the cache store for gets/sets/deletes. It will be + * better for performance and possible only becase we know the keys are safe. * + simpledata * [bool] If set to true we know that the data is scalar or array of scalar. * + requireidentifiers @@ -129,6 +134,12 @@ class cache_definition { */ protected $area; + /** + * If set to true we know the keys are simple. a-zA-Z0-9_ + * @var bool + */ + protected $simplekeys = false; + /** * Set to true if we know the data is scalar or array of scalar. * @var bool @@ -289,6 +300,7 @@ public static function load($id, array $definition, $datasourceaggregate = null) $area = (string)$definition['area']; // Set the defaults. + $simplekeys = false; $simpledata = false; $requireidentifiers = array(); $requiredataguarantee = false; @@ -306,6 +318,9 @@ public static function load($id, array $definition, $datasourceaggregate = null) $mappingsonly = false; $invalidationevents = array(); + if (array_key_exists('simplekeys', $definition)) { + $simplekeys = (bool)$definition['simplekeys']; + } if (array_key_exists('simpledata', $definition)) { $simpledata = (bool)$definition['simpledata']; } @@ -410,6 +425,7 @@ public static function load($id, array $definition, $datasourceaggregate = null) $cachedefinition->mode = $mode; $cachedefinition->component = $component; $cachedefinition->area = $area; + $cachedefinition->simplekeys = $simplekeys; $cachedefinition->simpledata = $simpledata; $cachedefinition->requireidentifiers = $requireidentifiers; $cachedefinition->requiredataguarantee = $requiredataguarantee; @@ -515,6 +531,14 @@ public function get_component() { return $this->component; } + /** + * Returns the PARAM type that best describes the expected keys. + * @return bool + */ + public function uses_simple_keys() { + return $this->simplekeys; + } + /** * Returns the identifiers that are being used for this definition. * @return array diff --git a/cache/classes/factory.php b/cache/classes/factory.php index 729a46bf6c8a4..ddf4e8da871db 100644 --- a/cache/classes/factory.php +++ b/cache/classes/factory.php @@ -164,8 +164,6 @@ public function create_cache_from_params($mode, $component, $area, array $identi $definition->set_identifiers($identifiers); $cache = $this->create_cache($definition, $identifiers); if ($definition->should_be_persistent()) { - $cache->persist = true; - $cache->persistcache = array(); $this->cachesfromparams[$key] = $cache; } return $cache; diff --git a/cache/classes/helper.php b/cache/classes/helper.php index dfb3346bcfc09..d8a58a4229301 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -446,11 +446,16 @@ public static function get_definition_name($definition) { } /** - * Hashes a descriptive key to make it shorter and stil unique. - * @param string $key + * Hashes a descriptive key to make it shorter and still unique. + * @param string|int $key + * @param cache_definition $definition * @return string */ - public static function hash_key($key) { - return crc32($key); + public static function hash_key($key, cache_definition $definition) { + if ($definition->uses_simple_keys()) { + return $definition->generate_single_key_prefix() . '-' . (string)$key; + } + $key = $definition->generate_single_key_prefix() . '-' . $key; + return sha1($key); } } \ No newline at end of file diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index ef7b308d11b54..d0b3a4de1e524 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -794,12 +794,14 @@ public function purge() { * @return string|array String unless the store supports multi-identifiers in which case an array if returned. */ protected function parse_key($key) { + // First up if the store supports multiple keys we'll go with that. if ($this->store->supports_multiple_indentifiers()) { $result = $this->definition->generate_multi_key_parts(); $result['key'] = $key; return $result; } - return cache_helper::hash_key($this->definition->generate_single_key_prefix().'-'.$key); + // If not we need to generate a hash and to for that we use the cache_helper. + return cache_helper::hash_key($key, $this->definition); } /** diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index 324d361d6b3cb..eb85dc95f758e 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -364,6 +364,43 @@ public function test_definition_overridden_loader() { $this->assertEquals('Test has no value really.', $cache->get('Test')); } + /** + * Test a very basic definition. + */ + public function test_definition() { + $instance = cache_config_phpunittest::instance(); + $instance->phpunit_add_definition('phpunit/test', array( + 'mode' => cache_store::MODE_APPLICATION, + 'component' => 'phpunit', + 'area' => 'test', + )); + $cache = cache::make('phpunit', 'test'); + + $this->assertTrue($cache->set('testkey1', 'test data 1')); + $this->assertEquals('test data 1', $cache->get('testkey1')); + $this->assertTrue($cache->set('testkey2', 'test data 2')); + $this->assertEquals('test data 2', $cache->get('testkey2')); + } + + /** + * Test a definition using the simple keys. + */ + public function test_definition_simplekeys() { + $instance = cache_config_phpunittest::instance(); + $instance->phpunit_add_definition('phpunit/simplekeytest', array( + 'mode' => cache_store::MODE_APPLICATION, + 'component' => 'phpunit', + 'area' => 'simplekeytest', + 'simplekeys' => true + )); + $cache = cache::make('phpunit', 'simplekeytest'); + + $this->assertTrue($cache->set('testkey1', 'test data 1')); + $this->assertEquals('test data 1', $cache->get('testkey1')); + $this->assertTrue($cache->set('testkey2', 'test data 2')); + $this->assertEquals('test data 2', $cache->get('testkey2')); + } + public function test_definition_ttl() { $instance = cache_config_phpunittest::instance(true); $instance->phpunit_add_definition('phpunit/ttltest', array( @@ -515,13 +552,15 @@ public function test_distributed_application_event_invalidation() { // OK data added, data invalidated, and invalidation time has been set. // Now we need to manually add back the data and adjust the invalidation time. - $timefile = $CFG->dataroot.'/cache/cachestore_file/default_application/phpunit_eventinvalidationtest/494515064.cache'; + $timefile = $CFG->dataroot.'/cache/cachestore_file//default_application/phpunit_eventinvalidationtest/a6/5b/a65b1dc524cf6e03c1795197c84d5231eb229b86.cache'; $timecont = serialize(cache::now() - 60); // Back 60sec in the past to force it to re-invalidate. + make_writable_directory(dirname($timefile)); file_put_contents($timefile, $timecont); $this->assertTrue(file_exists($timefile)); - $datafile = $CFG->dataroot.'/cache/cachestore_file/default_application/phpunit_eventinvalidationtest/3140056538.cache'; + $datafile = $CFG->dataroot.'/cache/cachestore_file/default_application/phpunit_eventinvalidationtest/62/6e/626e9c7a45febd98f064c2b383de8d9d4ebbde7b.cache'; $datacont = serialize("test data 1"); + make_writable_directory(dirname($datafile)); file_put_contents($datafile, $datacont); $this->assertTrue(file_exists($datafile)); diff --git a/lib/db/caches.php b/lib/db/caches.php index acf9b0eab606d..5d44fd1241ec4 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -29,14 +29,19 @@ $definitions = array( // Used to store processed lang files. + // The keys used are the component of the string file. 'string' => array( 'mode' => cache_store::MODE_APPLICATION, + 'simplekeys' => true, 'simpledata' => true, 'persistent' => true, 'persistentmaxsize' => 3 ), // Used to store database meta information. + // The database meta information includes information about tables and there columns. + // Its keys are the table names. + // When creating an instance of this definition you must provide the database family that is being used. 'databasemeta' => array( 'mode' => cache_store::MODE_APPLICATION, 'requireidentifiers' => array( @@ -47,13 +52,24 @@ ), // Used to store data from the config + config_plugins table in the database. + // The key used is the component: + // - core for all core config settings + // - plugin component for all plugin settings. + // Persistence is used because normally several settings within a script. 'config' => array( 'mode' => cache_store::MODE_APPLICATION, 'persistent' => true, + 'simplekeys' => true, 'simpledata' => true ), // Event invalidation cache. + // This cache is used to manage event invalidation, its keys are the event names. + // Whenever something is invalidated it is both purged immediately and an event record created with the timestamp. + // When a new cache is initialised all timestamps are looked at and if past data is once more invalidated. + // Data guarantee is required in order to ensure invalidation always occurs. + // Persistence has been turned on as normally events are used for frequently used caches and this event invalidation + // cache will likely be used either lots or never. 'eventinvalidation' => array( 'mode' => cache_store::MODE_APPLICATION, 'persistent' => true, @@ -66,6 +82,7 @@ // question_bank::load_question. 'questiondata' => array( 'mode' => cache_store::MODE_APPLICATION, + 'simplekeys' => true, // The id of the question is used. 'requiredataguarantee' => false, 'datasource' => 'question_finder', 'datasourcefile' => 'question/engine/bank.php', diff --git a/version.php b/version.php index 0812207e388fc..b6cffad1e958a 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2012110600.01; // YYYYMMDD = weekly release date of this DEV branch +$version = 2012110700.00; // YYYYMMDD = weekly release date of this DEV branch // RR = release increments - 00 in DEV branches // .XX = incremental changes