From d117d4de8cb466d5c39f77ad1c7c880a4a5b5790 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Wed, 13 Nov 2024 00:37:30 +0700 Subject: [PATCH] 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 --- CHANGELOG.md | 4 ++ .../ArrayLikeMetadata/ArrayLikeMetadata.php | 12 ----- .../ArrayLikeMetadataFactory.php | 4 +- .../TraversableToArrayAccessTransformer.php | 12 +++-- src/Transformer/Model/LazyArray.php | 2 + src/Transformer/Model/LazyList.php | 1 + .../Trait/ArrayLikeTransformerTrait.php | 47 ++++++++++++++----- .../rekalogika-mapper/generated-mappings.php | 12 +++++ tests/src/Fixtures/Array/ObjectWithList.php | 20 ++++++++ .../Array/ObjectWithListWithAllowDelete.php | 23 +++++++++ .../src/IntegrationTest/ArrayToArrayTest.php | 47 +++++++++++++++++++ 11 files changed, 153 insertions(+), 31 deletions(-) create mode 100644 tests/src/Fixtures/Array/ObjectWithList.php create mode 100644 tests/src/Fixtures/Array/ObjectWithListWithAllowDelete.php create mode 100644 tests/src/IntegrationTest/ArrayToArrayTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 1888e903..d041b373 100644 --- 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 index 159c601b..f5478e62 100644 --- 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 index a39bef71..36ad18aa 100644 --- 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 index dc757a40..3aad8710 100644 --- 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 index 7fa0a3df..ae96ef74 100644 --- 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 index 811b4b55..d5d0d9e3 100644 --- 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 index 75f0812b..5830ebd4 100644 --- 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 index 4da7fece..310f22fb 100644 --- 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 new file mode 100644 index 00000000..052f926c --- /dev/null +++ b/tests/src/Fixtures/Array/ObjectWithList.php @@ -0,0 +1,20 @@ + + * + * 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 */ + public array $array = []; +} diff --git a/tests/src/Fixtures/Array/ObjectWithListWithAllowDelete.php b/tests/src/Fixtures/Array/ObjectWithListWithAllowDelete.php new file mode 100644 index 00000000..5cb19e39 --- /dev/null +++ b/tests/src/Fixtures/Array/ObjectWithListWithAllowDelete.php @@ -0,0 +1,23 @@ + + * + * 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 */ + #[AllowDelete] + public array $array = []; +} diff --git a/tests/src/IntegrationTest/ArrayToArrayTest.php b/tests/src/IntegrationTest/ArrayToArrayTest.php new file mode 100644 index 00000000..7621550f --- /dev/null +++ b/tests/src/IntegrationTest/ArrayToArrayTest.php @@ -0,0 +1,47 @@ + + * + * 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); + } +}