Merge branch 'wip-MDL-36120-m24' of git://github.com/samhemelryk/moodle

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.

cache/classes/definition.php

@@ -39,6 +39,11 @@ defined('MOODLE_INTERNAL') || die();
  *          [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 @@ class cache_definition {
         $area = (string)$definition['area'];
 
         // Set the defaults.
+        $simplekeys = false;
         $simpledata = false;
         $requireidentifiers = array();
         $requiredataguarantee = false;
@@ -306,6 +318,9 @@ class cache_definition {
         $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 @@ class cache_definition {
         $cachedefinition->mode = $mode;
         $cachedefinition->component = $component;
         $cachedefinition->area = $area;
+        $cachedefinition->simplekeys = $simplekeys;
         $cachedefinition->simpledata = $simpledata;
         $cachedefinition->requireidentifiers = $requireidentifiers;
         $cachedefinition->requiredataguarantee = $requiredataguarantee;
@@ -515,6 +531,17 @@ class cache_definition {
         return $this->component;
     }
 
+    /**
+     * Returns true if this definition is using simple keys.
+     *
+     * Simple keys contain only a-zA-Z0-9_
+     *
+     * @return bool
+     */
+    public function uses_simple_keys() {
+        return $this->simplekeys;
+    }
+
     /**
      * Returns the identifiers that are being used for this definition.
      * @return array

cache/classes/factory.php

@@ -164,8 +164,6 @@ class cache_factory {
         $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;

cache/classes/helper.php

@@ -446,12 +446,18 @@ class cache_helper {
     }
 
     /**
-     * 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()) {
+            // We put the key first so that we can be sure the start of the key changes.
+            return (string)$key . '-' . $definition->generate_single_key_prefix();
+        }
+        $key = $definition->generate_single_key_prefix() . '-' . $key;
+        return sha1($key);
     }
 
     /**

cache/classes/loaders.php

@@ -794,12 +794,14 @@ class cache implements cache_loader {
      * @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);
     }
 
     /**

cache/stores/file/addinstanceform.php

@@ -52,6 +52,10 @@ class cachestore_file_addinstance_form extends cachestore_addinstance_form {
         $form->addHelpButton('autocreate', 'autocreate', 'cachestore_file');
         $form->disabledIf('autocreate', 'path', 'eq', '');
 
+        $form->addElement('checkbox', 'singledirectory', get_string('singledirectory', 'cachestore_file'));
+        $form->setType('singledirectory', PARAM_BOOL);
+        $form->addHelpButton('singledirectory', 'singledirectory', 'cachestore_file');
+
         $form->addElement('checkbox', 'prescan', get_string('prescan', 'cachestore_file'));
         $form->setType('prescan', PARAM_BOOL);
         $form->addHelpButton('prescan', 'prescan', 'cachestore_file');

cache/stores/file/lang/en/cachestore_file.php

@@ -35,3 +35,18 @@ $string['path_help'] = 'The directory that should be used to store files for thi
 $string['pluginname'] = 'File cache';
 $string['prescan'] = 'Prescan directory';
 $string['prescan_help'] = 'If enabled the directory is scanned when the cache is first used and requests for files are first checked against the scan data. This can help if you have a slow file system and are finding that file operations are causing you a bottle neck.';
+$string['singledirectory'] = 'Single directory store';
+$string['singledirectory_help'] = 'If enabled files (cached items) will be stored in a single directory rather than being broken up into multiple directories.<br />
+Enabling this will speed up file interactions but comes at the cost of increased risk of hitting file system limitations.<br />
+It is advisable to only turn this on if the following is true:<br />
+  - If you know the number of items in the cache is going to be small enough that it won\'t cause issues on the file system you are running with.<br />
+  - The data being cached is not expensive to generate. If it is then sticking with the default may still be the better option as it reduces the chance of issues.';
+
+/**
+ * This is is like the file store, but designed for siutations where:
+ *   - many more things are likely to be stored in the cache, so CRC hashing is
+ *     too likely to give collisions, and storing everything in a completely flat
+ *     directory structure is inadvisable.
+ *   - the things we are caching are more expensive to calculate, so the extra
+ *     time to computer a better hash is a worthwhile trade-off.
+ */

cache/stores/file/lib.php

@@ -57,6 +57,14 @@ class cachestore_file implements cache_store, cache_is_key_aware {
      */
     protected $prescan = false;
 
+    /**
+     * Set to true if we should store files within a single directory.
+     * By default we use a nested structure in order to reduce the chance of conflicts and avoid any file system
+     * limitations such as maximum files per directory.
+     * @var bool
+     */
+    protected $singledirectory = false;
+
     /**
      * Set to true when the path should be automatically created if it does not yet exist.
      * @var bool
@@ -122,7 +130,20 @@ class cachestore_file implements cache_store, cache_is_key_aware {
         }
         $this->isready = $path !== false;
         $this->path = $path;
-        $this->prescan = array_key_exists('prescan', $configuration) ? (bool)$configuration['prescan'] : false;
+        // Check if we should prescan the directory.
+        if (array_key_exists('prescan', $configuration)) {
+            $this->prescan = (bool)$configuration['prescan'];
+        } else {
+            // Default is no, we should not prescan.
+            $this->prescan = false;
+        }
+        // Check if we should be storing in a single directory.
+        if (array_key_exists('singledirectory', $configuration)) {
+            $this->singledirectory = (bool)$configuration['singledirectory'];
+        } else {
+            // Default: No, we will use multiple directories.
+            $this->singledirectory = false;
+        }
     }
 
     /**
@@ -226,10 +247,51 @@ class cachestore_file implements cache_store, cache_is_key_aware {
             $this->prescan = false;
         }
         if ($this->prescan) {
-            $pattern = $this->path.'/*.cache';
-            foreach (glob($pattern, GLOB_MARK | GLOB_NOSORT) as $filename) {
-                $this->keys[basename($filename)] = filemtime($filename);
+            $this->prescan_keys();
+        }
+    }
+
+    /**
+     * Pre-scan the cache to see which keys are present.
+     */
+    protected function prescan_keys() {
+        foreach (glob($this->glob_keys_pattern(), GLOB_MARK | GLOB_NOSORT) as $filename) {
+            $this->keys[basename($filename)] = filemtime($filename);
+        }
+    }
+
+    /**
+     * Gets a pattern suitable for use with glob to find all keys in the cache.
+     * @return string The pattern.
+     */
+    protected function glob_keys_pattern() {
+        if ($this->singledirectory) {
+            return $this->path . '/*.cache';
+        } else {
+            return $this->path . '/*/*.cache';
+        }
+    }
+
+    /**
+     * Returns the file path to use for the given key.
+     *
+     * @param string $key The key to generate a file path for.
+     * @param bool $create If set to the true the directory structure the key requires will be created.
+     * @return string The full path to the file that stores a particular cache key.
+     */
+    protected function file_path_for_key($key, $create = false) {
+        if ($this->singledirectory) {
+            // Its a single directory, easy, just the store instances path + the file name.
+            return $this->path . '/' . $key . '.cache';
+        } else {
+            // We are using a single subdirectory to achieve 1 level.
+            $subdir = substr($key, 0, 3);
+            $dir = $this->path . '/' . $subdir;
+            if ($create) {
+                // Create the directory. This function does it recursivily!
+                make_writable_directory($dir);
             }
+            return $dir . '/' . $key . '.cache';
         }
     }
 
@@ -241,7 +303,7 @@ class cachestore_file implements cache_store, cache_is_key_aware {
      */
     public function get($key) {
         $filename = $key.'.cache';
-        $file = $this->path.'/'.$filename;
+        $file = $this->file_path_for_key($key);
         $ttl = $this->definition->get_ttl();
         if ($ttl) {
             $maxtime = cache::now() - $ttl;
@@ -307,7 +369,7 @@ class cachestore_file implements cache_store, cache_is_key_aware {
      */
     public function delete($key) {
         $filename = $key.'.cache';
-        $file = $this->path.'/'.$filename;
+        $file = $this->file_path_for_key($key);
         $result = @unlink($file);
         unset($this->keys[$filename]);
         return $result;
@@ -339,7 +401,7 @@ class cachestore_file implements cache_store, cache_is_key_aware {
     public function set($key, $data) {
         $this->ensure_path_exists();
         $filename = $key.'.cache';
-        $file = $this->path.'/'.$filename;
+        $file = $this->file_path_for_key($key, true);
         $result = $this->write_file($file, $this->prep_data_before_save($data));
         if (!$result) {
             // Couldn't write the file.
@@ -404,11 +466,11 @@ class cachestore_file implements cache_store, cache_is_key_aware {
      */
     public function has($key) {
         $filename = $key.'.cache';
-        $file = $this->path.'/'.$key.'.cache';
         $maxtime = cache::now() - $this->definition->get_ttl();
         if ($this->prescan) {
             return array_key_exists($filename, $this->keys) && $this->keys[$filename] >= $maxtime;
         }
+        $file = $this->file_path_for_key($key);
         return (file_exists($file) && ($this->definition->get_ttl() == 0 || filemtime($file) >= $maxtime));
     }
 
@@ -448,8 +510,7 @@ class cachestore_file implements cache_store, cache_is_key_aware {
      * @return boolean True on success. False otherwise.
      */
     public function purge() {
-        $pattern = $this->path.'/*.cache';
-        foreach (glob($pattern, GLOB_MARK | GLOB_NOSORT) as $filename) {
+        foreach (glob($this->glob_keys_pattern(), GLOB_MARK | GLOB_NOSORT) as $filename) {
             @unlink($filename);
         }
         $this->keys = array();
@@ -471,6 +532,9 @@ class cachestore_file implements cache_store, cache_is_key_aware {
         if (isset($data->autocreate)) {
             $config['autocreate'] = $data->autocreate;
         }
+        if (isset($data->singledirectory)) {
+            $config['singledirectory'] = $data->singledirectory;
+        }
         if (isset($data->prescan)) {
             $config['prescan'] = $data->prescan;
         }

cache/tests/cache_test.php

@@ -364,6 +364,50 @@ class cache_phpunit_tests extends advanced_testcase {
         $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'));
+
+        $cache->purge();
+
+        $this->assertTrue($cache->set('1', 'test data 1'));
+        $this->assertEquals('test data 1', $cache->get('1'));
+        $this->assertTrue($cache->set('2', 'test data 2'));
+        $this->assertEquals('test data 2', $cache->get('2'));
+    }
+
     public function test_definition_ttl() {
         $instance = cache_config_phpunittest::instance(true);
         $instance->phpunit_add_definition('phpunit/ttltest', array(
@@ -515,13 +559,15 @@ class cache_phpunit_tests extends advanced_testcase {
 
         // 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/a65/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/626/626e9c7a45febd98f064c2b383de8d9d4ebbde7b.cache';
         $datacont = serialize("test data 1");
+        make_writable_directory(dirname($datafile));
         file_put_contents($datafile, $datacont);
         $this->assertTrue(file_exists($datafile));
 

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 @@ $definitions = array(
     ),
 
     // 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 @@ $definitions = array(
     // 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',