From 7dfa10b02e0dfaf537cb72dbca7d666a52073cf7 Mon Sep 17 00:00:00 2001
From: Sam Hemelryk <sam@moodle.com>
Date: Fri, 19 Jul 2013 12:47:23 +1200
Subject: [PATCH] MDL-40700 cache: memcache and memcached stores now allow
 alphanumext prefixes

---
 cache/stores/memcache/addinstanceform.php     |  4 +--
 .../memcache/lang/en/cachestore_memcache.php  |  1 +
 cache/stores/memcache/lib.php                 |  2 +-
 cache/stores/memcache/tests/memcache_test.php | 27 +++++++++++++++++++
 cache/stores/memcached/addinstanceform.php    |  3 ++-
 .../lang/en/cachestore_memcached.php          |  1 +
 cache/stores/memcached/lib.php                |  2 +-
 .../stores/memcached/tests/memcached_test.php | 27 +++++++++++++++++++
 lang/en/cache.php                             |  1 -
 9 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/cache/stores/memcache/addinstanceform.php b/cache/stores/memcache/addinstanceform.php
index bf03aaa0038..eb7c999a1be 100644
--- a/cache/stores/memcache/addinstanceform.php
+++ b/cache/stores/memcache/addinstanceform.php
@@ -49,8 +49,8 @@ class cachestore_memcache_addinstance_form extends cachestore_addinstance_form {
         $form->addElement('text', 'prefix', get_string('prefix', 'cachestore_memcache'),
                 array('maxlength' => 5, 'size' => 5));
         $form->addHelpButton('prefix', 'prefix', 'cachestore_memcache');
-        $form->addRule('prefix', get_string('storeprefixinvalid', 'cache'), 'regex', '#^[a-zA-Z0-9\-_ ]+$#');
-        $form->setType('prefix', PARAM_TEXT);
+        $form->setType('prefix', PARAM_TEXT); // We set to text but we have a rule to limit to alphanumext.
         $form->setDefault('prefix', 'mdl_');
+        $form->addRule('prefix', get_string('prefixinvalid', 'cachestore_memcache'), 'regex', '#^[a-zA-Z0-9\-_]+$#');
     }
 }
\ No newline at end of file
diff --git a/cache/stores/memcache/lang/en/cachestore_memcache.php b/cache/stores/memcache/lang/en/cachestore_memcache.php
index 5cba887de78..fb87d289836 100644
--- a/cache/stores/memcache/lang/en/cachestore_memcache.php
+++ b/cache/stores/memcache/lang/en/cachestore_memcache.php
@@ -31,6 +31,7 @@ $string['prefix'] = 'Key prefix';
 $string['prefix_help'] = 'This prefix is used for all key names on the memcache server.
 * If you only have one Moodle instance using this server, you can leave this value default.
 * Due to key length restrictions, a maximum of 5 characters is permitted.';
+$string['prefixinvalid'] = 'Invalid prefix. You can only use a-z A-Z 0-9-_.';
 $string['servers'] = 'Servers';
 $string['servers_help'] = 'This sets the servers that should be utilised by this memcache adapter.
 Servers should be defined one per line and consist of a server address and optionally a port and weight.
