fix: fix list handling (#252)

* add test for array mapping * add test for array mapping * fix: fix list handling * fix --------- Co-authored-by: Vladyslav Yarysh <[email protected]>
rekalogika · Nov 12, 2024 · d117d4d · d117d4d
1 parent 3c40856
commit d117d4d
Show file tree

Hide file tree

Showing 11 changed files with 153 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## 1.13.4
+
+* fix: fix list handling
+
 ## 1.13.3
 
 * fix: rethrow our exceptions in `writeTargetProperty`, fix confusing error

diff --git a/src/Transformer/ArrayLikeMetadata/ArrayLikeMetadata.php b/src/Transformer/ArrayLikeMetadata/ArrayLikeMetadata.php
@@ -43,11 +43,9 @@ public function __construct(
         private array $targetMemberValueTypes,
         private bool $sourceMemberKeyCanBeInt,
         private bool $sourceMemberKeyCanBeString,
-        private bool $sourceMemberKeyCanBeIntOnly,
         private bool $sourceMemberKeyCanBeOtherThanIntOrString,
         private bool $targetMemberKeyCanBeInt,
         private bool $targetMemberKeyCanBeString,
-        private bool $targetMemberKeyCanBeIntOnly,
         private bool $targetMemberKeyCanBeOtherThanIntOrString,
         private bool $targetMemberValueIsUntyped,
     ) {}
@@ -128,11 +126,6 @@ public function targetMemberKeyCanBeString(): bool
         return $this->targetMemberKeyCanBeString;
     }
 
-    public function targetMemberKeyCanBeIntOnly(): bool
-    {
-        return $this->targetMemberKeyCanBeIntOnly;
-    }
-
     public function targetMemberKeyCanBeOtherThanIntOrString(): bool
     {
         return $this->targetMemberKeyCanBeOtherThanIntOrString;
@@ -163,11 +156,6 @@ public function sourceMemberKeyCanBeString(): bool
         return $this->sourceMemberKeyCanBeString;
     }
 
