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] Allow AbstractNormalizer to use null for non-optional nullable constructor parameters without default value#40522

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

Merged
chalasr merged 1 commit intosymfony:4.4frompounard:5.x
Mar 29, 2021

Conversation

@pounard
Copy link
Contributor

@pounardpounard commentedMar 19, 2021
edited by nicolas-grekas
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#40511
LicenseMIT

Serializer component AbstractNormalizer attemps to guess constructor parameters, and falls back using default values when possible. Yet, it misses one use case: nullable non-optional parameter with value not being present in incoming input, case in which null is a valid value, not the default one, yet still valid.

This PR introduce a two-line fix that forcefully set null as value for missing from input non-optional nullable constructor parameters values.

@pounardpounard changed the titleissue #40511 - Allow AbstractNormalizer to use null for nullable constructor parameters without default valueissue #40511 - Allow AbstractNormalizer to use null for non-optional nullable constructor parameters without default valueMar 19, 2021
@pounard
Copy link
ContributorAuthor

Build failure seems to be unrelated, am I misreading it ?

@nicolas-grekasnicolas-grekas changed the titleissue #40511 - Allow AbstractNormalizer to use null for non-optional nullable constructor parameters without default value[Serializer] Allow AbstractNormalizer to use null for non-optional nullable constructor parameters without default valueMar 22, 2021
@nicolas-grekasnicolas-grekas added this to the5.x milestoneMar 22, 2021
@pounard
Copy link
ContributorAuthor

@ro0NL@nicolas-grekas thank you. I hope it will land in next 5.2 release, I have to maintain a patch in the meantime.

@nicolas-grekasnicolas-grekas modified the milestones:5.x,4.4Mar 22, 2021
…constructor parameter denormalization when not present in input
@nicolas-grekasnicolas-grekas changed the base branch from5.x to4.4March 22, 2021 16:48
@carsonbotcarsonbot changed the title[Serializer] Allow AbstractNormalizer to use null for non-optional nullable constructor parameters without default valueAllow AbstractNormalizer to use null for non-optional nullable constructor parameters without default valueMar 22, 2021
@carsonbotcarsonbot changed the titleAllow AbstractNormalizer to use null for non-optional nullable constructor parameters without default value[Serializer] Allow AbstractNormalizer to use null for non-optional nullable constructor parameters without default valueMar 22, 2021
@nicolas-grekas
Copy link
Member

I rebased on 4.4, where bugfixes should be merged.

pounard reacted with thumbs up emoji

@pounard
Copy link
ContributorAuthor

I rebased on 4.4, where bugfixes should be merged.

Oh, nice ! Thank you very much.

@chalasr
Copy link
Member

Thank you@pounard.

@chalasrchalasr merged commite16be83 intosymfony:4.4Mar 29, 2021
This was referencedMay 1, 2021
nicolas-grekas added a commit that referenced this pull requestMay 5, 2023
…listed in the input (Christian Kolb)This PR was merged into the 6.3 branch.Discussion----------[Serializer] Add flag to require all properties to be listed in the input| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       |Fix#49504| License       | MIT| Doc PR        |symfony/symfony-docs#17979The PR#40522 introduced a fallback for nullable properties to be set to `null` when they aren't provided as parameters.A drawback of that approach is that it easier for bugs to appear through typos or renamings of those properties. I think the current implementation makes perfect sense as a default. Therefore, this PR introduces a new context flag that prevents that fallback behaviour. This way nothing changes for existing systems, but for people wanting more control, it's possible to set a flag.### Example```phpfinal class Product{    public function __construct(        public string $name,        public ?int $costsInCent,    ) {    }}// This works and results in $costsInCent as null$product = $this->serializer->deserialize(    '{"name": "foo"}',    Product::class,    JsonEncoder::FORMAT,);// When using the flag, only the following JSON is valid$product = $this->serializer->deserialize(    '{"name": "foo", "costsInCent": null}',    Product::class,    JsonEncoder::FORMAT,    [        AbstractNormalizer::PREVENT_NULLABLE_FALLBACK => true,    ],);// This would result in an error due to missing parameters$product = $this->serializer->deserialize(    '{"name": "foo"}',    Product::class,    JsonEncoder::FORMAT,    [        AbstractNormalizer::PREVENT_NULLABLE_FALLBACK => true,    ],);```Commits-------d62410a [Serializer] Add flag to require all properties to be listed in the input
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@chalasrchalasrchalasr approved these changes

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

@jderussejderusseAwaiting requested review from jderusse

@lyrixxlyrixxAwaiting requested review from lyrixx

@srozesrozeAwaiting requested review from sroze

@wouterjwouterjAwaiting requested review from wouterj

@xabbuhxabbuhAwaiting requested review from xabbuh

@ycerutoycerutoAwaiting requested review from yceruto

+2 more reviewers

@ro0NLro0NLro0NL approved these changes

@maxheliasmaxheliasmaxhelias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

Allow AbstractNormalizer to use null for nullable constructor parameters without default value

6 participants

@pounard@nicolas-grekas@chalasr@ro0NL@maxhelias@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp