Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Serializer] Improve nested payload validation for#[MapRequestPayload] using a new serialization context#53250

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
mmouih wants to merge1 commit intosymfony:7.4
base:7.4
Choose a base branch
Loading
frommmouih:improve_payload_mapper_error_reporting

Conversation

mmouih
Copy link

@mmouihmmouih commentedDec 28, 2023
edited
Loading

QA
Branch?7.1
Bug fix?no
New feature?yes
Deprecations?no
LicenseMIT

Hello everyone,

When validating anested object using MapRequestPayload, and given payload schema doesn't respect the DTO provided, Symfony serializer throwsPartialDenormalizationException.

Unfortunately theMapRequestPayloadResolver doesn't have enough information about the Dto structure to be able to provide a more detailed error, Instead symfony throws the errors "This value should be of type unknown."

image

Thisunknown type is originated from theAbstractNormalizer in the serializer component, which injects this value toNotNormalizableValueException class.
I propose to add new Serialization contextAbstractNormalizer::USE_CLASS_AS_DEFAULT_EXPECTED_TYPE,
which allows us to retrieve the class as the expected type for Our DTO if needed.

Example when using the context AbstractNormalizer::USE_CLASS_AS_DEFAULT_EXPECTED_TYPE:
image

I honestly do not know the reason behind using unkown instead of the FQCN. I find adding a new context more safe, and certainlyBackward compatible to support both usages.

Usage example in a controller:

https://github.com/mmouih/apps-demo/blob/19f9edfa4361567a4b592a32041ead327fb3b898/src/Controller/HomeController.php#L22,L28

I included tests for the new feature:

https://github.com/mmouih/symfony/blob/05ef92028cb75c556ebbf2857617bdf40b8f1643/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php#L288C21-L288C94

Best regards.

@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.1" but it seems your PR description refers to branch "7.1 for features 6.3, 6.4, 7.0".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbotcarsonbot changed the title#[MapRequestPayload][Serializer] improve nested payload validation for MapRequestPayload using a new serialization context[Serializer] #[MapRequestPayload] improve nested payload validation for MapRequestPayload using a new serialization contextDec 28, 2023
@mmouihmmouihforce-pushed theimprove_payload_mapper_error_reporting branch 2 times, most recently froma25d919 to62d7c88CompareDecember 28, 2023 03:08
@OskarStarkOskarStark changed the title[Serializer] #[MapRequestPayload] improve nested payload validation for MapRequestPayload using a new serialization context[Serializer] Improve nested payload validation for#[MapRequestPayload] using a new serialization contextDec 28, 2023
@mmouihmmouihforce-pushed theimprove_payload_mapper_error_reporting branch from62d7c88 to55880a5CompareDecember 29, 2023 00:15
…r MapRequestPayload using a new serialization context
@mmouihmmouihforce-pushed theimprove_payload_mapper_error_reporting branch from55880a5 to988304eCompareDecember 29, 2023 00:18
$arguments = $resolver->resolve($request, $argument);
$event = new ControllerArgumentsEvent($kernel, function () {}, $arguments, $request, HttpKernelInterface::MAIN_REQUEST);

try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

You can leverageexpectException andexpectExceptionMessage instead

mmouih reacted with thumbs up emoji
@mmouihmmouih marked this pull request as draftJanuary 6, 2024 01:18
@xabbuhxabbuh modified the milestones:7.1,7.2May 15, 2024
@fabpotfabpot modified the milestones:7.2,7.3Nov 20, 2024
@fabpotfabpot modified the milestones:7.3,7.4May 26, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@mtarldmtarldmtarld left review comments

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

5 participants
@mmouih@carsonbot@mtarld@fabpot@xabbuh

[8]ページ先頭

©2009-2025 Movatter.jp