diff --git a/cache/stores/memcache/lib.php b/cache/stores/memcache/lib.php
index 882ab3775cb..2e104bd6385 100644
--- a/cache/stores/memcache/lib.php
+++ b/cache/stores/memcache/lib.php
@@ -404,7 +404,7 @@ class cachestore_memcache extends cache_store implements cache_is_configurable {
      * Generates an instance of the cache store that can be used for testing.
      *
      * @param cache_definition $definition
-     * @return false
+     * @return cachestore_memcache|false
      */
     public static function initialise_test_instance(cache_definition $definition) {
         if (!self::are_requirements_met()) {
diff --git a/cache/stores/memcache/tests/memcache_test.php b/cache/stores/memcache/tests/memcache_test.php
index 023e6cc699b..05ece1b4bc8 100644
--- a/cache/stores/memcache/tests/memcache_test.php
+++ b/cache/stores/memcache/tests/memcache_test.php
@@ -59,4 +59,31 @@ class cachestore_memcache_test extends cachestore_tests {
     protected function get_class_name() {
         return 'cachestore_memcache';
     }
+
+    /**
+     * Tests the valid keys to ensure they work.
+     */
+    public function test_valid_keys() {
+        $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_memcache', 'phpunit_test');
+        $instance = cachestore_memcache::initialise_test_instance($definition);
+
+        $keys = array(
+            // Alphanumeric.
+            'abc', 'ABC', '123', 'aB1', '1aB',
+            // Hyphens.
+            'a-1', '1-a', '-a1', 'a1-',
+            // Underscores.
+            'a_1', '1_a', '_a1', 'a1_'
+        );
+        foreach ($keys as $key) {
+            $this->assertTrue($instance->set($key, $key), "Failed to set key `$key`");
+        }
+        foreach ($keys as $key) {
+            $this->assertEquals($key, $instance->get($key), "Failed to get key `$key`");
+        }
+        $values = $instance->get_many($keys);
+        foreach ($values as $key => $value) {
+            $this->assertEquals($key, $value);
+        }
+    }
 }
\ No newline at end of file
diff --git a/cache/stores/memcached/addinstanceform.php b/cache/stores/memcached/addinstanceform.php
index a8e98bf0f48..7d11f43e660 100644
--- a/cache/stores/memcached/addinstanceform.php
+++ b/cache/stores/memcached/addinstanceform.php
@@ -60,8 +60,9 @@ class cachestore_memcached_addinstance_form extends cachestore_addinstance_form
         $form->setType('serialiser', PARAM_NUMBER);
 
         $form->addElement('text', 'prefix', get_string('prefix', 'cachestore_memcached'), array('size' => 16));
-        $form->setType('prefix', PARAM_ALPHANUM);
+        $form->setType('prefix', PARAM_TEXT); // We set to text but we have a rule to limit to alphanumext.
         $form->addHelpButton('prefix', 'prefix', 'cachestore_memcached');
+        $form->addRule('prefix', get_string('prefixinvalid', 'cachestore_memcached'), 'regex', '#^[a-zA-Z0-9\-_]+$#');
 
         $hashoptions = cachestore_memcached::config_get_hash_options();
         $form->addElement('select', 'hash', get_string('hash', 'cachestore_memcached'), $hashoptions);
diff --git a/cache/stores/memcached/lang/en/cachestore_memcached.php b/cache/stores/memcached/lang/en/cachestore_memcached.php
index 67c3cd22ec2..87713f6755d 100644
--- a/cache/stores/memcached/lang/en/cachestore_memcached.php
+++ b/cache/stores/memcached/lang/en/cachestore_memcached.php
@@ -42,6 +42,7 @@ $string['hash_murmur'] = 'Murmur';
 $string['pluginname'] = 'Memcached';
 $string['prefix'] = 'Prefix key';
 $string['prefix_help'] = 'This can be used to create a "domain" for your item keys allowing you to create multiple memcached stores on a single memcached installation. It cannot be longer than 16 characters in order to ensure key length issues are not encountered.';
+$string['prefixinvalid'] = 'Invalid prefix. You can only use a-z A-Z 0-9-_.';
 $string['serialiser_igbinary'] = 'The igbinary serializer.';
 $string['serialiser_json'] = 'The JSON serializer.';
 $string['serialiser_php'] = 'The default PHP serializer.';
diff --git a/cache/stores/memcached/lib.php b/cache/stores/memcached/lib.php
index 6e3834cce0f..696781ac8f2 100644
--- a/cache/stores/memcached/lib.php
+++ b/cache/stores/memcached/lib.php
@@ -444,7 +444,7 @@ class cachestore_memcached extends cache_store implements cache_is_configurable
      * Generates an instance of the cache store that can be used for testing.
      *
      * @param cache_definition $definition
-     * @return false
+     * @return cachestore_memcached|false
      */
     public static function initialise_test_instance(cache_definition $definition) {
 
diff --git a/cache/stores/memcached/tests/memcached_test.php b/cache/stores/memcached/tests/memcached_test.php
index f0896ca4954..85a8e4a91cb 100644
--- a/cache/stores/memcached/tests/memcached_test.php
+++ b/cache/stores/memcached/tests/memcached_test.php
@@ -59,4 +59,31 @@ class cachestore_memcached_test extends cachestore_tests {
     protected function get_class_name() {
         return 'cachestore_memcached';
     }
+
+    /**
+     * Tests the valid keys to ensure they work.
+     */
+    public function test_valid_keys() {
+        $definition = cache_definition::load_adhoc(cache_store::MODE_APPLICATION, 'cachestore_memcached', 'phpunit_test');
+        $instance = cachestore_memcached::initialise_test_instance($definition);
+
+        $keys = array(
+            // Alphanumeric.
+            'abc', 'ABC', '123', 'aB1', '1aB',
+            // Hyphens.
+            'a-1', '1-a', '-a1', 'a1-',
+            // Underscores.
+            'a_1', '1_a', '_a1', 'a1_'
+        );
+        foreach ($keys as $key) {
+            $this->assertTrue($instance->set($key, $key), "Failed to set key `$key`");
+        }
+        foreach ($keys as $key) {
+            $this->assertEquals($key, $instance->get($key), "Failed to get key `$key`");
+        }
+        $values = $instance->get_many($keys);
+        foreach ($values as $key => $value) {
+            $this->assertEquals($key, $value);
+        }
+    }
 }
\ No newline at end of file
diff --git a/lang/en/cache.php b/lang/en/cache.php
index 0be7c12cd66..72e913d37e8 100644
--- a/lang/en/cache.php
+++ b/lang/en/cache.php
@@ -141,7 +141,6 @@ $string['storename_help'] = 'This sets the store name. It is used to identify th
 $string['storenamealreadyused'] = 'You must choose a unique name for this store.';
 $string['storenameinvalid'] = 'Invalid store name. You can only use a-z A-Z 0-9 -_ and spaces.';
 $string['storeperformance'] = 'Cache store performance reporting - {$a} unique requests per operation.';
-$string['storeprefixinvalid'] = 'Invalid store prefix. You can only use a-z A-Z 0-9 -_ and spaces.';
 $string['storeready'] = 'Ready';
 $string['storenotready'] = 'Store not ready';
 $string['storerequiresattention'] = 'Requires attention.';