Merge branch 'wip-MDL-36322-m25' of https://github.com/samhemelryk/moodle

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

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

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

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

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;
     }
 
     /**

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;
     }
 
     /**

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;
     }
 
     /**