From 954f1d72c22829366aae09b59d311ff2316cf8da Mon Sep 17 00:00:00 2001 From: Baptiste Date: Tue, 22 Oct 2024 14:12:38 +0200 Subject: [PATCH] Added skip unitialized values context param --- CHANGELOG.md | 7 ++ composer.json | 2 +- src/Extractor/ReadAccessor.php | 86 ++++++++++++++++++- src/GeneratedMapper.php | 3 + .../CreateTargetStatementsGenerator.php | 4 +- src/Generator/MapperConstructorGenerator.php | 23 +++++ src/Generator/MapperGenerator.php | 1 - src/Generator/PropertyConditionsGenerator.php | 8 +- src/MapperContext.php | 17 +++- src/Transformer/BuiltinTransformer.php | 2 +- tests/AutoMapperTest.php | 20 ++++- tests/Fixtures/Issue189/User.php | 48 +++++++++++ tests/Fixtures/Issue189/UserPatchInput.php | 12 +++ tests/MapperContextTest.php | 20 +++-- .../Features/SkipNullValuesTestTrait.php | 3 +- 15 files changed, 235 insertions(+), 21 deletions(-) create mode 100644 tests/Fixtures/Issue189/User.php create mode 100644 tests/Fixtures/Issue189/UserPatchInput.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 889d2839..3156e0a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- [GH#200](https://github.com/jolicode/automapper/pull/200) Added skip_uninitialized_values context to skip non initialized properties +- [GH#200](https://github.com/jolicode/automapper/pull/200) Changed skip_null_values behavior to not handle initialized properties anymore + ### Fixed - [GH#207](https://github.com/jolicode/automapper/pull/207) [GH#208](https://github.com/jolicode/automapper/pull/208) Fix implicity nullable parameter deprecations +### Removed +- [GH#200](https://github.com/jolicode/automapper/pull/200) Drop nikic/php-parser < 5.0 compatibility + ## [9.2.0] - 2024-11-19 ### Added - [GH#180](https://github.com/jolicode/automapper/pull/180) Add configuration to generate code with strict types diff --git a/composer.json b/composer.json index bd22b84a..b75e9e7a 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ ], "require": { "php": "^8.2", - "nikic/php-parser": "^4.18 || ^5.0", + "nikic/php-parser": "^5.0", "symfony/deprecation-contracts": "^3.0", "symfony/event-dispatcher": "^6.4 || ^7.0", "symfony/expression-language": "^6.4 || ^7.0", diff --git a/src/Extractor/ReadAccessor.php b/src/Extractor/ReadAccessor.php index 8b2d40f5..8cf6f2ca 100644 --- a/src/Extractor/ReadAccessor.php +++ b/src/Extractor/ReadAccessor.php @@ -188,18 +188,18 @@ public function getIsNullExpression(Expr\Variable $input): Expr /* * Use the property fetch to read the value * - * isset($input->property_name) + * isset($input->property_name) && null === $input->property_name */ - return new Expr\BooleanNot(new Expr\Isset_([new Expr\PropertyFetch($input, $this->accessor)])); + 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))); } if (self::TYPE_ARRAY_DIMENSION === $this->type) { /* * Use the array dim fetch to read the value * - * isset($input['property_name']) + * isset($input['property_name']) && null === $input->property_name */ - return new Expr\BooleanNot(new Expr\Isset_([new Expr\ArrayDimFetch($input, new Scalar\String_($this->accessor))])); + 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))); } if (self::TYPE_SOURCE === $this->type) { @@ -212,6 +212,52 @@ public function getIsNullExpression(Expr\Variable $input): Expr throw new CompileException('Invalid accessor for read expression'); } + public function getIsUndefinedExpression(Expr\Variable $input): Expr + { + if (\in_array($this->type, [self::TYPE_METHOD, self::TYPE_SOURCE])) { + /* + * false + */ + return new Expr\ConstFetch(new Name('false')); + } + + 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::getExtractIsUndefinedCallback() + * + * $this->extractIsUndefinedCallbacks['property_name']($input) + */ + return new Expr\FuncCall( + new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractIsUndefinedCallbacks'), new Scalar\String_($this->accessor)), + [ + new Arg($input), + ] + ); + } + + /* + * Use the property fetch to read the value + * + * !isset($input->property_name) + */ + return new Expr\BooleanNot(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 + * + * !array_key_exists('property_name', $input) + */ + return new Expr\BooleanNot(new Expr\FuncCall(new Name('array_key_exists'), [new Arg(new Scalar\String_($this->accessor)), new Arg($input)])); + } + + throw new CompileException('Invalid accessor for read expression'); + } + /** * Get AST expression for binding closure when dealing with a private property. */ @@ -261,6 +307,38 @@ public function getExtractIsNullCallback(string $className): ?Expr return null; } + /* + * Create extract is null callback for this accessor + * + * \Closure::bind(function ($object) { + * return !isset($object->property_name) && null === $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\BinaryOp\LogicalAnd(new Expr\BooleanNot(new Expr\Isset_([new Expr\PropertyFetch(new Expr\Variable('object'), $this->accessor)])), new Expr\BinaryOp\Identical(new Expr\ConstFetch(new Name('null')), new Expr\PropertyFetch(new Expr\Variable('object'), $this->accessor)))), + ], + ]) + ), + new Arg(new Expr\ConstFetch(new Name('null'))), + new Arg(new Scalar\String_($className)), + ]); + } + + /** + * Get AST expression for binding closure when dealing with a private property. + */ + public function getExtractIsUndefinedCallback(string $className): ?Expr + { + if ($this->type !== self::TYPE_PROPERTY || !$this->private) { + return null; + } + /* * Create extract is null callback for this accessor * diff --git a/src/GeneratedMapper.php b/src/GeneratedMapper.php index e520943c..8a009e4b 100644 --- a/src/GeneratedMapper.php +++ b/src/GeneratedMapper.php @@ -48,6 +48,9 @@ public function registerMappers(AutoMapperRegistryInterface $registry): void /** @var array) */ protected array $extractIsNullCallbacks = []; + /** @var array) */ + protected array $extractIsUndefinedCallbacks = []; + /** @var Target|\ReflectionClass */ protected mixed $cachedTarget; } diff --git a/src/Generator/CreateTargetStatementsGenerator.php b/src/Generator/CreateTargetStatementsGenerator.php index 000383a9..17d1e635 100644 --- a/src/Generator/CreateTargetStatementsGenerator.php +++ b/src/Generator/CreateTargetStatementsGenerator.php @@ -193,7 +193,7 @@ private function constructorArgument(GeneratorMetadata $metadata, PropertyMetada 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))), new Arg(create_scalar_int(0)), new Arg(new Expr\ConstFetch(new Name('null'))), - new Arg(new Expr\Array_([ // @phpstan-ignore argument.type + new Arg(new Expr\Array_([ create_expr_array_item(new Scalar\String_($propertyMetadata->target->property)), ])), new Arg(new Scalar\String_($metadata->mapperMetadata->target)), @@ -262,7 +262,7 @@ private function constructorArgumentWithoutSource(GeneratorMetadata $metadata, \ 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, $constructorParameter->getName()))), new Arg(create_scalar_int(0)), new Arg(new Expr\ConstFetch(new Name('null'))), - new Arg(new Expr\Array_([ // @phpstan-ignore argument.type + new Arg(new Expr\Array_([ create_expr_array_item(new Scalar\String_($constructorParameter->getName())), ])), new Arg(new Scalar\String_($constructorParameter->getName())), diff --git a/src/Generator/MapperConstructorGenerator.php b/src/Generator/MapperConstructorGenerator.php index d27b4c55..75c73340 100644 --- a/src/Generator/MapperConstructorGenerator.php +++ b/src/Generator/MapperConstructorGenerator.php @@ -31,6 +31,7 @@ public function getStatements(GeneratorMetadata $metadata): array foreach ($metadata->propertiesMetadata as $propertyMetadata) { $constructStatements[] = $this->extractCallbackForProperty($metadata, $propertyMetadata); $constructStatements[] = $this->extractIsNullCallbackForProperty($metadata, $propertyMetadata); + $constructStatements[] = $this->extractIsUndefinedCallbackForProperty($metadata, $propertyMetadata); $constructStatements[] = $this->hydrateCallbackForProperty($metadata, $propertyMetadata); } @@ -83,6 +84,28 @@ private function extractIsNullCallbackForProperty(GeneratorMetadata $metadata, P )); } + /** + * Add read callback to the constructor of the generated mapper. + * + * ```php + * $this->extractIsUndefinedCallbacks['propertyName'] = $extractIsNullCallback; + * ``` + */ + private function extractIsUndefinedCallbackForProperty(GeneratorMetadata $metadata, PropertyMetadata $propertyMetadata): ?Stmt\Expression + { + $extractUndefinedCallback = $propertyMetadata->source->accessor?->getExtractIsUndefinedCallback($metadata->mapperMetadata->source); + + if (!$extractUndefinedCallback) { + return null; + } + + return new Stmt\Expression( + new Expr\Assign( + new Expr\ArrayDimFetch(new Expr\PropertyFetch(new Expr\Variable('this'), 'extractIsUndefinedCallbacks'), new Scalar\String_($propertyMetadata->source->property)), + $extractUndefinedCallback + )); + } + /** * Add hydrate callback to the constructor of the generated mapper. * diff --git a/src/Generator/MapperGenerator.php b/src/Generator/MapperGenerator.php index f575d56f..d43467cc 100644 --- a/src/Generator/MapperGenerator.php +++ b/src/Generator/MapperGenerator.php @@ -74,7 +74,6 @@ public function generate(GeneratorMetadata $metadata): array $statements = []; if ($metadata->strictTypes) { - // @phpstan-ignore argument.type $statements[] = new Stmt\Declare_([create_declare_item('strict_types', create_scalar_int(1))]); } $statements[] = (new Builder\Class_($metadata->mapperMetadata->className)) diff --git a/src/Generator/PropertyConditionsGenerator.php b/src/Generator/PropertyConditionsGenerator.php index 220f2d33..ca1cd923 100644 --- a/src/Generator/PropertyConditionsGenerator.php +++ b/src/Generator/PropertyConditionsGenerator.php @@ -138,7 +138,11 @@ private function isAllowedAttribute(GeneratorMetadata $metadata, PropertyMetadat return new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'isAllowedAttribute', [ new Arg($variableRegistry->getContext()), new Arg(new Scalar\String_($propertyMetadata->source->property)), - new Arg($propertyMetadata->source->accessor->getIsNullExpression($variableRegistry->getSourceInput())), + new Arg(new Expr\Closure([ + 'uses' => [new Expr\ClosureUse($variableRegistry->getSourceInput())], + 'stmts' => [new Stmt\Return_($propertyMetadata->source->accessor->getIsNullExpression($variableRegistry->getSourceInput()))], + ])), + new Arg($propertyMetadata->source->accessor->getIsUndefinedExpression($variableRegistry->getSourceInput())), ]); } @@ -172,7 +176,7 @@ private function groupsCheck(VariableRegistry $variableRegistry, ?array $groups new Expr\Array_() ) ), - new Arg(new Expr\Array_(array_map(function (string $group) { // @phpstan-ignore argument.type + new Arg(new Expr\Array_(array_map(function (string $group) { return create_expr_array_item(new Scalar\String_($group)); }, $groups))), ]) diff --git a/src/MapperContext.php b/src/MapperContext.php index a18aab01..e36c949b 100644 --- a/src/MapperContext.php +++ b/src/MapperContext.php @@ -28,6 +28,7 @@ * "deep_target_to_populate"?: bool, * "constructor_arguments"?: array>, * "skip_null_values"?: bool, + * "skip_uninitialized_values"?: bool, * "allow_readonly_target_to_populate"?: bool, * "datetime_format"?: string, * "datetime_force_timezone"?: string, @@ -49,6 +50,7 @@ class MapperContext public const DEEP_TARGET_TO_POPULATE = 'deep_target_to_populate'; public const CONSTRUCTOR_ARGUMENTS = 'constructor_arguments'; public const SKIP_NULL_VALUES = 'skip_null_values'; + public const SKIP_UNINITIALIZED_VALUES = 'skip_uninitialized_values'; public const ALLOW_READONLY_TARGET_TO_POPULATE = 'allow_readonly_target_to_populate'; public const DATETIME_FORMAT = 'datetime_format'; public const DATETIME_FORCE_TIMEZONE = 'datetime_force_timezone'; @@ -135,6 +137,13 @@ public function setSkipNullValues(bool $skipNullValues): self return $this; } + public function setSkipUnitializedValues(bool $skipUnitializedValues): self + { + $this->context[self::SKIP_UNINITIALIZED_VALUES] = $skipUnitializedValues; + + return $this; + } + public function setAllowReadOnlyTargetToPopulate(bool $allowReadOnlyTargetToPopulate): self { $this->context[self::ALLOW_READONLY_TARGET_TO_POPULATE] = $allowReadOnlyTargetToPopulate; @@ -231,9 +240,13 @@ public static function withReference(array $context, string $reference, mixed &$ * * @internal */ - public static function isAllowedAttribute(array $context, string $attribute, bool $valueIsNullOrUndefined): bool + public static function isAllowedAttribute(array $context, string $attribute, callable $valueIsNull, bool $valueIsUndefined): bool { - if (($context[self::SKIP_NULL_VALUES] ?? false) && $valueIsNullOrUndefined) { + if (($context[self::SKIP_UNINITIALIZED_VALUES] ?? false) && $valueIsUndefined) { + return false; + } + + if (($context[self::SKIP_NULL_VALUES] ?? false) && !$valueIsUndefined && $valueIsNull()) { return false; } diff --git a/src/Transformer/BuiltinTransformer.php b/src/Transformer/BuiltinTransformer.php index ed9142a8..9ddd5ec6 100644 --- a/src/Transformer/BuiltinTransformer.php +++ b/src/Transformer/BuiltinTransformer.php @@ -136,7 +136,7 @@ public function getCheckExpression(Expr $input, Expr $target, PropertyMetadata $ private function toArray(Expr $input): Expr { - return new Expr\Array_([create_expr_array_item($input)]); // @phpstan-ignore argument.type + return new Expr\Array_([create_expr_array_item($input)]); } private function fromIteratorToArray(Expr $input): Expr diff --git a/tests/AutoMapperTest.php b/tests/AutoMapperTest.php index fc792233..235f5e3e 100644 --- a/tests/AutoMapperTest.php +++ b/tests/AutoMapperTest.php @@ -42,6 +42,8 @@ use AutoMapper\Tests\Fixtures\Issue111\Colour; use AutoMapper\Tests\Fixtures\Issue111\ColourTransformer; use AutoMapper\Tests\Fixtures\Issue111\FooDto; +use AutoMapper\Tests\Fixtures\Issue189\User as Issue189User; +use AutoMapper\Tests\Fixtures\Issue189\UserPatchInput as Issue189UserPatchInput; use AutoMapper\Tests\Fixtures\ObjectsUnion\Bar; use AutoMapper\Tests\Fixtures\ObjectsUnion\Foo; use AutoMapper\Tests\Fixtures\ObjectsUnion\ObjectsUnionProperty; @@ -1009,7 +1011,7 @@ public function testSkipNullValues(): void $input = new Fixtures\SkipNullValues\Input(); /** @var Fixtures\SkipNullValues\Entity $entity */ - $entity = $this->autoMapper->map($input, $entity, ['skip_null_values' => true]); + $entity = $this->autoMapper->map($input, $entity, [MapperContext::SKIP_NULL_VALUES => true]); self::assertEquals('foobar', $entity->getName()); } @@ -1353,7 +1355,7 @@ public function testNoErrorWithUninitializedProperty(): void self::assertSame( ['bar' => 'bar'], - $this->autoMapper->map(new Uninitialized(), 'array', ['skip_null_values' => true]) + $this->autoMapper->map(new Uninitialized(), 'array', [MapperContext::SKIP_UNINITIALIZED_VALUES => true]) ); } @@ -1603,4 +1605,18 @@ public function testParamDocBlock(): void 'foo' => ['foo1', 'foo2'], ], $array); } + + public function testUninitializedProperties(): void + { + $payload = new Issue189UserPatchInput(); + $payload->firstName = 'John'; + $payload->lastName = 'Doe'; + + /** @var Issue189User $data */ + $data = $this->autoMapper->map($payload, Issue189User::class, [MapperContext::SKIP_UNINITIALIZED_VALUES => true]); + + $this->assertEquals('John', $data->getFirstName()); + $this->assertEquals('Doe', $data->getLastName()); + $this->assertTrue(!isset($data->birthDate)); + } } diff --git a/tests/Fixtures/Issue189/User.php b/tests/Fixtures/Issue189/User.php new file mode 100644 index 00000000..e92b0813 --- /dev/null +++ b/tests/Fixtures/Issue189/User.php @@ -0,0 +1,48 @@ +lastName; + } + + public function setLastName(string $lastName): self + { + $this->lastName = $lastName; + + return $this; + } + + public function getFirstName(): string + { + return $this->firstName; + } + + public function setFirstName(string $firstName): self + { + $this->firstName = $firstName; + + return $this; + } + + public function getBirthDate(): ?\DateTimeImmutable + { + return $this->birthDate; + } + + public function setBirthDate(?\DateTimeImmutable $birthDate): self + { + $this->birthDate = $birthDate; + + return $this; + } +} diff --git a/tests/Fixtures/Issue189/UserPatchInput.php b/tests/Fixtures/Issue189/UserPatchInput.php new file mode 100644 index 00000000..463810a4 --- /dev/null +++ b/tests/Fixtures/Issue189/UserPatchInput.php @@ -0,0 +1,12 @@ +setAllowedAttributes(['id', 'age']); $context->setIgnoredAttributes(['age']); - self::assertTrue(MapperContext::isAllowedAttribute($context->toArray(), 'id', true)); - self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'age', true)); - self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'name', true)); + $user = new UserDTO(); + + self::assertTrue(MapperContext::isAllowedAttribute($context->toArray(), 'id', function () use ($user) { return !isset($user->id) && null === $user->id; }, false)); + self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'age', function () use ($user) { return !isset($user->age) && null === $user->age; }, false)); + self::assertFalse(MapperContext::isAllowedAttribute($context->toArray(), 'name', function () { return false; }, false)); } public function testCircularReferenceLimit(): void @@ -121,6 +124,12 @@ public function testWithNewContextIgnoredAttributesNested(): void public function testWithNewContextAllowedAttributesNested(): void { + $data = new \stdClass(); + $data->foo = new \stdClass(); + $data->foo->bar = 'baz'; + $data->bar = 'popo'; + $data->baz = 'papa'; + $context = [ MapperContext::ALLOWED_ATTRIBUTES => [ 'foo' => ['bar'], @@ -128,7 +137,7 @@ public function testWithNewContextAllowedAttributesNested(): void ], ]; - self::assertTrue(MapperContext::isAllowedAttribute($context, 'foo', true)); + self::assertTrue(MapperContext::isAllowedAttribute($context, 'foo', function () use ($data) { return !isset($data->foo) && null === $data->foo; }, false)); $newContext = MapperContext::withNewContext($context, 'foo'); self::assertEquals(['bar'], $newContext[MapperContext::ALLOWED_ATTRIBUTES]); @@ -136,8 +145,9 @@ public function testWithNewContextAllowedAttributesNested(): void public function testSkipNullValues(): void { + $data = new UserDTO(); $context = [MapperContext::SKIP_NULL_VALUES => true]; - self::assertFalse(MapperContext::isAllowedAttribute($context, 'id', true)); + self::assertFalse(MapperContext::isAllowedAttribute($context, 'id', function () use ($data) { return !isset($data->id) && null === $data->id; }, false)); } /** diff --git a/tests/Normalizer/Features/SkipNullValuesTestTrait.php b/tests/Normalizer/Features/SkipNullValuesTestTrait.php index e3a43cde..d4880bd9 100644 --- a/tests/Normalizer/Features/SkipNullValuesTestTrait.php +++ b/tests/Normalizer/Features/SkipNullValuesTestTrait.php @@ -13,6 +13,7 @@ namespace AutoMapper\Tests\Normalizer\Features; +use AutoMapper\MapperContext; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; /** @@ -28,7 +29,7 @@ public function testSkipNullValues() $dummy->bar = 'present'; $normalizer = $this->getNormalizerForSkipNullValues(); - $result = $normalizer->normalize($dummy, null, ['skip_null_values' => true]); + $result = $normalizer->normalize($dummy, null, [MapperContext::SKIP_NULL_VALUES => true]); $this->assertSame(['bar' => 'present', 'fooBar' => 'present'], $result); } }