MDL-36466 cache: tidy up after review
authorSam Hemelryk <sam@moodle.com>
Wed, 21 Nov 2012 21:33:57 +0000 (10:33 +1300)
committerSam Hemelryk <sam@moodle.com>
Sun, 25 Nov 2012 19:12:00 +0000 (08:12 +1300)
admin/cli/install.php
admin/index.php
cache/README.md
cache/classes/factory.php
cache/disabledlib.php
cache/tests/cache_test.php
cache/tests/fixtures/lib.php
install.php
lib/setup.php
lib/upgradelib.php

index b662d4c..ca8aaef 100644 (file)
@@ -127,6 +127,9 @@ if (function_exists('date_default_timezone_set') and function_exists('date_defau
 /** Used by library scripts to check they are being called by Moodle */
 define('MOODLE_INTERNAL', true);
 
+// Disables caching.. just in case.
+define('CACHE_DISABLE_ALL', true);
+
 // Check that PHP is of a sufficient version
 if (version_compare(phpversion(), "5.3.2") < 0) {
     $phpversion = phpversion();
@@ -169,9 +172,6 @@ require_once($CFG->dirroot.'/cache/lib.php');
 require($CFG->dirroot.'/version.php');
 $CFG->target_release = $release;
 
-// Disable the cache API.
-cache_factory::disable();
-
 //Database types
 $databases = array('mysqli' => moodle_database::get_driver_instance('mysqli', 'native'),
                    'pgsql'  => moodle_database::get_driver_instance('pgsql',  'native'),
index 6ce81f9..d75e364 100644 (file)
@@ -105,9 +105,6 @@ if (!$version or !$release) {
 }
 
 if (!core_tables_exist()) {
-    // Disable the Cache API as much as possible for installation.
-    cache_factory::disable();
-
     $PAGE->set_pagelayout('maintenance');
     $PAGE->set_popup_notification_allowed(false);
 
@@ -197,9 +194,6 @@ if (empty($CFG->version)) {
 if ($version > $CFG->version) {  // upgrade
     purge_all_caches();
 
-    // Disable the Cache API as much as possible for upgrade.
-    cache_factory::disable();
-
     $PAGE->set_pagelayout('maintenance');
     $PAGE->set_popup_notification_allowed(false);
 
@@ -304,8 +298,6 @@ if ($branch <> $CFG->branch) {  // Update the branch
 }
 
 if (moodle_needs_upgrading()) {
-    // Disable the Cache API as much as possible for upgrade.
-    cache_factory::disable();
     if (!$PAGE->headerprinted) {
         // means core upgrade or installation was not already done
         if (!$confirmplugins) {
index ef8c82a..981e043 100644 (file)
@@ -72,10 +72,10 @@ Disabling the cache entirely.
 Like above there are times when you want the cache to avoid initialising anything it doesn't absolutely need. Things such as installation and upgrade require this functionality.
 When the cache API is disabled it is still functional however special "disabled" classes will be used instead of the regular classes that make the Cache API tick.
 These disabled classes do the least work possible and through this means we avoid all manner of intialisation and configuration.
-Once disabled its not recommened to reneable the Cache API. Instead if you need it, redirect.
+Once disabled it cannot be re-enabled.
 
     // To disable the cache entirely call the following:
-    cache_factory::disable();
+    define('CACHE_DISABLE_ALL', true);
 
 Cache API parts
 ---------------
index 3ea7a5b..1458cf0 100644 (file)
@@ -110,11 +110,21 @@ class cache_factory {
      * @return cache_factory
      */
     public static function instance($forcereload = false) {
+        global $CFG;
         if ($forcereload || self::$instance === null) {
-            self::$instance = new cache_factory();
-            if (defined('CACHE_DISABLE_STORES') && CACHE_DISABLE_STORES !== false) {
-                // The cache stores have been disabled.
-                self::$instance->set_state(self::STATE_STORES_DISABLED);;
+            // Initialise a new factory to facilitate our needs.
+            if (defined('CACHE_DISABLE_ALL') && CACHE_DISABLE_ALL !== false) {
+                // The cache has been disabled. Load disabledlib and start using the factory designed to handle this
+                // situation. It will use disabled alternatives where available.
+                require_once($CFG->dirroot.'/cache/disabledlib.php');
+                self::$instance = new cache_factory_disabled();
+            } else {
+                // We're using the regular factory.
+                self::$instance = new cache_factory();
+                if (defined('CACHE_DISABLE_STORES') && CACHE_DISABLE_STORES !== false) {
+                    // The cache stores have been disabled.
+                    self::$instance->set_state(self::STATE_STORES_DISABLED);;
+                }
             }
         }
         return self::$instance;
@@ -420,11 +430,14 @@ class cache_factory {
      * In switching out the factory for the disabled factory it gains full control over the initialisation of objects
      * and can use all of the disabled alternatives.
      * Simple!
+     *
+     * This function has been marked as protected so that it cannot be abused through the public API presently.
+     * Perhaps in the future we will allow this, however as per the build up to the first release containing
+     * MUC it was decided that this was just to risky and abusable.
      */
-    public static function disable() {
+    protected static function disable() {
         global $CFG;
         require_once($CFG->dirroot.'/cache/disabledlib.php');
-        self::$instance = null;
         self::$instance = new cache_factory_disabled();
     }
 
index ea70329..18cc11e 100644 (file)
@@ -225,10 +225,13 @@ class cache_factory_disabled extends cache_factory {
      * @param string $component
      * @param string $area
      * @param array $identifiers
-     * @param bool $persistent Unused.
+     * @param array $options An array of options, available options are:
+     *   - simplekeys : Set to true if the keys you will use are a-zA-Z0-9_
+     *   - simpledata : Set to true if the type of the data you are going to store is scalar, or an array of scalar vars
+     *   - persistent : If set to true the cache will persist construction requests.
      * @return cache_application|cache_session|cache_request
      */
-    public function create_cache_from_params($mode, $component, $area, array $identifiers = array(), $persistent = false) {
+    public function create_cache_from_params($mode, $component, $area, array $identifiers = array(), array $options = array()) {
         $definition = cache_definition::load_adhoc($mode, $component, $area);
         $cache = $this->create_cache($definition, $identifiers);
         return $cache;
index af7fa36..6ad2152 100644 (file)
@@ -714,7 +714,7 @@ class cache_phpunit_tests extends advanced_testcase {
         $this->assertTrue(@unlink($configfile));
 
         // Disable the cache
-        cache_factory::disable();
+        cache_phpunit_factory::phpunit_disable();
 
         // Check we get the expected disabled factory.
         $factory = cache_factory::instance();
index 50b8584..180d768 100644 (file)
@@ -216,4 +216,21 @@ class cache_phpunit_request extends cache_request {
  */
 class cache_phpunit_dummy_overrideclass extends cache_application {
     // Satisfying the code pre-checker is just part of my day job.
+}
+
+/**
+ * Cache PHPUnit specific factory.
+ *
+ * @copyright  2012 Sam Hemelryk
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class cache_phpunit_factory extends cache_factory {
+    /**
+     * Exposes the cache_factory's disable method.
+     *
+     * Perhaps one day that method will be made public, for the time being it is protected.
+     */
+    public static function phpunit_disable() {
+        parent::disable();
+    }
 }
\ No newline at end of file
index fcaf485..8c5bcf7 100644 (file)
@@ -45,6 +45,7 @@ if (file_exists($configfile)) {
 
 define('CLI_SCRIPT', false); // prevents some warnings later
 define('AJAX_SCRIPT', false); // prevents some warnings later
+define('CACHE_DISABLE_ALL', true); // Disables caching.. just in case.
 
 // Servers should define a default timezone in php.ini, but if they don't then make sure something is defined.
 // This is a quick hack.  Ideally we should ask the admin for a value.  See MDL-22625 for more on this.
@@ -220,9 +221,6 @@ $hint_dataroot = '';
 $hint_admindir = '';
 $hint_database = '';
 
-// Disable the cache API.
-cache_factory::disable();
-
 // Are we in help mode?
 if (isset($_GET['help'])) {
     install_print_help_page($_GET['help']);
index ccc4d4e..3037136 100644 (file)
@@ -138,6 +138,14 @@ if (!defined('PHPUNIT_TEST')) {
     define('PHPUNIT_TEST', false);
 }
 
+// When set to true MUC (Moodle caching) will be disabled as much as possible.
+// A special cache factory will be used to handle this situation and will use special "disabled" equivalents objects.
+// This ensure we don't attempt to read or create the config file, don't use stores, don't provide persistence or
+// storage of any kind.
+if (!defined('CACHE_DISABLE_ALL')) {
+    define('CACHE_DISABLE_ALL', false);
+}
+
 // When set to true MUC (Moodle caching) will not use any of the defined or default stores.
 // The Cache API will continue to function however this will force the use of the cachestore_dummy so all requests
 // will be interacting with a static property and will never go to the proper cache stores.
index 7bd0d97..8adb97a 100644 (file)
@@ -1419,6 +1419,10 @@ function install_core($version, $verbose) {
     global $CFG, $DB;
 
     try {
+        // Disable the use of cache stores here. We will reset the factory after we've performed the installation.
+        // This ensures that we don't permanently cache anything during installation.
+        cache_factory::disable_stores();
+
         set_time_limit(600);
         print_upgrade_part_start('moodle', true, $verbose); // does not store upgrade running flag
 
@@ -1442,6 +1446,9 @@ function install_core($version, $verbose) {
         admin_apply_default_settings(NULL, true);
 
         print_upgrade_part_end(null, true, $verbose);
+
+        // Reset the cache, this returns it to a normal operation state.
+        cache_factory::reset();
     } catch (exception $ex) {
         upgrade_handle_exception($ex);
     }
@@ -1463,6 +1470,9 @@ function upgrade_core($version, $verbose) {
     try {
         // Reset caches before any output
         purge_all_caches();
+        // Disable the use of cache stores here. We will reset the factory after we've performed the installation.
+        // This ensures that we don't permanently cache anything during installation.
+        cache_factory::disable_stores();
 
         // Upgrade current language pack if we can
         upgrade_language_pack();
@@ -1495,7 +1505,9 @@ function upgrade_core($version, $verbose) {
         // Update core definitions.
         cache_helper::update_definitions(true);
 
-        // Reset caches again, just to be sure
+        // Reset the cache, this returns it to a normal operation state.
+        cache_factory::reset();
+        // Purge caches again, just to be sure we arn't holding onto old stuff now.
         purge_all_caches();
 
         // Clean up contexts - more and more stuff depends on existence of paths and contexts
@@ -1523,11 +1535,18 @@ function upgrade_noncore($verbose) {
 
     // upgrade all plugins types
     try {
+        // Disable the use of cache stores here. We will reset the factory after we've performed the installation.
+        // This ensures that we don't permanently cache anything during installation.
+        cache_factory::disable_stores();
+
         $plugintypes = get_plugin_types();
         foreach ($plugintypes as $type=>$location) {
             upgrade_plugins($type, 'print_upgrade_part_start', 'print_upgrade_part_end', $verbose);
         }
+        // Update cache definitions. Involves scanning each plugin for any changes.
         cache_helper::update_definitions();
+        // Reset the cache system to a normal state.
+        cache_factory::reset();
     } catch (Exception $ex) {
         upgrade_handle_exception($ex);
     }