Skip to content

Update NotNormalizableValueException with the attribute information #42733

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Update NotNormalizableValueException with the attribute information #42733

wants to merge 1 commit into from

Conversation

acasajus
Copy link

@acasajus acasajus commented Aug 25, 2021 •
edited
Loading

Q A
Branch? 5.4
Bug fix? no
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

It's currently not possible to get the information about which attribute is raising the exception (except by doing regex in the exception message string). Having this information allows to handle the error effectively upstream.

It's similar to #42712 but for a different exception.

@acasajus acasajus requested a review from dunglas as a code owner August 25, 2021 13:15
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 5.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@acasajus acasajus mentioned this pull request Aug 25, 2021
Closed
@ro0NL
Copy link
Contributor

ro0NL commented Aug 25, 2021

see #42502

@chalasr chalasr added this to the 5.4 milestone Sep 1, 2021
@fabpot
Copy link
Member

fabpot commented Oct 29, 2021

/cc @lyrixx

@lyrixx
Copy link
Member

lyrixx commented Oct 29, 2021

This PR needs a rebase to see what's going on IMHO. Because ATM, there is too much difference between what lives in 5.4 and this PR

@lyrixx
Copy link
Member

lyrixx commented Oct 29, 2021

OK, I had a better look at this PR and I think it can be closed because I implemented it in #42502

Basically, you can use $exception->getPath() to get the full path of the error.

There is a full example in the test:

symfony/src/Symfony/Component/Serializer/Tests/SerializerTest.php

Lines 743 to 1053 in 726d360

