diff --git a/cache/classes/factory.php b/cache/classes/factory.php index c23a147c6a8..f3d520c51af 100644 --- a/cache/classes/factory.php +++ b/cache/classes/factory.php @@ -263,7 +263,13 @@ class cache_factory { if (!$store->is_ready() || !$store->is_supported_mode($definition->get_mode())) { return false; } - $store = clone($this->stores[$name]); + // We always create a clone of the original store. + // If we were to clone a store that had already been initialised with a definition then + // we'd run into a myriad of issues. + // We use a method of the store to create a clone rather than just creating it ourselves + // so that if any store out there doesn't handle cloning they can override this method in + // order to address the issues. + $store = $this->stores[$name]->create_clone($details); $store->initialise($definition); return $store; } diff --git a/cache/classes/store.php b/cache/classes/store.php index 7cbb069dd95..5bb26e4d4fd 100644 --- a/cache/classes/store.php +++ b/cache/classes/store.php @@ -128,7 +128,16 @@ abstract class cache_store implements cache_store_interface { /** * Constructs an instance of the cache store. * - * This method should not create connections or perform and processing, it should be used + * The constructor should be responsible for creating anything needed by the store that is not + * specific to a definition. + * Tasks such as opening a connection to check it is available are best done here. + * Tasks that are definition specific such as creating a storage area for the definition data + * or creating key tables and indexs are best done within the initialise method. + * + * Once a store has been constructed the cache API will check it is ready to be intialised with + * a definition by called $this->is_ready(). + * If the setup of the store failed (connection could not be established for example) then + * that method should return false so that the store instance is not selected for use. * * @param string $name The name of the cache store * @param array $configuration The configuration for this store instance. @@ -144,7 +153,12 @@ abstract class cache_store implements cache_store_interface { /** * Initialises a new instance of the cache store given the definition the instance is to be used for. * - * This function should prepare any given connections etc. + * This function should be used to run any definition specific setup the store instance requires. + * Tasks such as creating storage areas, or creating indexes are best done here. + * + * Its important to note that the initialise method is expected to always succeed. + * If there are setup tasks that may fail they should be done within the __construct method + * and should they fail is_ready should return false. * * @param cache_definition $definition */ @@ -292,4 +306,22 @@ abstract class cache_store implements cache_store_interface { public function supports_native_ttl() { return $this::get_supported_features() & self::SUPPORTS_NATIVE_TTL; } + + /** + * Creates a clone of this store instance ready to be initialised. + * + * This method is used so that a cache store needs only be constructed once. + * Future requests for an instance of the store will be given a cloned instance. + * + * If you are writing a cache store that isn't compatible with the clone operation + * you can override this method to handle any situations you want before cloning. + * + * @param array $details An array containing the details of the store from the cache config. + * @return cache_store + */ + public function create_clone(array $details = array()) { + // By default we just run clone. + // Any stores that have an issue with this will need to override the create_clone method. + return clone($this); + } } diff --git a/cache/stores/memcache/lib.php b/cache/stores/memcache/lib.php index d25b1c046d8..e9c6e1d32b6 100644 --- a/cache/stores/memcache/lib.php +++ b/cache/stores/memcache/lib.php @@ -69,6 +69,12 @@ class cachestore_memcache extends cache_store implements cache_is_configurable { */ protected $isready = false; + /** + * Set to true once this store instance has been initialised. + * @var bool + */ + protected $isinitialised = false; + /** * The cache definition this store was initialised for. * @var cache_definition @@ -106,7 +112,12 @@ class cachestore_memcache extends cache_store implements cache_is_configurable { $this->servers[] = $server; } - $this->isready = true; + $this->connection = new Memcache; + foreach ($this->servers as $server) { + $this->connection->addServer($server[0], $server[1], true, $server[2]); + // Test the connection to this server. + $this->isready = @$this->connection->set("ping", 'ping', MEMCACHE_COMPRESSED, 1); + } } /** @@ -121,10 +132,7 @@ class cachestore_memcache extends cache_store implements cache_is_configurable { throw new coding_exception('This memcache instance has already been initialised.'); } $this->definition = $definition; - $this->connection = new Memcache; - foreach ($this->servers as $server) { - $this->connection->addServer($server[0], $server[1], true, $server[2]); - } + $this->isinitialised = true; } /** @@ -133,7 +141,7 @@ class cachestore_memcache extends cache_store implements cache_is_configurable { * @return bool */ public function is_initialised() { - return ($this->connection !== null); + return ($this->isinitialised); } /** @@ -276,7 +284,10 @@ class cachestore_memcache extends cache_store implements cache_is_configurable { * @return boolean True on success. False otherwise. */ public function purge() { - $this->connection->flush(); + if ($this->isready) { + $this->connection->flush(); + } + return true; } diff --git a/cache/stores/memcached/lib.php b/cache/stores/memcached/lib.php index 4370a6d84b1..4cad20cb72b 100644 --- a/cache/stores/memcached/lib.php +++ b/cache/stores/memcached/lib.php @@ -74,6 +74,12 @@ class cachestore_memcached extends cache_store implements cache_is_configurable */ protected $isready = false; + /** + * Set to true when this store instance has been initialised. + * @var bool + */ + protected $isinitialised = false; + /** * The cache definition this store was initialised with. * @var cache_definition @@ -127,7 +133,15 @@ class cachestore_memcached extends cache_store implements cache_is_configurable $this->options[Memcached::OPT_HASH] = $hashmethod; $this->options[Memcached::OPT_BUFFER_WRITES] = $bufferwrites; - $this->isready = true; + $this->connection = new Memcached(crc32($this->name)); + $servers = $this->connection->getServerList(); + if (empty($servers)) { + foreach ($this->options as $key => $value) { + $this->connection->setOption($key, $value); + } + $this->connection->addServers($this->servers); + $this->isready = @$this->connection->set("ping", 'ping', 1); + } } /** @@ -142,14 +156,7 @@ class cachestore_memcached extends cache_store implements cache_is_configurable throw new coding_exception('This memcached instance has already been initialised.'); } $this->definition = $definition; - $this->connection = new Memcached(crc32($this->name)); - $servers = $this->connection->getServerList(); - if (empty($servers)) { - foreach ($this->options as $key => $value) { - $this->connection->setOption($key, $value); - } - $this->connection->addServers($this->servers); - } + $this->isinitialised = true; } /** @@ -158,7 +165,7 @@ class cachestore_memcached extends cache_store implements cache_is_configurable * @return bool */ public function is_initialised() { - return ($this->connection !== null); + return ($this->isinitialised); } /** @@ -302,7 +309,10 @@ class cachestore_memcached extends cache_store implements cache_is_configurable * @return boolean True on success. False otherwise. */ public function purge() { - $this->connection->flush(); + if ($this->isready) { + $this->connection->flush(); + } + return true; } diff --git a/cache/stores/mongodb/lib.php b/cache/stores/mongodb/lib.php index cefc299f4b1..39a180683c7 100644 --- a/cache/stores/mongodb/lib.php +++ b/cache/stores/mongodb/lib.php @@ -100,7 +100,17 @@ class cachestore_mongodb extends cache_store implements cache_is_configurable { protected $definitionhash = null; /** - * Constructs a new instance of the Mongo store but does not connect to it. + * Set to true once this store is ready to be initialised and used. + * @var bool + */ + protected $isready = false; + + /** + * Constructs a new instance of the Mongo store. + * + * Noting that this function is not an initialisation. It is used to prepare the store for use. + * The store will be initialised when required and will be provided with a cache_definition at that time. + * * @param string $name * @param array $configuration */ @@ -130,7 +140,12 @@ class cachestore_mongodb extends cache_store implements cache_is_configurable { $this->extendedmode = $configuration['extendedmode']; } - $this->isready = self::are_requirements_met(); + try { + $this->connection = new Mongo($this->server, $this->options); + $this->isready = true; + } catch (MongoConnectionException $e) { + // We only want to catch MongoConnectionExceptions here. + } } /** @@ -166,7 +181,7 @@ class cachestore_mongodb extends cache_store implements cache_is_configurable { /** * Initialises the store instance for use. * - * This function is reponsible for making the connection. + * Once this has been done the cache is all set to be used. * * @param cache_definition $definition * @throws coding_exception @@ -175,9 +190,8 @@ class cachestore_mongodb extends cache_store implements cache_is_configurable { if ($this->is_initialised()) { throw new coding_exception('This mongodb instance has already been initialised.'); } - $this->definitionhash = $definition->generate_definition_hash(); - $this->connection = new Mongo($this->server, $this->options); $this->database = $this->connection->selectDB($this->databasename); + $this->definitionhash = $definition->generate_definition_hash(); $this->collection = $this->database->selectCollection($this->definitionhash); $this->collection->ensureIndex(array('key' => 1), array( 'safe' => $this->usesafe, @@ -366,8 +380,12 @@ class cachestore_mongodb extends cache_store implements cache_is_configurable { * @return boolean True on success. False otherwise. */ public function purge() { - $this->collection->drop(); - $this->collection = $this->database->selectCollection($this->definitionhash); + if ($this->isready) { + $this->collection->drop(); + $this->collection = $this->database->selectCollection($this->definitionhash); + } + + return true; } /** diff --git a/cache/stores/session/lib.php b/cache/stores/session/lib.php index 5a2b9a09d00..8cb42f0d0e0 100644 --- a/cache/stores/session/lib.php +++ b/cache/stores/session/lib.php @@ -361,6 +361,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar */ public function purge() { $this->store = array(); + return true; } /** diff --git a/cache/stores/static/lib.php b/cache/stores/static/lib.php index 6b4ab4ed58d..4ac32de7448 100644 --- a/cache/stores/static/lib.php +++ b/cache/stores/static/lib.php @@ -358,6 +358,7 @@ class cachestore_static extends static_data_store implements cache_is_key_aware public function purge() { $this->flush_store_by_id($this->storeid); $this->store = &self::register_store_id($this->storeid); + return true; } /**