MDL-36466 cache: tidy up after review

admin/cli/install.php

@@ -127,6 +127,9 @@
 /** 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($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'),

admin/index.php

@@ -105,9 +105,6 @@
 }
 
 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 ($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 (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) {

cache/README.md

@@ -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
 ---------------

cache/classes/factory.php

@@ -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 @@ public function is_disabled() {
      * 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();
     }
 

cache/disabledlib.php

@@ -225,10 +225,13 @@ public function create_cache_from_definition($component, $area, array $identifie
      * @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;

cache/tests/cache_test.php

@@ -714,7 +714,7 @@ public function test_disable() {
         $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();

cache/tests/fixtures/lib.php

@@ -216,4 +216,21 @@ public function phpunit_get_store_class() {
  */
 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();
+    }
 }

install.php

@@ -45,6 +45,7 @@
 
 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_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']);

lib/setup.php

@@ -138,6 +138,14 @@
     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.

lib/upgradelib.php

@@ -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);
     }