Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(generator): allow skipping uninitialized property when skipping null values #44

Merged
merged 3 commits into from
Mar 11, 2024
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
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
joelwurtz marked this conversation as resolved.
Show resolved Hide resolved
{
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) {
joelwurtz marked this conversation as resolved.
Show resolved Hide resolved
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): ?
));
}

/**
* Add read callback to the constructor of the generated mapper.
*
* ```php
* $this->extractIsNullCallbacks['propertyName'] = $extractIsNullCallback;
* ```
*/
private function extractIsNullCallbackForProperty(PropertyMapping $propertyMapping): ?Stmt\Expression
{
$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
10 changes: 2 additions & 8 deletions src/Generator/PropertyConditionsGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private function propertyExistsForArray(PropertyMapping $propertyMapping): ?Expr
* In case of supporting attributes checking, we check if the property is allowed to be mapped.
*
* ```php
* MapperContext::isAllowedAttribute($context, 'propertyName', $source).
* MapperContext::isAllowedAttribute($context, 'propertyName', isset($source->field)).
* ```
*/
private function isAllowedAttribute(PropertyMapping $propertyMapping): ?Expr
Expand All @@ -112,16 +112,10 @@ private function isAllowedAttribute(PropertyMapping $propertyMapping): ?Expr

$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())),
joelwurtz marked this conversation as resolved.
Show resolved Hide resolved
]);
}

Expand Down
8 changes: 6 additions & 2 deletions src/Generator/PropertyStatementsGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,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
6 changes: 4 additions & 2 deletions src/MapperContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,12 @@ public static function withReference(array $context, string $reference, &$object

/**
* Check whether an attribute is allowed to be mapped.
*
* @internal
*/
public static function isAllowedAttribute(array $context, string $attribute, $value): bool
public static function isAllowedAttribute(array $context, string $attribute, bool $valueIsNotNullOrNotUndefined): bool
joelwurtz marked this conversation as resolved.
Show resolved Hide resolved
{
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])
);
}
}
12 changes: 12 additions & 0 deletions tests/Fixtures/Uninitialized.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

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
Loading