Skip to content

Commit

Permalink
feat(generator): allow skipping uninitialized property when skipping …
Browse files Browse the repository at this point in the history
…null values
  • Loading branch information
joelwurtz committed Mar 8, 2024
1 parent cae8b2b commit 53fc76f
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 21 deletions.
90 changes: 90 additions & 0 deletions src/Extractor/ReadAccessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,64 @@ public function getExpression(Expr\Variable $input): Expr
throw new CompileException('Invalid accessor for read expression');
}

public function getIsNullExpression(Expr\Variable $input): Expr
{
if (self::TYPE_METHOD === $this->type) {
$methodCallExpr = $this->getExpression($input);

/*
* null !== $methodCallExpr
*/
return new Expr\BinaryOp\Identical(
new Expr\ConstFetch(new Name('null')),
$methodCallExpr,
);
}

if (self::TYPE_PROPERTY === $this->type) {
if ($this->private) {
/*
* When the property is private we use the extract callback that can read this value
*
* @see \AutoMapper\Extractor\ReadAccessor::getExtractIsNullCallback()
*
* $this->extractIsNullCallbacks['property_name']($input)
*/
return new Expr\FuncCall(
new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractIsNullCallbacks'), new Scalar\String_($this->accessor)),
[
new Arg($input),
]
);
}

/*
* Use the property fetch to read the value
*
* isset($input->property_name)
*/
return new Expr\Isset_([new Expr\PropertyFetch($input, $this->accessor)]);
}

if (self::TYPE_ARRAY_DIMENSION === $this->type) {
/*
* Use the array dim fetch to read the value
*
* isset($input['property_name'])
*/
return new Expr\Isset_([new Expr\ArrayDimFetch($input, new Scalar\String_($this->accessor))]);
}

if (self::TYPE_SOURCE === $this->type) {
return new Expr\BinaryOp\Identical(
new Expr\ConstFetch(new Name('null')),
$input,
);
}

throw new CompileException('Invalid accessor for read expression');
}

/**
* Get AST expression for binding closure when dealing with a private property.
*/
Expand Down Expand Up @@ -193,4 +251,36 @@ public function getExtractCallback(string $className): ?Expr
new Arg(new Scalar\String_($className)),
]);
}

/**
* Get AST expression for binding closure when dealing with a private property.
*/
public function getExtractIsNullCallback(string $className): ?Expr
{
if ($this->type !== self::TYPE_PROPERTY || !$this->private) {
return null;
}

/*
* Create extract is null callback for this accessor
*
* \Closure::bind(function ($object) {
* return isset($object->property_name);
* }, null, $className)
*/
return new Expr\StaticCall(new Name\FullyQualified(\Closure::class), 'bind', [
new Arg(
new Expr\Closure([
'params' => [
new Param(new Expr\Variable('object')),
],
'stmts' => [
new Stmt\Return_(new Expr\Isset_([new Expr\PropertyFetch(new Expr\Variable('object'), $this->accessor)])),
],
])
),
new Arg(new Expr\ConstFetch(new Name('null'))),
new Arg(new Scalar\String_($className)),
]);
}
}
3 changes: 3 additions & 0 deletions src/GeneratedMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ abstract class GeneratedMapper implements MapperInterface

protected $extractCallbacks = [];

/** @var Callable[] */
protected array $extractIsNullCallbacks = [];

protected $cachedTarget;

protected $circularReferenceHandler;
Expand Down
25 changes: 25 additions & 0 deletions src/Generator/MapperConstructorGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public function getStatements(MapperGeneratorMetadataInterface $mapperMetadata):

foreach ($mapperMetadata->getPropertiesMapping() as $propertyMapping) {
$constructStatements[] = $this->extractCallbackForProperty($propertyMapping);
$constructStatements[] = $this->extractIsNullCallbackForProperty($propertyMapping);
$constructStatements[] = $this->hydrateCallbackForProperty($propertyMapping);
}

Expand Down Expand Up @@ -62,6 +63,30 @@ private function extractCallbackForProperty(PropertyMapping $propertyMapping): S
));
}

/**
* Add read callback to the constructor of the generated mapper.
*
* ```php
* $this->extractIsNullCallbacks['propertyName'] = $extractIsNullCallback;
* ```
*/
private function extractIsNullCallbackForProperty(PropertyMapping $propertyMapping): Stmt\Expression|null
{
$mapperMetadata = $propertyMapping->mapperMetadata;

$extractNullCallback = $propertyMapping->readAccessor?->getExtractIsNullCallback($mapperMetadata->getSource());

if (!$extractNullCallback) {
return null;
}

return new Stmt\Expression(
new Expr\Assign(
new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractIsNullCallbacks'), new Scalar\String_($propertyMapping->property)),
$extractNullCallback
));
}

/**
* Add hydrate callback to the constructor of the generated mapper.
*
Expand Down
8 changes: 1 addition & 7 deletions src/Generator/PropertyConditionsGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,10 @@ private function isAllowedAttribute(PropertyMapping $propertyMapping): Expr|null

$variableRegistry = $mapperMetadata->getVariableRegistry();

/** Create expression on how to read the value from the source */
$sourcePropertyAccessor = new Expr\Assign(
$variableRegistry->getFieldValueVariable($propertyMapping),
$propertyMapping->readAccessor->getExpression($variableRegistry->getSourceInput())
);

