Repository: JsonMapper/JsonMapper (php)

Issue:
Improve caching
Hi!

I might did it wrong way but this drastically improves parsing speed on my system (from 16s to 6s on 36MB json file with nested objects)!!!
```php
$cache = new ArrayCache();
$mapper = (new JsonMapperFactory())->create(
    new PropertyMapper(),
    new DocBlockAnnotations($cache),
    new TypedProperties($cache),
    new NamespaceResolver($cache)

);
```
than this `(new JsonMapperFactory())->bestFit()`

It is slow because of individual cache object is used per middleware instead of shared one.

https://github.com/JsonMapper/JsonMapper/blob/97e4c6b6f5df2f1943e51cc8c76dbf1b58d84711/src/JsonMapperBuilder.php#L85-L98

May it be better to use a shared cache?

Pull request:
Preserve cache keys uniqueness within a single cache instance
Resolves #94 

Patch:
diff --git a/src/Handler/FactoryRegistry.php b/src/Handler/FactoryRegistry.php
index 9e63f7d..3ed2a3a 100644
--- a/src/Handler/FactoryRegistry.php
+++ b/src/Handler/FactoryRegistry.php
@@ -11,7 +11,7 @@ class FactoryRegistry
     /** @var callable[] */
     private $factories = [];
 
-    public static function WithNativePhpClassesAdded(): self
+    public static function withNativePhpClassesAdded(): self
     {
         $factory = new self();
         $factory->addFactory(\DateTime::class, static function (string $value) {
diff --git a/src/Handler/PropertyMapper.php b/src/Handler/PropertyMapper.php
index 5e5e467..82f3b81 100644
--- a/src/Handler/PropertyMapper.php
+++ b/src/Handler/PropertyMapper.php
@@ -25,7 +25,7 @@ class PropertyMapper
         FactoryRegistry $nonInstantiableTypeResolver = null
     ) {
         if ($classFactoryRegistry === null) {
-            $classFactoryRegistry = FactoryRegistry::WithNativePhpClassesAdded();
+            $classFactoryRegistry = FactoryRegistry::withNativePhpClassesAdded();
         }
 
         if ($nonInstantiableTypeResolver === null) {
@@ -250,7 +250,11 @@ class PropertyMapper
             $mapper->mapObject($value, $instance);
             return $instance;
         } catch (ClassFactoryException $e) {
-            throw new \RuntimeException("Unable to resolve un-instantiable {$type} as no factory was registered", 0, $e);
+            throw new \RuntimeException(
+                "Unable to resolve un-instantiable {$type} as no factory was registered",
+                0,
+                $e
+            );
         }
     }
 }
diff --git a/src/JsonMapperBuilder.php b/src/JsonMapperBuilder.php
index 0798c87..82410d3 100644
--- a/src/JsonMapperBuilder.php
+++ b/src/JsonMapperBuilder.php
@@ -67,34 +67,46 @@ class JsonMapperBuilder
         }
 
         $this->jsonMapperClassName = $jsonMapperClassName;
+
         return $this;
     }
 
     public function withPropertyMapper(PropertyMapper $propertyMapper): JsonMapperBuilder
     {
         $this->propertyMapper = $propertyMapper;
+
         return $this;
     }
 
     public function withDefaultCache(CacheInterface $defaultCache): JsonMapperBuilder
     {
         $this->defaultCache = $defaultCache;
+
         return $this;
     }
 
     public function withDocBlockAnnotationsMiddleware(?CacheInterface $cache = null): JsonMapperBuilder
     {
-        return $this->withMiddleware(new DocBlockAnnotations($cache ?: clone $this->defaultCache), DocBlockAnnotations::class);
+        return $this->withMiddleware(
+            new DocBlockAnnotations($cache ?: $this->defaultCache),
+            DocBlockAnnotations::class
+        );
     }
 
     public function withNamespaceResolverMiddleware(?CacheInterface $cache = null): JsonMapperBuilder
     {
-        return $this->withMiddleware(new NamespaceResolver($cache ?: clone $this->defaultCache), NamespaceResolver::class);
+        return $this->withMiddleware(
+            new NamespaceResolver($cache ?: $this->defaultCache),
+            NamespaceResolver::class
+        );
     }
 
     public function withTypedPropertiesMiddleware(?CacheInterface $cache = null): JsonMapperBuilder
     {
-        return $this->withMiddleware(new TypedProperties($cache ?: clone $this->defaultCache), TypedProperties::class);
+        return $this->withMiddleware(
+            new TypedProperties($cache ?: $this->defaultCache),
+            TypedProperties::class
+        );
     }
 
     public function withAttributesMiddleware(): JsonMapperBuilder
