Skip to content

Commit 8083bfd

Browse files
joelwurtzjoelwurtz
authored
fix(constructor): better check for property exists when building constructor args (#231)
2 parents 9314013 + e8614f4 commit 8083bfd

File tree

8 files changed

+127
-53
lines changed

8 files changed

+127
-53
lines changed

‎src/Extractor/ReadAccessor.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public function getIsNullExpression(Expr\Variable $input): Expr
199199
*
200200
* isset($input['property_name']) && null === $input->property_name
201201
*/
202-
return new Expr\BinaryOp\LogicalAnd(new Expr\BooleanNot(new Expr\Isset_([new Expr\PropertyFetch($input, $this->accessor)])), new Expr\BinaryOp\Identical(new Expr\ConstFetch(new Name('null')), new Expr\PropertyFetch($input, $this->accessor)));
202+
return new Expr\BinaryOp\LogicalAnd(new Expr\BooleanNot(new Expr\Isset_([new Expr\ArrayDimFetch($input, new Scalar\String_($this->accessor))])), new Expr\BinaryOp\Identical(new Expr\ConstFetch(new Name('null')), new Expr\ArrayDimFetch($input, new Scalar\String_($this->accessor))));
203203
}
204204

205205
if (self::TYPE_SOURCE === $this->type) {

‎src/Generator/CreateTargetStatementsGenerator.php‎

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public function __construct(
3535
private DiscriminatorStatementsGenerator $discriminatorStatementsGeneratorSource,
3636
private DiscriminatorStatementsGenerator $discriminatorStatementsGeneratorTarget,
3737
private CachedReflectionStatementsGenerator $cachedReflectionStatementsGenerator,
38+
private PropertyConditionsGenerator $propertyConditionsGenerator,
3839
?Parser $parser = null,
3940
) {
4041
$this->parser = $parser ?? (new ParserFactory())->createForHostVersion();
@@ -161,12 +162,12 @@ private function constructorArguments(GeneratorMetadata $metadata): array
161162
* If source missing a constructor argument, check if there is a constructor argument in the context, otherwise we use the default value or throw exception.
162163
*
163164
* ```php
164-
* {transformation of value}
165-
* $constructarg = $value ?? (
166-
* MapperContext::hasConstructorArgument($context, $target, 'propertyName')
167-
* ? MapperContext::getConstructorArgument($context, $target, 'propertyName')
168-
* : {defaultValueExpr} // default value or throw exception
169-
* )
165+
*
166+
* if ($value not defined) {
167+
* $constructarg = MapperContext::hasConstructorArgument($context, $target, 'propertyName') ? MapperContext::getConstructorArgument($context, $target, 'propertyName') : {defaultValueExpr} // default value or throw exception
168+
* } else {
169+
* $constructarg = transformation of value
170+
* }
170171
* ```
171172
*
172173
* @return array{Stmt[], Arg, string}|array{null, null, null}
@@ -177,6 +178,12 @@ private function constructorArgument(GeneratorMetadata $metadata, PropertyMetada
177178
$constructVar = $variableRegistry->getVariableWithUniqueName('constructArg');
178179
$fieldValueExpr = $propertyMetadata->source->accessor?->getExpression($variableRegistry->getSourceInput());
179180

181+
$condition = $this->propertyConditionsGenerator->generate(
182+
$metadata,
183+
$propertyMetadata,
184+
true
185+
);
186+
180187
if (null === $fieldValueExpr) {
181188
if (!($propertyMetadata->transformer instanceof AllowNullValueTransformerInterface)) {
182189
return [null, null, null];
@@ -185,9 +192,6 @@ private function constructorArgument(GeneratorMetadata $metadata, PropertyMetada
185192
$fieldValueExpr = new Expr\ConstFetch(new Name('null'));
186193
}
187194

188-
/* Get extract and transform statements for this property */
189-
[$output, $propStatements] = $propertyMetadata->transformer->transform($fieldValueExpr, $constructVar, $propertyMetadata, $variableRegistry->getUniqueVariableScope(), $variableRegistry->getSourceInput());
190-
191195
$defaultValueExpr = new Expr\Throw_(
192196
new Expr\New_(new Name\FullyQualified(MissingConstructorArgumentsException::class), [
193197
new Arg(new Scalar\String_(sprintf('Cannot create an instance of "%s" from mapping data because its constructor requires the following parameters to be present : "$%s".', $metadata->mapperMetadata->target, $propertyMetadata->target->property))),
@@ -206,36 +210,42 @@ private function constructorArgument(GeneratorMetadata $metadata, PropertyMetada
206210
$defaultValueExpr = new Expr\ConstFetch(new Name('null'));
207211
}
208212

209-
if ($defaultValueExpr instanceof Expr\Array_) {
210-
// $constructarg = count($values) > 0 ? $values : {expression};
211-
$argumentAssignClosure = static fn (Expr $expr) => new Expr\Assign($constructVar, new Expr\Ternary(
212-
new Expr\BinaryOp\Greater(new Expr\FuncCall(new Name('count'), [new Arg($output)]), create_scalar_int(0)),
213-
$output,
214-
$expr,
215-
));
216-
} else {
217-
// $constructarg = $values ?? {expression};
218-
$argumentAssignClosure = static fn (Expr $expr) => new Expr\Assign($constructVar, new Expr\BinaryOp\Coalesce($output, $expr));
213+
/* Get extract and transform statements for this property */
214+
[$output, $propStatements] = $propertyMetadata->transformer->transform($fieldValueExpr, $constructVar, $propertyMetadata, $variableRegistry->getUniqueVariableScope(), $variableRegistry->getSourceInput());
215+
216+
if (!$condition) {
217+
return [
218+
[
219+
new Stmt\Expression(new Expr\Assign($constructVar, $output)),
220+
],
221+
new Arg($constructVar, name: new Identifier($parameter->getName())),
222+
$parameter->getName(),
223+
];
219224
}
220225

221226
return [
222227
[
223-
...$propStatements,
224-
new Stmt\Expression($argumentAssignClosure(
225-
new Expr\Ternary(
226-
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'hasConstructorArgument', [
227-
new Arg($variableRegistry->getContext()),
228-
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
229-
new Arg(new Scalar\String_($propertyMetadata->target->property)),
230-
]),
231-
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'getConstructorArgument', [
232-
new Arg($variableRegistry->getContext()),
233-
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
234-
new Arg(new Scalar\String_($propertyMetadata->target->property)),
235-
]),
236-
$defaultValueExpr,
237-
),
238-
)),
228+
new Stmt\If_($condition, [
229+
'stmts' => [
230+
...$propStatements,
231+
new Stmt\Expression(new Expr\Assign($constructVar, $output)),
232+
],
233+
'else' => new Stmt\Else_([
234+
new Stmt\Expression(new Expr\Assign($constructVar, new Expr\Ternary(
235+
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'hasConstructorArgument', [
236+
new Arg($variableRegistry->getContext()),
237+
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
238+
new Arg(new Scalar\String_($propertyMetadata->target->property)),
239+
]),
240+
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'getConstructorArgument', [
241+
new Arg($variableRegistry->getContext()),
242+
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
243+
new Arg(new Scalar\String_($propertyMetadata->target->property)),
244+
]),
245+
$defaultValueExpr,
246+
))),
247+
]),
248+
]),
239249
],
240250
new Arg($constructVar, name: new Identifier($parameter->getName())),
241251
$parameter->getName(),

‎src/Generator/MapMethodStatementsGenerator.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,16 @@ public function __construct(
3232
CachedReflectionStatementsGenerator $cachedReflectionStatementsGenerator,
3333
ExpressionLanguage $expressionLanguage,
3434
) {
35+
$propertyConditionGenerator = new PropertyConditionsGenerator($expressionLanguage);
36+
3537
$this->createObjectStatementsGenerator = new CreateTargetStatementsGenerator(
3638
$discriminatorStatementsGeneratorSource,
3739
$discriminatorStatementsGeneratorTarget,
3840
$cachedReflectionStatementsGenerator,
41+
$propertyConditionGenerator
3942
);
40-
$this->propertyStatementsGenerator = new PropertyStatementsGenerator($expressionLanguage);
43+
44+
$this->propertyStatementsGenerator = new PropertyStatementsGenerator($propertyConditionGenerator);
4145
}
4246

4347
/**

‎src/Generator/PropertyConditionsGenerator.php‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,30 @@ public function __construct(
3636
$this->parser = $parser ?? (new ParserFactory())->createForHostVersion();
3737
}
3838

39-
public function generate(GeneratorMetadata $metadata, PropertyMetadata $propertyMetadata): ?Expr
39+
public function generate(GeneratorMetadata $metadata, PropertyMetadata $propertyMetadata, bool $onlyExists = false): ?Expr
4040
{
4141
$conditions = [];
4242

4343
$conditions[] = $this->propertyExistsForStdClass($metadata, $propertyMetadata);
4444
$conditions[] = $this->propertyExistsForArray($metadata, $propertyMetadata);
45-
$conditions[] = $this->isAllowedAttribute($metadata, $propertyMetadata);
4645

47-
if (!$propertyMetadata->disableGroupsCheck) {
48-
$conditions[] = $this->groupsCheck($metadata->variableRegistry, $propertyMetadata->groups); // Property groups
46+
if (!$onlyExists) {
47+
$conditions[] = $this->isAllowedAttribute($metadata, $propertyMetadata);
4948

50-
if ($propertyMetadata->groups === null) {
51-
$conditions[] = $this->groupsCheck($metadata->variableRegistry, $propertyMetadata->source->groups); // Source groups
52-
$conditions[] = $this->groupsCheck($metadata->variableRegistry, $propertyMetadata->target->groups); // Target groups
49+
if (!$propertyMetadata->disableGroupsCheck) {
50+
$conditions[] = $this->groupsCheck($metadata->variableRegistry, $propertyMetadata->groups); // Property groups
51+
52+
if ($propertyMetadata->groups === null) {
53+
$conditions[] = $this->groupsCheck($metadata->variableRegistry, $propertyMetadata->source->groups); // Source groups
54+
$conditions[] = $this->groupsCheck($metadata->variableRegistry, $propertyMetadata->target->groups); // Target groups
55+
}
56+
57+
$conditions[] = $this->noGroupsCheck($metadata, $propertyMetadata);
5358
}
5459

55-
$conditions[] = $this->noGroupsCheck($metadata, $propertyMetadata);
60+
$conditions[] = $this->maxDepthCheck($metadata, $propertyMetadata);
5661
}
5762

58-
$conditions[] = $this->maxDepthCheck($metadata, $propertyMetadata);
5963
$conditions[] = $this->customCondition($metadata, $propertyMetadata);
6064

6165
$conditions = array_values(array_filter($conditions));

‎src/Generator/PropertyStatementsGenerator.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,15 @@
1212
use PhpParser\Node\Expr;
1313
use PhpParser\Node\Name;
1414
use PhpParser\Node\Stmt;
15-
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
1615

1716
/**
1817
* @internal
1918
*/
2019
final readonly class PropertyStatementsGenerator
2120
{
22-
private PropertyConditionsGenerator $propertyConditionsGenerator;
23-
2421
public function __construct(
25-
ExpressionLanguage $expressionLanguage
22+
private PropertyConditionsGenerator $propertyConditionsGenerator
2623
) {
27-
$this->propertyConditionsGenerator = new PropertyConditionsGenerator($expressionLanguage);
2824
}
2925

3026
/**

‎tests/AutoMapperBaseTest.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,12 @@ protected function buildAutoMapper(
3838
?ExpressionLanguageProvider $expressionLanguageProvider = null,
3939
EventDispatcherInterface $eventDispatcher = new EventDispatcher(),
4040
): AutoMapper {
41-
$fs = new Filesystem();
42-
$fs->remove(__DIR__ . '/cache/');
41+
$skipCacheRemove = $_SERVER['SKIP_CACHE_REMOVE'] ?? false;
42+
43+
if (!$skipCacheRemove) {
44+
$fs = new Filesystem();
45+
$fs->remove(__DIR__ . '/cache/');
46+
}
4347

4448
$configuration = new Configuration(
4549
classPrefix: $classPrefix,

‎tests/AutoMapperTest.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,43 @@ public function testConstructor(): void
478478
self::assertTrue($userDto->getConstructor());
479479
}
480480

481+
public function testConstructorAndRelationMissing(): void
482+
{
483+
$user = ['name' => 'foo'];
484+
$this->expectException(MissingConstructorArgumentsException::class);
485+
486+
/** @var Fixtures\UserConstructorDTOWithRelation $userDto */
487+
$userDto = $this->autoMapper->map($user, Fixtures\UserConstructorDTOWithRelation::class);
488+
}
489+
490+
public function testConstructorAndRelationMissing2(): void
491+
{
492+
$user = ['name' => 'foo', 'int' => ['foo' => 1]];
493+
/** @var Fixtures\UserConstructorDTOWithRelation $userDto */
494+
$userDto = $this->autoMapper->map($user, Fixtures\UserConstructorDTOWithRelation::class);
495+
496+
self::assertInstanceOf(Fixtures\UserConstructorDTOWithRelation::class, $userDto);
497+
self::assertSame(1, $userDto->int->foo);
498+
self::assertSame('foo', $userDto->name);
499+
self::assertSame(30, $userDto->age);
500+
}
501+
502+
public function testConstructorAndRelationMissingAndContext(): void
503+
{
504+
$user = ['name' => 'foo'];
505+
/** @var Fixtures\UserConstructorDTOWithRelation $userDto */
506+
$userDto = $this->autoMapper->map($user, Fixtures\UserConstructorDTOWithRelation::class, [
507+
MapperContext::CONSTRUCTOR_ARGUMENTS => [
508+
Fixtures\UserConstructorDTOWithRelation::class => ['int' => new Fixtures\IntDTO(1)],
509+
],
510+
]);
511+
512+
self::assertInstanceOf(Fixtures\UserConstructorDTOWithRelation::class, $userDto);
513+
self::assertSame(1, $userDto->int->foo);
514+
self::assertSame('foo', $userDto->name);
515+
self::assertSame(30, $userDto->age);
516+
}
517+
481518
public function testConstructorArrayArgumentFromContext(): void
482519
{
483520
$data = ['baz' => 'baz'];

‎tests/Fixtures/UserConstructorDTOWithRelation.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace AutoMapper\Tests\Fixtures;
6+
7+
class UserConstructorDTOWithRelation
8+
{
9+
public IntDTO $int;
10+
public string $name;
11+
public int $age;
12+
13+
public function __construct(IntDTO $int, string $name, int $age = 30)
14+
{
15+
$this->int = $int;
16+
$this->name = $name;
17+
$this->age = $age;
18+
}
19+
}

0 commit comments

Comments
 (0)