public function testCollectDenormalizationErrors()
{
$json = '
{
"string": null,
"int": null,
"float": null,
"bool": null,
"dateTime": null,
"dateTimeImmutable": null,
"dateTimeZone": null,
"splFileInfo": null,
"uuid": null,
"array": null,
"collection": [
{
"string": "string"
},
{
"string": null
}
],
"php74FullWithConstructor": {},
"dummyMessage": {
}
}';
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
$extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
$serializer = new Serializer(
[
new ArrayDenormalizer(),
new DateTimeNormalizer(),
new DateTimeZoneNormalizer(),
new DataUriNormalizer(),
new UidNormalizer(),
new ObjectNormalizer($classMetadataFactory, null, null, $extractor, new ClassDiscriminatorFromClassMetadata($classMetadataFactory)),
],
['json' => new JsonEncoder()]
);
try {
$serializer->deserialize($json, Php74Full::class, 'json', [
DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true,
]);
$this->fail();
} catch (\Throwable $th) {
$this->assertInstanceOf(PartialDenormalizationException::class, $th);
}
$this->assertInstanceOf(Php74Full::class, $th->getData());
$exceptionsAsArray = array_map(function (NotNormalizableValueException $e): array {
return [
'currentType' => $e->getCurrentType(),
'expectedTypes' => $e->getExpectedTypes(),
'path' => $e->getPath(),
'useMessageForUser' => $e->canUseMessageForUser(),
'message' => $e->getMessage(),
];
}, $th->getErrors());
$expected = [
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'string',
'useMessageForUser' => false,
'message' => 'The type of the "string" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "string" ("null" given).',
],
[
'currentType' => 'null',
'expectedTypes' => [
'int',
],
'path' => 'int',
'useMessageForUser' => false,
'message' => 'The type of the "int" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "int" ("null" given).',
],
[
'currentType' => 'null',
'expectedTypes' => [
'float',
],
'path' => 'float',
'useMessageForUser' => false,
'message' => 'The type of the "float" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "float" ("null" given).',
],
[
'currentType' => 'null',
'expectedTypes' => [
'bool',
],
'path' => 'bool',
'useMessageForUser' => false,
'message' => 'The type of the "bool" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "bool" ("null" given).',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'dateTime',
'useMessageForUser' => true,
'message' => 'The data is either an empty string or null, you should pass a string that can be parsed with the passed format or a valid DateTime string.',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'dateTimeImmutable',
'useMessageForUser' => true,
'message' => 'The data is either an empty string or null, you should pass a string that can be parsed with the passed format or a valid DateTime string.',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'dateTimeZone',
'useMessageForUser' => true,
'message' => 'The data is either an empty string or null, you should pass a string that can be parsed as a DateTimeZone.',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'splFileInfo',
'useMessageForUser' => true,
'message' => 'The provided "data:" URI is not valid.',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'uuid',
'useMessageForUser' => true,
'message' => 'The data is not a valid UUID string representation.',
],
[
'currentType' => 'null',
'expectedTypes' => [
'array',
],
'path' => 'array',
'useMessageForUser' => false,
'message' => 'The type of the "array" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "array" ("null" given).',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'collection[1].string',
'useMessageForUser' => false,
'message' => 'The type of the "string" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\Php74Full" must be one of "string" ("null" given).',
],
[
'currentType' => 'array',
'expectedTypes' => [
'unknown',
],
'path' => 'php74FullWithConstructor',
'useMessageForUser' => true,
'message' => 'Failed to create object because the object miss the "constructorArgument" property.',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => 'dummyMessage.type',
'useMessageForUser' => false,
'message' => 'Type property "type" not found for the abstract object "Symfony\Component\Serializer\Tests\Fixtures\DummyMessageInterface".',
],
];
$this->assertSame($expected, $exceptionsAsArray);
}
/** @requires PHP 7.4 */
public function testCollectDenormalizationErrors2()
{
$json = '
[
{
"string": null
},
{
"string": null
}
]';
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
$extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
$serializer = new Serializer(
[
new ArrayDenormalizer(),
new ObjectNormalizer($classMetadataFactory, null, null, $extractor, new ClassDiscriminatorFromClassMetadata($classMetadataFactory)),
],
['json' => new JsonEncoder()]
);
try {
$serializer->deserialize($json, Php74Full::class.'[]', 'json', [
DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true,
]);
$this->fail();
} catch (\Throwable $th) {
$this->assertInstanceOf(PartialDenormalizationException::class, $th);
}
$this->assertCount(2, $th->getData());
$this->assertInstanceOf(Php74Full::class, $th->getData()[0]);
$this->assertInstanceOf(Php74Full::class, $th->getData()[1]);
$exceptionsAsArray = array_map(function (NotNormalizableValueException $e): array {
return [
'currentType' => $e->getCurrentType(),
'expectedTypes' => $e->getExpectedTypes(),
'path' => $e->getPath(),
'useMessageForUser' => $e->canUseMessageForUser(),
'message' => $e->getMessage(),
];
}, $th->getErrors());
$expected = [
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => '[0].string',
'useMessageForUser' => false,
'message' => 'The type of the "string" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "string" ("null" given).',
],
[
'currentType' => 'null',
'expectedTypes' => [
'string',
],
'path' => '[1].string',
'useMessageForUser' => false,
'message' => 'The type of the "string" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "string" ("null" given).',
],
];
$this->assertSame($expected, $exceptionsAsArray);
}
/** @requires PHP 8.0 */
public function testCollectDenormalizationErrorsWithConstructor()
{
$json = '{"bool": "bool"}';
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
$extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
$serializer = new Serializer(
[
new ObjectNormalizer($classMetadataFactory, null, null, $extractor, new ClassDiscriminatorFromClassMetadata($classMetadataFactory)),
],
['json' => new JsonEncoder()]
);
try {
$serializer->deserialize($json, Php80WithPromotedTypedConstructor::class, 'json', [
DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true,
]);
$this->fail();
} catch (\Throwable $th) {
$this->assertInstanceOf(PartialDenormalizationException::class, $th);
}
$this->assertInstanceOf(Php80WithPromotedTypedConstructor::class, $th->getData());
$exceptionsAsArray = array_map(function (NotNormalizableValueException $e): array {
return [
'currentType' => $e->getCurrentType(),
'expectedTypes' => $e->getExpectedTypes(),
'path' => $e->getPath(),
'useMessageForUser' => $e->canUseMessageForUser(),
'message' => $e->getMessage(),
];
}, $th->getErrors());
$expected = [
[
'currentType' => 'string',
'expectedTypes' => [
'bool',
],
'path' => 'bool',
'useMessageForUser' => false,
'message' => 'The type of the "bool" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php80WithPromotedTypedConstructor" must be one of "bool" ("string" given).',
],
];
$this->assertSame($expected, $exceptionsAsArray);
}
}

@fabpot
Copy link
Member

fabpot commented Oct 29, 2021

Thank you for the review.

@fabpot fabpot closed this Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
6 participants
@acasajus @carsonbot @ro0NL @fabpot @lyrixx @chalasr