From 8d40fcaaac722916ab91fe43054cfa6a2a262367 Mon Sep 17 00:00:00 2001
From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com>
Date: Sat, 13 Jan 2024 22:54:33 +0700
Subject: [PATCH] refactor: Simplify object caching.

---
 src/MainTransformer/MainTransformer.php       |  6 ++++++
 src/MethodMapper/ClassMethodTransformer.php   |  4 ----
 src/ObjectCache/ObjectCache.php               | 21 ++++++++++---------
 src/Transformer/InheritanceMapTransformer.php |  7 -------
 tests/UnitTest/Model/ObjectCacheTest.php      |  3 ++-
 5 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/src/MainTransformer/MainTransformer.php b/src/MainTransformer/MainTransformer.php
index 08dc8497..32df022b 100644
--- a/src/MainTransformer/MainTransformer.php
+++ b/src/MainTransformer/MainTransformer.php
@@ -151,6 +151,12 @@ public function transform(
                 throw new TransformerReturnsUnexpectedValueException($targetType, $result);
             }
 
+            // if the target type is not null, cache it
+
+            if ($targetTypeForTransformer !== null) {
+                $objectCache->saveTarget($source, $targetTypeForTransformer, $result, true);
+            }
+
             return $result;
         }
 
diff --git a/src/MethodMapper/ClassMethodTransformer.php b/src/MethodMapper/ClassMethodTransformer.php
index 3a872097..c9729ecc 100644
--- a/src/MethodMapper/ClassMethodTransformer.php
+++ b/src/MethodMapper/ClassMethodTransformer.php
@@ -89,10 +89,6 @@ public function transform(
 
         $objectCache = $context->get(ObjectCache::class);
 
-        // save to object cache
-
-        $objectCache->saveTarget($source, $targetType, $target);
-
         return $result;
     }
 
diff --git a/src/ObjectCache/ObjectCache.php b/src/ObjectCache/ObjectCache.php
index 6e7f9e2f..5d614ca8 100644
--- a/src/ObjectCache/ObjectCache.php
+++ b/src/ObjectCache/ObjectCache.php
@@ -23,22 +23,22 @@
 final class ObjectCache
 {
     /**
-     * @var \WeakMap<object,\ArrayObject<string,object>>
+     * @var \SplObjectStorage<object,\ArrayObject<string,object>>
      */
-    private \WeakMap $cache;
+    private \SplObjectStorage $cache;
 
     /**
-     * @var \WeakMap<object,\ArrayObject<string,true>>
+     * @var \SplObjectStorage<object,\ArrayObject<string,true>>
      */
-    private \WeakMap $preCache;
+    private \SplObjectStorage $preCache;
 
     public function __construct(
         private TypeResolverInterface $typeResolver
     ) {
         /** @psalm-suppress MixedPropertyTypeCoercion */
-        $this->cache = new \WeakMap();
+        $this->cache = new \SplObjectStorage();
         /** @psalm-suppress MixedPropertyTypeCoercion */
-        $this->preCache = new \WeakMap();
+        $this->preCache = new \SplObjectStorage();
     }
 
     private function isBlacklisted(mixed $source): bool
@@ -83,7 +83,7 @@ public function preCache(mixed $source, Type $targetType): void
             $this->preCache[$source] = $arrayObject;
         }
 
-        $this->preCache->offsetGet($source)?->offsetSet($targetTypeString, true);
+        $this->preCache->offsetGet($source)->offsetSet($targetTypeString, true);
     }
 
     private function isPreCached(mixed $source, Type $targetType): bool
@@ -157,7 +157,8 @@ public function getTarget(mixed $source, Type $targetType): mixed
     public function saveTarget(
         mixed $source,
         Type $targetType,
-        mixed $target
+        mixed $target,
+        bool $addIfAlreadyExists = false
     ): void {
         if (!is_object($source) || !is_object($target)) {
             return;
@@ -169,7 +170,7 @@ public function saveTarget(
 
         $targetTypeString = $this->typeResolver->getTypeString($targetType);
 
-        if (isset($this->cache[$source][$targetTypeString])) {
+        if ($addIfAlreadyExists === false && $this->containsTarget($source, $targetType)) {
             throw new LogicException(sprintf(
                 'Target object for source object "%s" and target type "%s" already exists',
                 get_class($source),
@@ -183,7 +184,7 @@ public function saveTarget(
             $this->cache[$source] = $arrayObject;
         }
 
-        $this->cache->offsetGet($source)?->offsetSet($targetTypeString, $target);
+        $this->cache->offsetGet($source)->offsetSet($targetTypeString, $target);
         $this->removePrecache($source, $targetType);
     }
 }
diff --git a/src/Transformer/InheritanceMapTransformer.php b/src/Transformer/InheritanceMapTransformer.php
index 15ee1ebb..50dc745c 100644
--- a/src/Transformer/InheritanceMapTransformer.php
+++ b/src/Transformer/InheritanceMapTransformer.php
@@ -17,7 +17,6 @@
 use Rekalogika\Mapper\Context\Context;
 use Rekalogika\Mapper\Exception\InvalidArgumentException;
 use Rekalogika\Mapper\Exception\UnexpectedValueException;
-use Rekalogika\Mapper\ObjectCache\ObjectCache;
 use Rekalogika\Mapper\Transformer\Contracts\MainTransformerAwareInterface;
 use Rekalogika\Mapper\Transformer\Contracts\MainTransformerAwareTrait;
 use Rekalogika\Mapper\Transformer\Contracts\TransformerInterface;
@@ -111,12 +110,6 @@ public function transform(
             );
         }
 
-        # cache the result. we cache the abstract class/interface as the key.
-        # the concrete should be cached by whatever transformer that handles it.
-
-        $context->get(ObjectCache::class)
-            ->saveTarget($source, $targetType, $result);
-
         return $result;
     }
 
diff --git a/tests/UnitTest/Model/ObjectCacheTest.php b/tests/UnitTest/Model/ObjectCacheTest.php
index 6c671989..a625da08 100644
--- a/tests/UnitTest/Model/ObjectCacheTest.php
+++ b/tests/UnitTest/Model/ObjectCacheTest.php
@@ -14,6 +14,7 @@
 namespace Rekalogika\Mapper\Tests\UnitTest\Model;
 
 use PHPUnit\Framework\TestCase;
+use Rekalogika\Mapper\ObjectCache\Exception\CachedTargetObjectNotFoundException;
 use Rekalogika\Mapper\ObjectCache\ObjectCache;
 use Rekalogika\Mapper\TypeResolver\TypeResolver;
 use Rekalogika\Mapper\Util\TypeFactory;
@@ -34,7 +35,7 @@ public function testObjectCache(): void
         $this->assertTrue($objectCache->containsTarget($source, TypeFactory::int()));
         $this->assertSame($target, $objectCache->getTarget($source, TypeFactory::int()));
 
-        $this->expectException(\Rekalogika\Mapper\ObjectCache\Exception\CachedTargetObjectNotFoundException::class);
+        $this->expectException(CachedTargetObjectNotFoundException::class);
         $objectCache->getTarget($source, TypeFactory::float());
     }
 }