Prevent cache key collisions

README.md

@@ -251,7 +251,8 @@ Create a new `DataLoaderPHP` given a batch loading instance and options.
 
 Loads a key, returning a `Promise` for the value represented by that key.
 
-- *$key*: An key value to load.
+- *$key*: A scalar, array, or object key to load. The default `CacheMap` does not
+  support resource keys or recursive array keys.
 
 ##### `loadMany($keys)`
 

src/CacheMap.php

@@ -14,48 +14,114 @@
 class CacheMap
 {
     private $promiseCache = [];
+    private $objectPromiseCache;
+    private $objectIds;
+    private $nextObjectId = 0;
 
     public function get($key)
     {
-        $key = self::serializedKey($key);
+        if (is_object($key)) {
+            return null !== $this->objectPromiseCache && isset($this->objectPromiseCache[$key])
+                ? $this->objectPromiseCache[$key]
+                : null;
+        }
+
+        $key = $this->serializedKey($key);
 
         return isset($this->promiseCache[$key]) ? $this->promiseCache[$key] : null;
     }
 
     public function has($key)
     {
-        return isset($this->promiseCache[self::serializedKey($key)]);
+        if (is_object($key)) {
+            return null !== $this->objectPromiseCache && isset($this->objectPromiseCache[$key]);
+        }
+
+        return isset($this->promiseCache[$this->serializedKey($key)]);
     }
 
     public function set($key, $promise)
     {
-        $this->promiseCache[self::serializedKey($key)] = $promise;
+        if (is_object($key)) {
+            if (null === $this->objectPromiseCache) {
+                $this->objectPromiseCache = new \WeakMap();
+            }
+            $this->objectPromiseCache[$key] = $promise;
+
+            return $this;
+        }
+
+        $this->promiseCache[$this->serializedKey($key)] = $promise;
 
         return $this;
     }
 
     public function clear($key)
     {
-        unset($this->promiseCache[self::serializedKey($key)]);
+        if (is_object($key)) {
+            if (null !== $this->objectPromiseCache) {
+                unset($this->objectPromiseCache[$key]);
+            }
+
+            return $this;
+        }
+
+        unset($this->promiseCache[$this->serializedKey($key)]);
 
         return $this;
     }
 
     public function clearAll()
     {
         $this->promiseCache = [];
+        $this->objectPromiseCache = null;
 
         return $this;
     }
 
-    private static function serializedKey($key)
+    private function serializedKey($key)
     {
-        if (is_object($key)) {
-            return spl_object_hash($key);
-        } elseif (is_array($key)) {
-            return json_encode($key);
+        $arrayReferences = [];
+
+        return serialize($this->encodeValue($key, $arrayReferences));
+    }
+
+    private function encodeValue(&$value, array &$arrayReferences)
+    {
+        $type = gettype($value);
+        if ('resource' === $type || 'resource (closed)' === $type) {
+            throw new \InvalidArgumentException('Resources cannot be used in CacheMap keys.');
+        }
+        if (is_object($value)) {
+            if (null === $this->objectIds) {
+                $this->objectIds = new \WeakMap();
+            }
+            if (!isset($this->objectIds[$value])) {
+                $this->objectIds[$value] = ++$this->nextObjectId;
+            }
+
+            return ['object', $this->objectIds[$value]];
+        }
+        if (!is_array($value)) {
+            return [$type, $value];
+        }
+
+        $referenceHolder = [&$value];
+        $referenceId = \ReflectionReference::fromArrayElement($referenceHolder, 0)->getId();
+        if (isset($arrayReferences[$referenceId])) {
+            throw new \InvalidArgumentException('Recursive arrays cannot be used in CacheMap keys.');
+        }
+
+        $arrayReferences[$referenceId] = true;
+        $items = [];
+        try {
+            foreach ($value as $key => &$item) {
+                $items[] = [[gettype($key), $key], $this->encodeValue($item, $arrayReferences)];
+            }
+        } finally {
+            unset($item, $arrayReferences[$referenceId]);
         }
 
-        return $key;
+        return ['array', $items];
     }
 }

tests/CacheMapTest.php

@@ -0,0 +1,156 @@
+<?php
+
+/*
+ * This file is part of the DataLoaderPhp package.
+ *
+ * (c) Overblog <http://github.com/overblog/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Overblog\DataLoader\Test;
+
+use Overblog\DataLoader\CacheMap;
+
+class CacheMapTest extends \PHPUnit\Framework\TestCase
+{
+    public function testDistinguishesScalarTypes()
+    {
+        $cacheMap = new CacheMap();
+        $cacheMap
+            ->set(1, 'integer')
+            ->set('1', 'string')
+            ->set(true, 'boolean')
+            ->set(1.0, 'float');
+
+        $this->assertSame('integer', $cacheMap->get(1));
+        $this->assertSame('string', $cacheMap->get('1'));
+        $this->assertSame('boolean', $cacheMap->get(true));
+        $this->assertSame('float', $cacheMap->get(1.0));
+    }
+
+    public function testDistinguishesArrayKeysFromStringsWithTheSameSerializedForm()
+    {
+        $cacheMap = new CacheMap();
+        $arrayKey = ['id' => 1];
+        $stringKey = json_encode($arrayKey);
+
+        $cacheMap->set($arrayKey, 'array')->set($stringKey, 'string');
+
+        $this->assertSame('array', $cacheMap->get($arrayKey));
+        $this->assertSame('string', $cacheMap->get($stringKey));
+    }
+
+    public function testDistinguishesArrayKeysThatJsonCannotEncode()
+    {
+        $cacheMap = new CacheMap();
+        $firstKey = ["\xB1\x31"];
+        $secondKey = ["\xB1\x32"];
+
+        $this->assertFalse(json_encode($firstKey));
+        $this->assertFalse(json_encode($secondKey));
+
+        $cacheMap->set($firstKey, 'first')->set($secondKey, 'second');
+
+        $this->assertSame('first', $cacheMap->get($firstKey));
+        $this->assertSame('second', $cacheMap->get($secondKey));
+    }
+
+    public function testRejectsResourceValuesInArrayKeys()
+    {
+        $resource = fopen('php://memory', 'r');
+
+        try {
+            $this->expectException(\InvalidArgumentException::class);
+            $this->expectExceptionMessage('Resources cannot be used in CacheMap keys.');
+
+            (new CacheMap())->set([$resource], 'value');
+        } finally {
+            fclose($resource);
+        }
+    }
+
+    public function testUsesObjectIdentity()
+    {
+        $cacheMap = new CacheMap();
+        $firstKey = (object) ['id' => 1];
+        $secondKey = (object) ['id' => 1];
+
+        $cacheMap->set($firstKey, 'first')->set($secondKey, 'second');
+
+        $this->assertSame('first', $cacheMap->get($firstKey));
+        $this->assertSame('second', $cacheMap->get($secondKey));
+
+        $cacheMap->clear($firstKey);
+
+        $this->assertFalse($cacheMap->has($firstKey));
+        $this->assertTrue($cacheMap->has($secondKey));
+    }
+
+    public function testUsesNestedObjectIdentity()
+    {
+        $cacheMap = new CacheMap();
+        $firstObject = (object) ['id' => 1];
+        $secondObject = (object) ['id' => 1];
+
+        $cacheMap->set([$firstObject], 'first')->set([$secondObject], 'second');
+
+        $this->assertSame('first', $cacheMap->get([$firstObject]));
+        $this->assertSame('second', $cacheMap->get([$secondObject]));
+    }
+
+    public function testReusesIdentityForTheSameNestedObject()
+    {
+        $cacheMap = new CacheMap();
+        $object = new \stdClass();
+
+        $cacheMap->set([$object, $object], 'value');
+
+        $this->assertSame('value', $cacheMap->get([$object, $object]));
+    }
+
+    public function testSupportsClosuresInArrayKeysByIdentity()
+    {
+        $cacheMap = new CacheMap();
+        $firstClosure = function () {
+        };
+        $secondClosure = function () {
+        };
+
+        $cacheMap->set([$firstClosure], 'first')->set([$secondClosure], 'second');
+
+        $this->assertSame('first', $cacheMap->get([$firstClosure]));
+        $this->assertSame('second', $cacheMap->get([$secondClosure]));
+    }
+
+    public function testRejectsRecursiveArrayKeys()
+    {
+        $key = [];
+        $key['self'] = &$key;
+
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Recursive arrays cannot be used in CacheMap keys.');
+
+        (new CacheMap())->set($key, 'value');
+    }
+
+    public function testClearAllClearsScalarAndObjectIdentityKeys()
+    {
+        $cacheMap = new CacheMap();
+        $object = new \stdClass();
+        $cacheMap
+            ->set('scalar', 'scalar value')
+            ->set($object, 'object value')
+            ->set([$object], 'nested object value')
+            ->clearAll();
+
+        $this->assertFalse($cacheMap->has('scalar'));
+        $this->assertFalse($cacheMap->has($object));
+        $this->assertFalse($cacheMap->has([$object]));
+
+        $cacheMap->set([$object], 'new value');
+
+        $this->assertSame('new value', $cacheMap->get([$object]));
+    }
+}