return new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'isAllowedAttribute', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($propertyMapping->property)),
new Arg($sourcePropertyAccessor),
new Arg($propertyMapping->readAccessor->getIsNullExpression($variableRegistry->getSourceInput())),
]);
}

Expand Down
9 changes: 7 additions & 2 deletions src/Generator/PropertyStatementsGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use AutoMapper\Extractor\WriteMutator;
use AutoMapper\Transformer\AssignedByReferenceTransformerInterface;
use PhpParser\Node\Stmt;
use PhpParser\Node\Expr;

/**
* @internal
Expand All @@ -35,12 +36,16 @@ public function generate(PropertyMapping $propertyMapping): array
}

$variableRegistry = $mapperMetadata->getVariableRegistry();

$fieldValueVariable = $variableRegistry->getFieldValueVariable($propertyMapping);

if ($propertyMapping->readAccessor) {
$fieldValueVariable = $propertyMapping->readAccessor->getExpression($variableRegistry->getSourceInput());
}

if ($propertyMapping->hasCustomTransformer()) {
$output = $this->customTransformerExtractor->extract($propertyMapping->transformer, $fieldValueVariable, $variableRegistry->getSourceInput());
$propStatements = [];

$output = $this->customTransformerExtractor->extract($propertyMapping->transformer, $fieldValueVariable, $variableRegistry->getSourceInput());
} else {
/* Create expression to transform the read value into the wanted written value, depending on the transform it may add new statements to get the correct value */
[$output, $propStatements] = $propertyMapping->transformer->transform(
Expand Down
4 changes: 2 additions & 2 deletions src/MapperContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,9 @@ public static function withReference(array $context, string $reference, &$object
/**
* Check whether an attribute is allowed to be mapped.
*/
public static function isAllowedAttribute(array $context, string $attribute, $value): bool
public static function isAllowedAttribute(array $context, string $attribute, bool $valueIsNotNullOrNotUndefined): bool
{
if (($context[self::SKIP_NULL_VALUES] ?? false) && null === $value) {
if (($context[self::SKIP_NULL_VALUES] ?? false) && !$valueIsNotNullOrNotUndefined) {
return false;
}

Expand Down
11 changes: 11 additions & 0 deletions tests/AutoMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use AutoMapper\Tests\Fixtures\Order;
use AutoMapper\Tests\Fixtures\PetOwner;
use AutoMapper\Tests\Fixtures\Transformer\MoneyTransformerFactory;
use AutoMapper\Tests\Fixtures\Uninitialized;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\Serializer\NameConverter\AdvancedNameConverterInterface;
use Symfony\Component\Uid\Ulid;
Expand Down Expand Up @@ -1266,4 +1267,14 @@ public function testItCanMapFromArrayWithMissingNullableProperty(): void
$this->autoMapper->map(['foo' => 1], ClassWithNullablePropertyInConstructor::class)
);
}

public function testNoErrorWithUninitializedProperty(): void
{
$this->buildAutoMapper(mapPrivatePropertiesAndMethod: true);

self::assertSame(
['bar' => 'bar'],
$this->autoMapper->map(new Uninitialized(), 'array', ['skip_null_values' => true])
);
}
}
10 changes: 10 additions & 0 deletions tests/Fixtures/Uninitialized.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

namespace AutoMapper\Tests\Fixtures;

class Uninitialized
{
public string $foo;

public string $bar = 'bar';
}
10 changes: 5 additions & 5 deletions tests/MapperContextTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public function testIsAllowedAttribute(): void
$context->setAllowedAttributes(['id', 'age']);
$context->setIgnoredAttributes(['age']);

self::assertTrue(MapperContext::isAllowedAttribute($context->toArray(), 'id', 1));
self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'age', 29));
self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'name', 'Baptiste'));
self::assertTrue(MapperContext::isAllowedAttribute($context->toArray(), 'id', true));
self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'age', true));
self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'name', true));
}

public function testCircularReferenceLimit(): void
Expand Down Expand Up @@ -127,7 +127,7 @@ public function testWithNewContextAllowedAttributesNested(): void
],
];

self::assertTrue(MapperContext::isAllowedAttribute($context, 'foo', 1));
self::assertTrue(MapperContext::isAllowedAttribute($context, 'foo', true));
$newContext = MapperContext::withNewContext($context, 'foo');

self::assertEquals(['bar'], $newContext[MapperContext::ALLOWED_ATTRIBUTES]);
Expand All @@ -136,6 +136,6 @@ public function testWithNewContextAllowedAttributesNested(): void
public function testSkipNullValues(): void
{
$context = [MapperContext::SKIP_NULL_VALUES => true];
self::assertFalse(MapperContext::isAllowedAttribute($context, 'id', null));
self::assertFalse(MapperContext::isAllowedAttribute($context, 'id', false));
}
}
5 changes: 0 additions & 5 deletions tools/phpstan/phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -345,11 +345,6 @@ parameters:
count: 1
path: ../../src/MapperContext.php

-
message: "#^Method AutoMapper\\\\MapperContext\\:\\:isAllowedAttribute\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1
path: ../../src/MapperContext.php

-
message: "#^Method AutoMapper\\\\MapperContext\\:\\:setAllowedAttributes\\(\\) has parameter \\$allowedAttributes with no value type specified in iterable type array\\.$#"
count: 1
Expand Down

0 comments on commit 53fc76f

Please sign in to comment.