From 8dbfbd71c94a0da4e9f7a9a7176a799c96038889 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Tue, 18 Sep 2012 17:37:41 +1200 Subject: [PATCH] MDL-25290 cache: Fixed up handling of objects by cache loader --- cache/classes/loaders.php | 70 ++++++++++++++++++++++++++++++++++++++ cache/tests/cache_test.php | 52 +++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php index 28b36aef53b..c97b50a3e4c 100644 --- a/cache/classes/loaders.php +++ b/cache/classes/loaders.php @@ -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. * diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php index 1ba849ca2c4..cc40a4e89fa 100644 --- a/cache/tests/cache_test.php +++ b/cache/tests/cache_test.php @@ -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); } /**