MDL-25290 cache: Fixed up handling of objects by cache loader
authorSam Hemelryk <sam@moodle.com>
Tue, 18 Sep 2012 05:37:41 +0000 (17:37 +1200)
committerSam Hemelryk <sam@moodle.com>
Sun, 7 Oct 2012 20:53:52 +0000 (09:53 +1300)
cache/classes/loaders.php
cache/tests/cache_test.php

index 28b36ae..c97b50a 100644 (file)
@@ -459,6 +459,12 @@ class cache implements cache_loader, cache_is_key_aware {
         }
         if (is_object($data) && $data instanceof cacheable_object) {
             $data = new cache_cached_object($data);
+        } else if (!is_scalar($data)) {
+            // If data is an object it will be a reference.
+            // If data is an array if may contain references.
+            // We want to break references so that the cache cannot be modified outside of itself.
+            // Call the function to unreference it (in the best way possible).
+            $data = $this->unref($data);
         }
         if ($this->has_a_ttl() && !$this->store_supports_native_ttl()) {
             $data = new cache_ttl_wrapper($data, $this->definition->get_ttl());
@@ -470,6 +476,70 @@ class cache implements cache_loader, cache_is_key_aware {
         return $this->store->set($parsedkey, $data);
     }
 
+    /**
+     * Removes references where required.
+     *
+     * @param stdClass|array $data
+     */
+    protected function unref($data) {
+        // Check if it requires serialisation in order to produce a reference free copy.
+        if ($this->requires_serialisation($data)) {
+            // Damn, its going to have to be serialise.
+            $data = serialize($data);
+            // We unserialise immediately so that we don't have to do it every time on get.
+            $data = unserialize($data);
+        } else if (!is_scalar($data)) {
+            // Its safe to clone, lets do it, its going to beat the pants of serialisation.
+            $data = $this->deep_clone($data);
+        }
+        return $data;
+    }
+
+    /**
+     * Checks to see if a var requires serialisation.
+     *
+     * @param mixed $value The value to check.
+     * @param int $depth Used to ensure we don't enter an endless loop (think recursion).
+     * @return bool Returns true if the value is going to require serialisation in order to ensure a reference free copy
+     *      or false if its safe to clone.
+     */
+    protected function requires_serialisation($value, $depth = 1) {
+        if (is_scalar($value)) {
+            return false;
+        } else if (is_array($value) || $value instanceof stdClass || $value instanceof Traversable) {
+            if ($depth > 5) {
+                // Skrew it, mega-deep object, developer you suck, we're just going to serialise.
+                return true;
+            }
+            foreach ($value as $key => $subvalue) {
+                if ($this->requires_serialisation($subvalue, $depth++)) {
+                    return true;
+                }
+            }
+        }
+        // Its not scalar, array, or stdClass so we'll need to serialise.
+        return true;
+    }
+
+    /**
+     * Creates a reference free clone of the given value.
+     *
+     * @param mixed $value
+     * @return mixed
+     */
+    protected function deep_clone($value) {
+        if (is_object($value)) {
+            // Objects must be cloned to begin with.
+            $value = clone $value;
+        }
+        if (is_array($value) || is_object($value)) {
+            foreach ($value as $key => $subvalue) {
+                $value[$key] = $this->deep_clone($subvalue);
+            }
+        }
+        return $value;
+    }
+
     /**
      * Sends several key => value pairs to the cache.
      *
index 1ba849c..cc40a4e 100644 (file)
@@ -60,7 +60,7 @@ class cache_phpunit_tests extends advanced_testcase {
         $this->assertTrue(cache_config_phpunittest::config_file_exists());
 
         $stores = $instance->get_all_stores();
-        $this->assertCount(4, $stores);
+        $this->assertCount(3, $stores);
         foreach ($stores as $name => $store) {
             // Check its an array.
             $this->assertInternalType('array', $store);
@@ -241,6 +241,56 @@ class cache_phpunit_tests extends advanced_testcase {
 
         $this->assertFalse($cache->get('key1'));
         $this->assertFalse($cache->get('key2'));
+
+        // Quick reference test.
+        $obj = new stdClass;
+        $obj->key = 'value';
+        $ref =& $obj;
+        $this->assertTrue($cache->set('obj', $obj));
+
+        $obj->key = 'eulav';
+        $var = $cache->get('obj');
+        $this->assertInstanceOf('stdClass', $var);
+        $this->assertEquals('value', $var->key);
+
+        $ref->key = 'eulav';
+        $var = $cache->get('obj');
+        $this->assertInstanceOf('stdClass', $var);
+        $this->assertEquals('value', $var->key);
+
+        $this->assertTrue($cache->delete('obj'));
+
+        // Deep reference test.
+        $obj1 = new stdClass;
+        $obj1->key = 'value';
+        $obj2 = new stdClass;
+        $obj2->key = 'test';
+        $obj3 = new stdClass;
+        $obj3->key = 'pork';
+        $obj1->subobj =& $obj2;
+        $obj2->subobj =& $obj3;
+        $this->assertTrue($cache->set('obj', $obj1));
+
+        $obj1->key = 'eulav';
+        $obj2->key = 'tset';
+        $obj3->key = 'krop';
+        $var = $cache->get('obj');
+        $this->assertInstanceOf('stdClass', $var);
+        $this->assertEquals('value', $var->key);
+        $this->assertInstanceOf('stdClass', $var->subobj);
+        $this->assertEquals('test', $var->subobj->key);
+        $this->assertInstanceOf('stdClass', $var->subobj->subobj);
+        $this->assertEquals('pork', $var->subobj->subobj->key);
+        $this->assertTrue($cache->delete('obj'));
+
+        // Death reference test... basicaly we don't want this to die.
+        $obj = new stdClass;
+        $obj->key = 'value';
+        $obj->self =& $obj;
+        $this->assertTrue($cache->set('obj', $obj));
+        $var = $cache->get('obj');
+        $this->assertInstanceOf('stdClass', $var);
+        $this->assertEquals('value', $var->key);
     }
 
     /**