From 94ef67cf5fad90b61a1ccc2918c7403b547bd63e Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Thu, 22 Nov 2012 10:33:57 +1300 Subject: [PATCH] MDL-36466 cache: tidy up after review --- admin/cli/install.php | 6 +++--- admin/index.php | 8 -------- cache/README.md | 4 ++-- cache/classes/factory.php | 25 +++++++++++++++++++------ cache/disabledlib.php | 7 +++++-- cache/tests/cache_test.php | 2 +- cache/tests/fixtures/lib.php | 17 +++++++++++++++++ install.php | 4 +--- lib/setup.php | 8 ++++++++ lib/upgradelib.php | 21 ++++++++++++++++++++- 10 files changed, 76 insertions(+), 26 deletions(-) diff --git a/admin/cli/install.php b/admin/cli/install.php index b662d4cd36884..ca8aaef07bc82 100644 --- a/admin/cli/install.php +++ b/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'), diff --git a/admin/index.php b/admin/index.php index 6ce81f9a475e4..d75e36488aeb5 100644 --- a/admin/index.php +++ b/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) { diff --git a/cache/README.md b/cache/README.md index ef8c82a63a63c..981e04345bb16 100644 --- a/cache/README.md +++ b/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 --------------- diff --git a/cache/classes/factory.php b/cache/classes/factory.php index 3ea7a5b37c13a..1458cf057aba7 100644 --- a/cache/classes/factory.php +++ b/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(); } diff --git a/cache/disabledlib.php b/cache/disabledlib.php index ea703295ffbe3..18cc11eb07da9 100644 --- a/cache/disabledlib.php +++ b/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; diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index af7fa36dd4964..6ad215262c805 100644 --- a/cache/tests/cache_test.php +++ b/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(); diff --git a/cache/tests/fixtures/lib.php b/cache/tests/fixtures/lib.php index 50b8584bab327..180d7680e35fc 100644 --- a/cache/tests/fixtures/lib.php +++ b/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(); + } } \ No newline at end of file diff --git a/install.php b/install.php index fcaf485a0c647..8c5bcf746fbf5 100644 --- a/install.php +++ b/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']); diff --git a/lib/setup.php b/lib/setup.php index ccc4d4eb1b37d..3037136f28214 100644 --- a/lib/setup.php +++ b/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. diff --git a/lib/upgradelib.php b/lib/upgradelib.php index 7bd0d9793a802..8adb97a60afd5 100644 --- a/lib/upgradelib.php +++ b/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); }