Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Extractor/MappingExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ public function getReadAccessor(string $class, string $property, bool $allowExtr
public function getWriteMutator(string $source, string $target, string $property, array $context = [], bool $allowExtraProperties = false): ?WriteMutator
{
$writeInfo = $this->writeInfoExtractor->getWriteInfo($target, $property, $context);
$removeMethodName = null;

if (null === $writeInfo || PropertyWriteInfo::TYPE_NONE === $writeInfo->getType()) {
if ('array' === $target) {
Expand Down Expand Up @@ -147,13 +148,16 @@ public function getWriteMutator(string $source, string $target, string $property

if (PropertyWriteInfo::TYPE_ADDER_AND_REMOVER === $writeInfo->getType()) {
$type = WriteMutator::TYPE_ADDER_AND_REMOVER;
$removeMethodName = $writeInfo->getRemoverInfo()->getName();
$writeInfo = $writeInfo->getAdderInfo();
}

return new WriteMutator(
$type,
$writeInfo->getName(),
PropertyReadInfo::VISIBILITY_PUBLIC !== $writeInfo->getVisibility()
PropertyReadInfo::VISIBILITY_PUBLIC !== $writeInfo->getVisibility(),
null,
$removeMethodName,
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Extractor/ReadAccessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function __construct(
*
* @throws CompileException
*/
public function getExpression(Expr\Variable $input): Expr
public function getExpression(Expr $input): Expr
{
if (self::TYPE_METHOD === $this->type) {
$methodCallArguments = [];
Expand Down
17 changes: 17 additions & 0 deletions src/Extractor/WriteMutator.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public function __construct(
private readonly string $property,
private readonly bool $private = false,
public readonly ?\ReflectionParameter $parameter = null,
private readonly ?string $removeMethodName = null,
) {
}

Expand Down Expand Up @@ -100,6 +101,22 @@ public function getExpression(Expr $output, Expr $value, bool $byRef = false): E
throw new CompileException('Invalid accessor for write expression');
}

public function getRemoveExpression(Expr $object, Expr $value): ?Expr
{
if (self::TYPE_ADDER_AND_REMOVER === $this->type && $this->removeMethodName) {
/*
* Create method call expression to remove value
*
* $object->removeMethodName($value);
*/
return new Expr\MethodCall($object, $this->removeMethodName, [
new Arg($value),
]);
}

return null;
}

/**
* Get AST expression for binding closure when dealing with private property.
*/
Expand Down
11 changes: 11 additions & 0 deletions src/Transformer/AbstractArrayTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,17 @@ public function transform(Expr $input, Expr $target, PropertyMetadata $propertyM
*
* $target->add($output);
*/
$loopRemoveValueVar = new Expr\Variable($uniqueVariableScope->getUniqueName('removeValue'));
$removeExpr = $propertyMapping->target->writeMutator->getRemoveExpression($target, $loopRemoveValueVar);

if ($propertyMapping->target->readAccessor !== null && $removeExpr !== null) {
$statements[] = new Stmt\Foreach_($propertyMapping->target->readAccessor->getExpression($target), $loopRemoveValueVar, [
'stmts' => [
new Stmt\Expression($removeExpr),
],
]);
}

$mappedValueVar = new Expr\Variable($uniqueVariableScope->getUniqueName('mappedValue'));
$itemStatements[] = new Stmt\Expression(new Expr\Assign($mappedValueVar, $output));
$itemStatements[] = new Stmt\If_(new Expr\BinaryOp\NotIdentical(new Expr\ConstFetch(new Name('null')), $mappedValueVar), [
Expand Down
5 changes: 5 additions & 0 deletions src/Transformer/DoctrineCollectionTransformerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace AutoMapper\Transformer;

use AutoMapper\Extractor\WriteMutator;
use AutoMapper\Metadata\MapperMetadata;
use AutoMapper\Metadata\SourcePropertyMetadata;
use AutoMapper\Metadata\TargetPropertyMetadata;
Expand Down Expand Up @@ -40,6 +41,10 @@ protected function createTransformer(Type $sourceType, Type $targetType, SourceP
$subItemTransformer->deepTargetToPopulate = false;
}

if ($target->writeMutator?->type === WriteMutator::TYPE_ADDER_AND_REMOVER) {
return new ArrayTransformer($subItemTransformer);
}

return new ArrayToDoctrineCollectionTransformer($subItemTransformer);
}

Expand Down
12 changes: 5 additions & 7 deletions tests/AutoMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -839,13 +839,11 @@ public function testAdderAndRemoverWithInstance(): void
$this->autoMapper->map($petOwnerAsArray, $petOwner);

self::assertIsArray($petOwner->getPets());
self::assertCount(3, $petOwner->getPets());
self::assertSame('Nemo', $petOwner->getPets()[0]->name);
self::assertSame('fish', $petOwner->getPets()[0]->type);
self::assertSame('Félix', $petOwner->getPets()[1]->name);
self::assertSame('cat', $petOwner->getPets()[1]->type);
self::assertSame('Coco', $petOwner->getPets()[2]->name);
self::assertSame('dog', $petOwner->getPets()[2]->type);
self::assertCount(2, $petOwner->getPets());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

poke @Korbeil is this acceptable ? should we consider this PR as a BC break given that i had to change that or do we consider this a bug ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will merge this, we can always revert before release, i need this for another PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think it's fine since it's kinda a bug that we are adding without removing nowadays.

self::assertSame('Félix', $petOwner->getPets()[0]->name);
self::assertSame('cat', $petOwner->getPets()[0]->type);
self::assertSame('Coco', $petOwner->getPets()[1]->name);
self::assertSame('dog', $petOwner->getPets()[1]->type);
}

public function testAdderAndRemoverWithNull(): void
Expand Down
7 changes: 7 additions & 0 deletions tests/AutoMapperTest/ArrayConsistency/expected.to.data
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
AutoMapper\Tests\AutoMapperTest\ArrayConsistency\To {
+values: [
1
2
3
]
}
7 changes: 7 additions & 0 deletions tests/AutoMapperTest/ArrayConsistency/expected.toAdder.data
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
AutoMapper\Tests\AutoMapperTest\ArrayConsistency\ToAdder {
-values: [
1
2
3
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
AutoMapper\Tests\AutoMapperTest\ArrayConsistency\ToAdderCollection {
-values: Doctrine\Common\Collections\ArrayCollection {
-elements: [
3 => 1
4 => 2
5 => 3
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
AutoMapper\Tests\AutoMapperTest\ArrayConsistency\ToCollection {
+values: Doctrine\Common\Collections\ArrayCollection {
-elements: [
1
2
3
]
}
}
112 changes: 112 additions & 0 deletions tests/AutoMapperTest/ArrayConsistency/map.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php

declare(strict_types=1);

namespace AutoMapper\Tests\AutoMapperTest\ArrayConsistency;

use AutoMapper\Tests\AutoMapperBuilder;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;

class From
{
public array $values;
}

class To
{
public array $values;
}

class ToAdder
{
private array $values;

public function addValue(mixed $value): void
{
$this->values[] = $value;
}

public function removeValue(mixed $value): void
{
foreach ($this->values as $key => $existing) {
if ($existing === $value) {
unset($this->values[$key]);
$this->values = array_values($this->values);
}
}
}

public function getValues(): array
{
return $this->values;
}
}

class ToCollection
{
public ArrayCollection $values;

public function __construct()
{
$this->values = new ArrayCollection();
}
}

class ToAdderCollection
{
private Collection $values;

public function __construct()
{
$this->values = new ArrayCollection();
}

public function addValue(mixed $value): void
{
$this->values->add($value);
}

public function removeValue(mixed $value): void
{
$this->values->removeElement($value);
}

public function getValues(): Collection
{
return $this->values;
}
}

return (function () {
$autoMapper = AutoMapperBuilder::buildAutoMapper();

$from = new From();
$from->values = [1, 2, 3];

$to = new To();
$to->values = [4, 5, 6];

$toAdder = new ToAdder();
$toAdder->addValue(4);
$toAdder->addValue(5);
$toAdder->addValue(6);

$toAdderCollection = new ToAdderCollection();
$toAdderCollection->addValue(4);
$toAdderCollection->addValue(5);
$toAdderCollection->addValue(6);

$toCollection = new ToCollection();
$toCollection->values->add(4);
$toCollection->values->add(5);
$toCollection->values->add(6);

yield 'to' => $autoMapper->map($from, $to);

yield 'toAdder' => $autoMapper->map($from, $toAdder);

yield 'toAdderCollection' => $autoMapper->map($from, $toAdderCollection);

yield 'toCollection' => $autoMapper->map($from, $toCollection);
})();
1 change: 1 addition & 0 deletions tests/Fixtures/PetOwner.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public function removePet(Pet $pet): void

if ($index !== false) {
unset($this->pets[$index]);
$this->pets = array_values($this->pets);
}
}
}