@@ -107,9 +119,14 @@ class JsonMapperBuilder
         return $this->withMiddleware(new Rename(...$mapping), Rename::class);
     }
 
-    public function withCaseConversionMiddleware(TextNotation $searchSeparator, TextNotation $replacementSeparator): JsonMapperBuilder
-    {
-        return $this->withMiddleware(new CaseConversion($searchSeparator, $replacementSeparator), CaseConversion::class);
+    public function withCaseConversionMiddleware(
+        TextNotation $searchSeparator,
+        TextNotation $replacementSeparator
+    ): JsonMapperBuilder {
+        return $this->withMiddleware(
+            new CaseConversion($searchSeparator, $replacementSeparator),
+            CaseConversion::class
+        );
     }
 
     public function withDebuggerMiddleware(LoggerInterface $logger): JsonMapperBuilder
@@ -117,8 +134,10 @@ class JsonMapperBuilder
         return $this->withMiddleware(new Debugger($logger), Debugger::class);
     }
 
-    public function withFinalCallbackMiddleware(callable $callback, bool $onlyApplyCallBackOnTopLevel = true): JsonMapperBuilder
-    {
+    public function withFinalCallbackMiddleware(
+        callable $callback,
+        bool $onlyApplyCallBackOnTopLevel = true
+    ): JsonMapperBuilder {
         return $this->withMiddleware(new FinalCallback($callback, $onlyApplyCallBackOnTopLevel), FinalCallback::class);
     }
 
diff --git a/src/Middleware/DocBlockAnnotations.php b/src/Middleware/DocBlockAnnotations.php
index a1d773c..6b05ba6 100644
--- a/src/Middleware/DocBlockAnnotations.php
+++ b/src/Middleware/DocBlockAnnotations.php
@@ -35,8 +35,9 @@ class DocBlockAnnotations extends AbstractMiddleware
 
     private function fetchPropertyMapForObject(ObjectWrapper $object): PropertyMap
     {
-        if ($this->cache->has($object->getName())) {
-            return $this->cache->get($object->getName());
+        $cacheKey = sprintf('%s::Cache::%s', __CLASS__, $object->getName());
+        if ($this->cache->has($cacheKey)) {
+            return $this->cache->get($cacheKey);
         }
 
         $properties = $object->getReflectedObject()->getProperties();
@@ -86,7 +87,7 @@ class DocBlockAnnotations extends AbstractMiddleware
             $intermediatePropertyMap->addProperty($property);
         }
 
-        $this->cache->set($object->getName(), $intermediatePropertyMap);
+        $this->cache->set($cacheKey, $intermediatePropertyMap);
 
         return $intermediatePropertyMap;
     }
diff --git a/src/Middleware/NamespaceResolver.php b/src/Middleware/NamespaceResolver.php
index 3284e88..9fcb2d3 100644
--- a/src/Middleware/NamespaceResolver.php
+++ b/src/Middleware/NamespaceResolver.php
@@ -17,7 +17,7 @@ use Psr\SimpleCache\CacheInterface;
 
 class NamespaceResolver extends AbstractMiddleware
 {
-    /** @var CacheInterface|null */
+    /** @var CacheInterface */
     private $cache;
 
     public function __construct(CacheInterface $cache = null)
diff --git a/src/Middleware/Rename/Rename.php b/src/Middleware/Rename/Rename.php
index 9a8c997..6f36bf5 100644
--- a/src/Middleware/Rename/Rename.php
+++ b/src/Middleware/Rename/Rename.php
@@ -24,9 +24,13 @@ class Rename extends AbstractMiddleware
         $this->mapping[] = new Mapping($class, $from, $to);
     }
 
-    public function handle(\stdClass $json, ObjectWrapper $object, PropertyMap $propertyMap, JsonMapperInterface $mapper): void
-    {
-        $mapping = array_filter($this->mapping, function ($map) use ($object) {
+    public function handle(
+        \stdClass $json,
+        ObjectWrapper $object,
+        PropertyMap $propertyMap,
+        JsonMapperInterface $mapper
+    ): void {
+        $mapping = array_filter($this->mapping, static function ($map) use ($object) {
             return $map->getClass() === get_class($object->getObject());
         });
         foreach ($mapping as $map) {
diff --git a/src/Middleware/TypedProperties.php b/src/Middleware/TypedProperties.php
index c6298d7..95c2e82 100644
--- a/src/Middleware/TypedProperties.php
+++ b/src/Middleware/TypedProperties.php
@@ -35,8 +35,9 @@ class TypedProperties extends AbstractMiddleware
 
     private function fetchPropertyMapForObject(ObjectWrapper $object): PropertyMap
     {
-        if ($this->cache->has($object->getName())) {
-            return $this->cache->get($object->getName());
+        $cacheKey = sprintf('%s::Cache::%s', __CLASS__, $object->getName());
+        if ($this->cache->has($cacheKey)) {
+            return $this->cache->get($cacheKey);
         }
 
         $reflectionProperties = $object->getReflectedObject()->getProperties();
@@ -85,7 +86,7 @@ class TypedProperties extends AbstractMiddleware
             }
         }
 
-        $this->cache->set($object->getName(), $intermediatePropertyMap);
+        $this->cache->set($cacheKey, $intermediatePropertyMap);
 
         return $intermediatePropertyMap;
     }


Test patch:
diff --git a/tests/Unit/Handler/FactoryRegistryTest.php b/tests/Unit/Handler/FactoryRegistryTest.php
index 8558062..540f6ac 100644
--- a/tests/Unit/Handler/FactoryRegistryTest.php
+++ b/tests/Unit/Handler/FactoryRegistryTest.php
@@ -92,7 +92,7 @@ class FactoryRegistryTest extends TestCase
      */
     public function testWithNativePhpClassesAddedAddsFactoriesForNativeClasses(): void
     {
-        $classFactoryRegistry = FactoryRegistry::WithNativePhpClassesAdded();
+        $classFactoryRegistry = FactoryRegistry::withNativePhpClassesAdded();
 
         self::assertTrue($classFactoryRegistry->hasFactory(\DateTime::class));
         self::assertTrue($classFactoryRegistry->hasFactory(\DateTimeImmutable::class));
diff --git a/tests/Unit/Handler/PropertyMapperTest.php b/tests/Unit/Handler/PropertyMapperTest.php
index 861b933..f57a05e 100644
--- a/tests/Unit/Handler/PropertyMapperTest.php
+++ b/tests/Unit/Handler/PropertyMapperTest.php
@@ -219,7 +219,7 @@ class PropertyMapperTest extends TestCase
         $json = (object) ['user' => (object) ['id' => 1234, 'name' => 'John Doe']];
         $object = new UserWithConstructorParent();
         $wrapped = new ObjectWrapper($object);
-        $classFactoryRegistry = FactoryRegistry::WithNativePhpClassesAdded();
+        $classFactoryRegistry = FactoryRegistry::withNativePhpClassesAdded();
         $classFactoryRegistry->addFactory(
             UserWithConstructor::class,
             static function ($params) {
@@ -253,7 +253,7 @@ class PropertyMapperTest extends TestCase
         ]];
         $object = new UserWithConstructorParent();
         $wrapped = new ObjectWrapper($object);
-        $classFactoryRegistry = FactoryRegistry::WithNativePhpClassesAdded();
+        $classFactoryRegistry = FactoryRegistry::withNativePhpClassesAdded();
         $classFactoryRegistry->addFactory(
             UserWithConstructor::class,
             static function ($params) {
@@ -291,7 +291,7 @@ class PropertyMapperTest extends TestCase
         ]];
         $object = new UserWithConstructorParent();
         $wrapped = new ObjectWrapper($object);
-        $classFactoryRegistry = FactoryRegistry::WithNativePhpClassesAdded();
+        $classFactoryRegistry = FactoryRegistry::withNativePhpClassesAdded();
         $classFactoryRegistry->addFactory(
             UserWithConstructor::class,
             static function ($params) {
diff --git a/tests/Unit/Middleware/DocBlockAnnotationsTest.php b/tests/Unit/Middleware/DocBlockAnnotationsTest.php
index 2b5fd8e..73d4ee4 100644
--- a/tests/Unit/Middleware/DocBlockAnnotationsTest.php
+++ b/tests/Unit/Middleware/DocBlockAnnotationsTest.php
@@ -12,6 +12,7 @@ use JsonMapper\Tests\Helpers\AssertThatPropertyTrait;
 use JsonMapper\Tests\Implementation\ComplexObject;
 use JsonMapper\ValueObjects\PropertyMap;
 use JsonMapper\Wrapper\ObjectWrapper;
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 use Psr\SimpleCache\CacheInterface;
 
@@ -119,8 +120,8 @@ class DocBlockAnnotationsTest extends TestCase
         $objectWrapper->method('getName')->willReturn(__METHOD__);
         $objectWrapper->expects(self::never())->method('getReflectedObject');
         $cache = $this->createMock(CacheInterface::class);
-        $cache->method('has')->with(__METHOD__)->willReturn(true);
-        $cache->method('get')->with(__METHOD__)->willReturn($propertyMap);
+        $cache->method('has')->with(Assert::stringContains(__METHOD__))->willReturn(true);
+        $cache->method('get')->with(Assert::stringContains(__METHOD__))->willReturn($propertyMap);
         $middleware = new DocBlockAnnotations($cache);
         $jsonMapper = $this->createMock(JsonMapperInterface::class);
 
diff --git a/tests/Unit/Middleware/TypedPropertiesTest.php b/tests/Unit/Middleware/TypedPropertiesTest.php
index c0aa068..b10d318 100644
--- a/tests/Unit/Middleware/TypedPropertiesTest.php
+++ b/tests/Unit/Middleware/TypedPropertiesTest.php
@@ -14,6 +14,7 @@ use JsonMapper\Tests\Implementation\Php80;
 use JsonMapper\Tests\Implementation\SimpleObject;
 use JsonMapper\ValueObjects\PropertyMap;
 use JsonMapper\Wrapper\ObjectWrapper;
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 use Psr\SimpleCache\CacheInterface;
 
@@ -98,8 +99,8 @@ class TypedPropertiesTest extends TestCase
         $objectWrapper->method('getName')->willReturn(__METHOD__);
         $objectWrapper->expects(self::never())->method('getReflectedObject');
         $cache = $this->createMock(CacheInterface::class);
-        $cache->method('has')->with(__METHOD__)->willReturn(true);
-        $cache->method('get')->with(__METHOD__)->willReturn($propertyMap);
+        $cache->method('has')->with(Assert::stringContains(__METHOD__))->willReturn(true);
+        $cache->method('get')->with(Assert::stringContains(__METHOD__))->willReturn($propertyMap);
         $middleware = new TypedProperties($cache);
         $jsonMapper = $this->createMock(JsonMapperInterface::class);