From e0d9b7c0d42a66dfc95a6be5b86cfa53b1e9f57b Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Wed, 6 Feb 2013 13:45:17 +1300 Subject: [PATCH] MDL-37683 cache: siteidentifier is now included in the keys --- cache/classes/config.php | 20 ++++++++++++++++++++ cache/classes/definition.php | 19 ++++++++++++++++++- cache/classes/factory.php | 3 +++ cache/classes/helper.php | 19 +++++++++++++++++++ cache/locallib.php | 10 ++++++++++ cache/tests/cache_test.php | 13 +++++++++++-- cache/tests/fixtures/lib.php | 10 ++++++++++ lib/db/upgrade.php | 10 ++++++++++ lib/moodlelib.php | 35 +++++++++++++++++++++++++++++++++-- lib/upgradelib.php | 11 +++++------ version.php | 2 +- 11 files changed, 140 insertions(+), 12 deletions(-) diff --git a/cache/classes/config.php b/cache/classes/config.php index fee8aea82af..232aa464f6b 100644 --- a/cache/classes/config.php +++ b/cache/classes/config.php @@ -71,6 +71,12 @@ class cache_config { */ protected $configlocks = array(); + /** + * The site identifier used when the cache config was last saved. + * @var string + */ + protected $siteidentifier = null; + /** * Please use cache_config::instance to get an instance of the cache config that is ready to be used. */ @@ -139,6 +145,12 @@ class cache_config { $this->configdefinitionmappings = array(); $this->configlockmappings = array(); + $siteidentifier = 'unknown'; + if (array_key_exists('siteidentifier', $configuration)) { + $siteidentifier = $configuration['siteidentifier']; + } + $this->siteidentifier = $siteidentifier; + // Filter the lock instances. $defaultlock = null; foreach ($configuration['locks'] as $conf) { @@ -271,6 +283,14 @@ class cache_config { return true; } + /** + * Returns the site identifier used by the cache API. + * @return string + */ + public function get_site_identifier() { + return $this->siteidentifier; + } + /** * Includes the configuration file and makes sure it contains the expected bits. * diff --git a/cache/classes/definition.php b/cache/classes/definition.php index e278bbbec21..b64df7e2374 100644 --- a/cache/classes/definition.php +++ b/cache/classes/definition.php @@ -274,6 +274,12 @@ class cache_definition { */ protected $definitionhash = null; + /** + * An identifier to make cache keys predictably unique. + * @var string + */ + protected $cacheidentifier = '0'; + /** * Creates a cache definition given a definition from the cache configuration or from a caches.php file. * @@ -674,6 +680,15 @@ class cache_definition { $this->keyprefixmulti = null; } + /** + * Sets an identifier for the cache. + * This can be used + * @param string $identifier + */ + public function set_cache_identifier($identifier) { + $this->cacheidentifier = (string)$identifier; + } + /** * Returns the requirements of this definition as a binary flag. * @return int @@ -723,7 +738,8 @@ class cache_definition { */ public function generate_single_key_prefix() { if ($this->keyprefixsingle === null) { - $this->keyprefixsingle = $this->mode.'/'.$this->mode; + $this->keyprefixsingle = $this->mode.'/'.$this->component.'/'.$this->area; + $this->keyprefixsingle .= '/'.$this->cacheidentifier; $identifiers = $this->get_identifiers(); if ($identifiers) { foreach ($identifiers as $key => $value) { @@ -746,6 +762,7 @@ class cache_definition { 'mode' => $this->mode, 'component' => $this->component, 'area' => $this->area, + 'siteidentifier' => $this->cacheidentifier ); if (!empty($this->identifiers)) { $identifiers = array(); diff --git a/cache/classes/factory.php b/cache/classes/factory.php index f3d520c51af..2c8cdee3e1f 100644 --- a/cache/classes/factory.php +++ b/cache/classes/factory.php @@ -203,6 +203,8 @@ class cache_factory { } // Get the class. Note this is a late static binding so we need to use get_called_class. $definition = cache_definition::load_adhoc($mode, $component, $area, $options); + $config = $this->create_config_instance(); + $definition->set_cache_identifier($config->get_site_identifier()); $definition->set_identifiers($identifiers); $cache = $this->create_cache($definition, $identifiers); if ($definition->should_be_persistent()) { @@ -390,6 +392,7 @@ class cache_factory { } else { $definition = cache_definition::load($id, $definition, $aggregate); } + $definition->set_cache_identifier($instance->get_site_identifier()); } $this->definitions[$id] = $definition; } diff --git a/cache/classes/helper.php b/cache/classes/helper.php index 4439a56bfc7..96b5afee1f9 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -234,6 +234,7 @@ class cache_helper { $factory = cache_factory::instance(); foreach ($instance->get_definitions() as $name => $definitionarr) { $definition = cache_definition::load($name, $definitionarr); + $definition->set_cache_identifier($instance->get_site_identifier()); if ($definition->invalidates_on_event($event)) { // OK at this point we know that the definition has information to invalidate on the event. // There are two routes, either its an application cache in which case we can invalidate it now. @@ -304,6 +305,7 @@ class cache_helper { $factory = cache_factory::instance(); foreach ($instance->get_definitions() as $name => $definitionarr) { $definition = cache_definition::load($name, $definitionarr); + $definition->set_cache_identifier($instance->get_site_identifier()); if ($definition->invalidates_on_event($event)) { // Create the cache. $cache = $factory->create_cache($definition); @@ -496,4 +498,21 @@ class cache_helper { // Second reset anything we have already initialised to ensure we're all up to date. cache_factory::reset(); } + + /** + * Update the site identifier stored by the cache API. + * + * @param string $siteidentifier + */ + public static function update_site_identifier($siteidentifier) { + global $CFG; + // Include locallib + require_once($CFG->dirroot.'/cache/locallib.php'); + $factory = cache_factory::instance(); + $factory->updating_started(); + $config = $factory->create_config_instance(true); + $config->update_site_identifier($siteidentifier); + $factory->updating_finished(); + cache_factory::reset(); + } } \ No newline at end of file diff --git a/cache/locallib.php b/cache/locallib.php index ce926a56e0b..9202c315812 100644 --- a/cache/locallib.php +++ b/cache/locallib.php @@ -119,6 +119,7 @@ class cache_config_writer extends cache_config { */ protected function generate_configuration_array() { $configuration = array(); + $configuration['siteidentifier'] = $this->siteidentifier;; $configuration['stores'] = $this->configstores; $configuration['modemappings'] = $this->configmodemappings; $configuration['definitions'] = $this->configdefinitions; @@ -524,6 +525,15 @@ class cache_config_writer extends cache_config { $this->config_save(); } + /** + * Update the site identifier stored by the cache API. + * + * @param string $siteidentifier + */ + public function update_site_identifier($siteidentifier) { + $this->siteidentifier = md5((string)$siteidentifier); + $this->config_save(); + } } /** diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index 98d9c5bafe3..545986ae34a 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -553,6 +553,8 @@ class cache_phpunit_tests extends advanced_testcase { 'mode' => cache_store::MODE_APPLICATION, 'component' => 'phpunit', 'area' => 'eventinvalidationtest', + 'simplekeys' => true, + 'simpledata' => true, 'invalidationevents' => array( 'crazyevent' ) @@ -567,13 +569,16 @@ 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/a65/a65b1dc524cf6e03c1795197c84d5231eb229b86.cache'; + $hash = md5(cache_store::MODE_APPLICATION.'/phpunit/eventinvalidationtest/'.$CFG->wwwroot.'phpunit'); + $timefile = $CFG->dataroot."/cache/cachestore_file/default_application/phpunit_eventinvalidationtest/las/lastinvalidation-$hash.cache"; + // Make sure the file is correct. + $this->assertTrue(file_exists($timefile)); $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/626/626e9c7a45febd98f064c2b383de8d9d4ebbde7b.cache'; + $datafile = $CFG->dataroot."/cache/cachestore_file/default_application/phpunit_eventinvalidationtest/tes/testkey1-$hash.cache"; $datacont = serialize("test data 1"); make_writable_directory(dirname($datafile)); file_put_contents($datafile, $datacont); @@ -586,6 +591,8 @@ class cache_phpunit_tests extends advanced_testcase { 'mode' => cache_store::MODE_APPLICATION, 'component' => 'phpunit', 'area' => 'eventinvalidationtest', + 'simplekeys' => true, + 'simpledata' => true, )); $cache = cache::make('phpunit', 'eventinvalidationtest'); $this->assertEquals('test data 1', $cache->get('testkey1')); @@ -597,6 +604,8 @@ class cache_phpunit_tests extends advanced_testcase { 'mode' => cache_store::MODE_APPLICATION, 'component' => 'phpunit', 'area' => 'eventinvalidationtest', + 'simplekeys' => true, + 'simpledata' => true, 'invalidationevents' => array( 'crazyevent' ) diff --git a/cache/tests/fixtures/lib.php b/cache/tests/fixtures/lib.php index d5e5297a293..471521b2556 100644 --- a/cache/tests/fixtures/lib.php +++ b/cache/tests/fixtures/lib.php @@ -102,6 +102,16 @@ class cache_config_phpunittest extends cache_config_writer { 'sort' => (int)$sort ); } + + /** + * Overrides the default site identifier used by the Cache API so that we can be sure of what it is. + * + * @return string + */ + public function get_site_identifier() { + global $CFG; + return $CFG->wwwroot.'phpunit'; + } } /** diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index b563121dd3f..06c28909a3b 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1577,5 +1577,15 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2013021100.01); } + if ($oldversion < 2013021800.00) { + // Add the site identifier to the cache config's file. + $siteidentifier = $DB->get_field('config', 'value', array('name' => 'siteidentifier')); + cache_helper::update_site_identifier($siteidentifier); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2013021800.00); + } + + return true; } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index eca1097b5fc..2d28bfe4acd 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -1327,6 +1327,9 @@ function set_config($name, $value, $plugin=NULL) { $DB->insert_record('config', $config, false); } } + if ($name === 'siteidentifier') { + cache_helper::update_site_identifier($value); + } cache_helper::invalidate_by_definition('core', 'config', array(), 'core'); } else { // plugin scope if ($id = $DB->get_field('config_plugins', 'id', array('name'=>$name, 'plugin'=>$plugin))) { @@ -1360,6 +1363,8 @@ function set_config($name, $value, $plugin=NULL) { * If called with 2 parameters it will return a string single * value or false if the value is not found. * + * @static $siteidentifier The site identifier is not cached. We use this static cache so + * that we need only fetch it once per request. * @param string $plugin full component name * @param string $name default NULL * @return mixed hash-like object or single value, return false no config found @@ -1367,6 +1372,8 @@ function set_config($name, $value, $plugin=NULL) { function get_config($plugin, $name = NULL) { global $CFG, $DB; + static $siteidentifier = null; + if ($plugin === 'moodle' || $plugin === 'core' || empty($plugin)) { $forced =& $CFG->config_php_settings; $iscore = true; @@ -1380,8 +1387,28 @@ function get_config($plugin, $name = NULL) { $iscore = false; } - if (!empty($name) && array_key_exists($name, $forced)) { - return (string)$forced[$name]; + if ($siteidentifier === null) { + try { + // This may fail during installation. + // If you have a look at {@link initialise_cfg()} you will see that this is how we detect the need to + // install the database. + $siteidentifier = $DB->get_field('config', 'value', array('name' => 'siteidentifier')); + } catch (dml_exception $ex) { + // It's failed. We'll use this oppertunity to disable cache stores so that we don't inadvertingly start using + // old caches. People should delete there moodledata dirs when reinstalling the database... but they don't. + cache_factory::disable_stores(); + // Set siteidentifier to false. We don't want to trip this continually. + $siteidentifier = false; + throw $ex; + } + } + + if (!empty($name)) { + if (array_key_exists($name, $forced)) { + return (string)$forced[$name]; + } else if ($name === 'siteidentifier' && $plugin == 'core') { + return $siteidentifier; + } } $cache = cache::make('core', 'config'); @@ -1405,6 +1432,10 @@ function get_config($plugin, $name = NULL) { return false; } + if ($plugin === 'core') { + $result['siteidentifier'] = $siteidentifier; + } + foreach ($forced as $key => $value) { if (is_null($value) or is_array($value) or is_object($value)) { // we do not want any extra mess here, just real settings that could be saved in db diff --git a/lib/upgradelib.php b/lib/upgradelib.php index 111026c1f10..938258334ac 100644 --- a/lib/upgradelib.php +++ b/lib/upgradelib.php @@ -1448,8 +1448,9 @@ function install_core($version, $verbose) { print_upgrade_part_end(null, true, $verbose); - // Reset the cache, this returns it to a normal operation state. - cache_factory::reset(); + // Purge all caches. They're disabled but this ensures that we don't have any persistent data just in case something + // during installation didn't use APIs. + cache_helper::purge_all(); } catch (exception $ex) { upgrade_handle_exception($ex); } @@ -1536,8 +1537,8 @@ 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. + // Disable the use of cache stores here. + // We don't reset this, the site can live without proper caching for life of this request. cache_factory::disable_stores(); $plugintypes = get_plugin_types(); @@ -1546,8 +1547,6 @@ function upgrade_noncore($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); } diff --git a/version.php b/version.php index 5ab02abea99..c0c8a6b3af5 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2013021400.00; // YYYYMMDD = weekly release date of this DEV branch +$version = 2013021800.00; // YYYYMMDD = weekly release date of this DEV branch // RR = release increments - 00 in DEV branches // .XX = incremental changes