-    public function sourceMemberKeyCanBeIntOnly(): bool
-    {
-        return $this->sourceMemberKeyCanBeIntOnly;
-    }
-
     public function sourceMemberKeyCanBeOtherThanIntOrString(): bool
     {
         return $this->sourceMemberKeyCanBeOtherThanIntOrString;

diff --git a/src/Transformer/ArrayLikeMetadata/Implementation/ArrayLikeMetadataFactory.php b/src/Transformer/ArrayLikeMetadata/Implementation/ArrayLikeMetadataFactory.php
@@ -70,7 +70,7 @@ public function createArrayLikeMetadata(
         $sourceMemberKeyTypeCanBeString = false;
         $sourceMemberKeyTypeCanBeOtherThanIntOrString = false;
 
-        foreach ($sourceType->getCollectionKeyTypes() as $sourceMemberKeyType) {
+        foreach ($sourceMemberKeyTypes as $sourceMemberKeyType) {
             if (TypeCheck::isInt($sourceMemberKeyType)) {
                 $sourceMemberKeyTypeCanBeInt = true;
             } elseif (TypeCheck::isString($sourceMemberKeyType)) {
@@ -123,11 +123,9 @@ public function createArrayLikeMetadata(
             targetMemberValueTypes: $targetMemberValueTypes,
             sourceMemberKeyCanBeInt: $sourceMemberKeyTypeCanBeInt,
             sourceMemberKeyCanBeString: $sourceMemberKeyTypeCanBeString,
-            sourceMemberKeyCanBeIntOnly: $sourceMemberKeyTypeCanBeInt && !$sourceMemberKeyTypeCanBeString,
             sourceMemberKeyCanBeOtherThanIntOrString: $sourceMemberKeyTypeCanBeOtherThanIntOrString,
             targetMemberKeyCanBeInt: $targetMemberKeyTypeCanBeInt,
             targetMemberKeyCanBeString: $targetMemberKeyTypeCanBeString,
-            targetMemberKeyCanBeIntOnly: $targetMemberKeyTypeCanBeInt && !$targetMemberKeyTypeCanBeString,
             targetMemberKeyCanBeOtherThanIntOrString: $targetMemberKeyTypeCanBeOtherThanIntOrString,
             targetMemberValueIsUntyped: $targetMemberValueIsUntyped,
         );

diff --git a/src/Transformer/Implementation/TraversableToArrayAccessTransformer.php b/src/Transformer/Implementation/TraversableToArrayAccessTransformer.php
@@ -184,19 +184,25 @@ private function eagerTransform(
             foreach ($transformed as $value) {
                 $target = $target->add($value);
 
-                if (\is_array($values)) {
+                if (\is_array($values)) { // allows delete
                     $values[] = $value;
                 }
             }
         } else {
             foreach ($transformed as $key => $value) {
                 if ($key === null) {
-                    $target[] = $value;
+                    if (\is_array($target)) {
+                        if (!\in_array($value, $target, true)) {
+                            $target[] = $value;
+                        }
+                    } else {
+                        $target[] = $value;
+                    }
                 } else {
                     $target[$key] = $value;
                 }
 
-                if (\is_array($values)) {
+                if (\is_array($values)) { // allows delete
                     $values[] = $value;
                 }
             }

diff --git a/src/Transformer/Model/LazyArray.php b/src/Transformer/Model/LazyArray.php
@@ -84,6 +84,7 @@ public function offsetGet(mixed $offset): mixed
         [$key, $value] = $this->transformMember(
             sourceMemberKey: $offset,
             sourceMemberValue: $this->source[$offset],
+            targetIsList: false,
             metadata: $this->metadata,
             context: $this->context,
         );
@@ -140,6 +141,7 @@ public function getIterator(): \Traversable
             [$key, $value] = $this->transformMember(
                 sourceMemberKey: $sourceMemberKey,
                 sourceMemberValue: $sourceMemberValue,
+                targetIsList: false,
                 metadata: $this->metadata,
                 context: $this->context,
             );

diff --git a/src/Transformer/Model/LazyList.php b/src/Transformer/Model/LazyList.php
@@ -102,6 +102,7 @@ public function getIterator(): \Traversable
             [, $value] = $this->transformMember(
                 sourceMemberKey: $sourceMemberKey,
                 sourceMemberValue: $sourceMemberValue,
+                targetIsList: true,
                 metadata: $this->metadata,
                 context: $this->context,
             );

diff --git a/src/Transformer/Trait/ArrayLikeTransformerTrait.php b/src/Transformer/Trait/ArrayLikeTransformerTrait.php
@@ -15,6 +15,7 @@
 
 use Rekalogika\Mapper\Context\Context;
 use Rekalogika\Mapper\Transformer\ArrayLikeMetadata\ArrayLikeMetadata;
+use Rekalogika\Mapper\Transformer\Model\AdderRemoverProxy;
 use Rekalogika\Mapper\Transformer\Model\SplObjectStorageWrapper;
 use Rekalogika\Mapper\Util\TypeCheck;
 use Symfony\Component\PropertyInfo\Type;
@@ -38,6 +39,14 @@ private function transformTraversableSource(
             $source = new SplObjectStorageWrapper($source);
         }
 
+        $targetIsList =
+            (
+                \is_array($target)
+                && $target !== []
+                && array_is_list($target)
+            )
+            || $target instanceof AdderRemoverProxy;
+
         $i = 0;
 
         /**
@@ -53,6 +62,7 @@ private function transformTraversableSource(
                 counter: $i,
                 sourceMemberKey: $sourceMemberKey,
                 sourceMemberValue: $sourceMemberValue,
+                targetIsList: $targetIsList,
                 target: $target,
                 metadata: $metadata,
                 context: $context,
@@ -71,6 +81,7 @@ private function transformTraversableSource(
     private function transformMember(
         mixed $sourceMemberKey,
         mixed $sourceMemberValue,
+        bool $targetIsList,
         ArrayLikeMetadata $metadata,
         Context $context,
         null|\ArrayAccess|array $target = null,
@@ -82,19 +93,19 @@ private function transformMember(
         if (\is_string($sourceMemberKey)) {
             // if the key is a string
 
-            if ($metadata->targetMemberKeyCanBeIntOnly()) {
+            if ($metadata->targetMemberKeyCanBeString()) {
+                // if target has string key type, we use the source key as
+                // the target key
+
+                $targetMemberKey = $sourceMemberKey;
+                $path = \sprintf('[%s]', $sourceMemberKey);
+            } elseif ($targetIsList || $metadata->targetMemberKeyCanBeInt()) {
                 // if target has int key type but the source has string key
                 // type, we discard the source key & use null key (i.e.
                 // $target[] = $value)
 
                 $targetMemberKey = null;
                 $path = \sprintf('[%d]', $counter ?? -1);
-            } elseif ($metadata->targetMemberKeyCanBeString()) {
-                // if target has string key type, we use the source key as
-                // the target key and let PHP cast it to string
-
-                $targetMemberKey = $sourceMemberKey;
-                $path = \sprintf('[%s]', $sourceMemberKey);
             } else {
                 // otherwise, the target must be non-int & non-string, so we
                 // delegate the transformation to the main transformer
@@ -119,15 +130,25 @@ private function transformMember(
             // if the key is an integer
 
             if (
-                $metadata->targetMemberKeyCanBeInt()
-                || $metadata->targetMemberKeyCanBeString()
+                $targetIsList
             ) {
-                // if the target has int or string key type, we use the
-                // source key as the target key, and let PHP cast it if
-                // needed
+                // if the target is a list, we don't use the source key as the
+                // target key
+
+                $targetMemberKey = null;
+                $path = \sprintf('[%d]', $counter ?? -1);
+            } elseif ($metadata->targetMemberKeyCanBeInt()) {
+                // if the target has int key type, we use the source key as the
+                // target key
 
                 $targetMemberKey = $sourceMemberKey;
                 $path = \sprintf('[%s]', $sourceMemberKey);
+            } elseif ($metadata->targetMemberKeyCanBeString()) {
+                // if the target has string key type, we cast source key to
+                // string
+
+                $targetMemberKey = (string) $sourceMemberKey;
+                $path = \sprintf('[%s]', $sourceMemberKey);
             } else {
                 // otherwise, the target must be non-int & non-string, so we
                 // delegate the transformation to the main transformer
@@ -188,7 +209,7 @@ private function transformMember(
         // Get the existing member value from the target
 
         try {
-            if ($metadata->targetMemberKeyCanBeIntOnly()) {
+            if ($targetIsList) {
                 $targetMemberValue = null;
             } elseif ($target !== null && $targetMemberKey !== null) {
                 /**

diff --git a/tests/config/rekalogika-mapper/generated-mappings.php b/tests/config/rekalogika-mapper/generated-mappings.php
@@ -260,6 +260,18 @@
         target: \Rekalogika\Mapper\Tests\Fixtures\ArrayLikeDto\ObjectWithTraversablePropertyDto::class
     );
 
+    $mappingCollection->addObjectMapping(
+        // tests/src/IntegrationTest/ArrayToArrayTest.php on line 43
+        source: \Rekalogika\Mapper\Tests\Fixtures\Array\ObjectWithList::class,
+        target: \Rekalogika\Mapper\Tests\Fixtures\Array\ObjectWithList::class
+    );
+
+    $mappingCollection->addObjectMapping(
+        // tests/src/IntegrationTest/ArrayToArrayTest.php on line 30
+        source: \Rekalogika\Mapper\Tests\Fixtures\Array\ObjectWithListWithAllowDelete::class,
+        target: \Rekalogika\Mapper\Tests\Fixtures\Array\ObjectWithListWithAllowDelete::class
+    );
+
     $mappingCollection->addObjectMapping(
         // tests/src/IntegrationTest/AttributeTest.php on line 42
         source: \Rekalogika\Mapper\Tests\Fixtures\Attribute\SomeObject::class,

diff --git a/tests/src/Fixtures/Array/ObjectWithList.php b/tests/src/Fixtures/Array/ObjectWithList.php
@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of rekalogika/mapper package.
+ *
+ * (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
+ *
+ * For the full copyright and license information, please view the LICENSE file
+ * that was distributed with this source code.
+ */
+
+namespace Rekalogika\Mapper\Tests\Fixtures\Array;
+
+class ObjectWithList
+{
+    /** @var array<mixed,mixed> */
+    public array $array = [];
+}
diff --git a/tests/src/Fixtures/Array/ObjectWithListWithAllowDelete.php b/tests/src/Fixtures/Array/ObjectWithListWithAllowDelete.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of rekalogika/mapper package.
+ *
+ * (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
+ *
+ * For the full copyright and license information, please view the LICENSE file
+ * that was distributed with this source code.
+ */
+
+namespace Rekalogika\Mapper\Tests\Fixtures\Array;
+
+use Rekalogika\Mapper\Attribute\AllowDelete;
+
+class ObjectWithListWithAllowDelete
+{
+    /** @var array<mixed,mixed> */
+    #[AllowDelete]
+    public array $array = [];
+}
diff --git a/tests/src/IntegrationTest/ArrayToArrayTest.php b/tests/src/IntegrationTest/ArrayToArrayTest.php
@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of rekalogika/mapper package.
+ *
+ * (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
+ *
+ * For the full copyright and license information, please view the LICENSE file
+ * that was distributed with this source code.
+ */
+
+namespace Rekalogika\Mapper\Tests\IntegrationTest;
+
+use Rekalogika\Mapper\Tests\Common\FrameworkTestCase;
+use Rekalogika\Mapper\Tests\Fixtures\Array\ObjectWithList;
+use Rekalogika\Mapper\Tests\Fixtures\Array\ObjectWithListWithAllowDelete;
+
+class ArrayToArrayTest extends FrameworkTestCase
+{
+    public function testListToListWithAllowDelete(): void
+    {
+        $target = new ObjectWithListWithAllowDelete();
+        $target->array = ['foo', 'bar'];
+
+        $source = new ObjectWithListWithAllowDelete();
+        $source->array = ['baz'];
+
+        $this->mapper->map($source, $target);
+
+        $this->assertEquals(['baz'], $target->array);
+    }
+
+    public function testListToListWithoutAllowDelete(): void
+    {
+        $target = new ObjectWithList();
+        $target->array = ['foo', 'bar'];
+
+        $source = new ObjectWithList();
+        $source->array = ['baz'];
+
+        $this->mapper->map($source, $target);
+
+        $this->assertEquals(['foo', 'bar', 'baz'], $target->array);
+    }
+}