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] Add context forCamelCaseToSnakeCaseNameConverter#53898

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

Conversation

@AurelienPillevesse
Copy link
Contributor

@AurelienPillevesseAurelienPillevesse commentedFeb 10, 2024•
edited
Loading

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

Currently, when we use a Symfony 7.0 project (or 6.4 for example) with thesymfony/serializer component installed.

With the#[MapRequestPayload] feature and this configuration :

framework:serializer:name_converter:'serializer.name_converter.camel_case_to_snake_case'

This configuration forces Symfony to return to the snake case format.
But when sending these twoPOST requests :

{"lastName":"MyLastName"}
{"last_name":"MyLastName"}

They will be handled exactly the same.

The idea is to add a contextCamelCaseToSnakeCaseNameConverter::REQUIRE_SNAKE_CASE_PROPERTIES which can be used in#[MapRequestPayload] attribute to only accept thePOST request withlast_name attribute body.

Example :

class MyRouteInput{publicstring$lastName;}#[MapRequestPayload(serializationContext: [CamelCaseToSnakeCaseNameConverter::REQUIRE_SNAKE_CASE_PROPERTIES =>true])]MyRouteInput$input

[Note]
This is a first draft to expose my idea. The exception thrown and where the new condition is added can probably be improved.
A possible improvement can be to use the existing logic withPartialDenormalizationException.
If for you it can added in 6.4 and 7.0 instead of 7.1, i'm OK.

@carsonbotcarsonbot added Status: Needs Review Feature RFCRFC = Request For Comments (proposals about features that you want to be discussed) Serializer labelsFeb 10, 2024
@carsonbotcarsonbot added this to the7.1 milestoneFeb 10, 2024
@carsonbotcarsonbot changed the title[RFC][Serializer] Add context for CamelCaseToSnakeCaseNameConverter[Serializer] Add context for CamelCaseToSnakeCaseNameConverterFeb 10, 2024
@OskarStark
Copy link
Contributor

Please add a test case to avoid further regression, thanks

@AurelienPillevesseAurelienPillevesseforce-pushed thefeature/context-force-snake-case branch 2 times, most recently from1373073 to45edbbbCompareFebruary 10, 2024 21:15
@OskarStarkOskarStark changed the title[Serializer] Add context for CamelCaseToSnakeCaseNameConverter[Serializer] Add context forCamelCaseToSnakeCaseNameConverterFeb 10, 2024
@AurelienPillevesseAurelienPillevesseforce-pushed thefeature/context-force-snake-case branch 3 times, most recently from796cf00 to6bf22cfCompareFebruary 10, 2024 21:43
@OskarStarkOskarStark removed the RFCRFC = Request For Comments (proposals about features that you want to be discussed) labelFeb 10, 2024
@AurelienPillevesseAurelienPillevesseforce-pushed thefeature/context-force-snake-case branch 2 times, most recently from4569651 tob2c5e57CompareFebruary 10, 2024 21:51
@AurelienPillevesseAurelienPillevesseforce-pushed thefeature/context-force-snake-case branch 2 times, most recently fromf871772 to75b0805CompareFebruary 11, 2024 08:36
@AurelienPillevesseAurelienPillevesseforce-pushed thefeature/context-force-snake-case branch from75b0805 to04e22ecCompareMarch 17, 2024 15:33
@fabpot
Copy link
Member

Thank you@AurelienPillevesse.

@fabpotfabpot merged commita5c0b0c intosymfony:7.1Mar 17, 2024
@AurelienPillevesseAurelienPillevesse deleted the feature/context-force-snake-case branchMarch 17, 2024 17:04
fabpot added a commit that referenced this pull requestMar 17, 2024
This PR was merged into the 7.1 branch.Discussion----------Add missing changelog for PR53898| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | -| License       | MITAdd missing changelog in my PR merged today :#53898Commits-------c86f689 add missing changelog for PR 53898
@soyuka
Copy link
Contributor

Isn't this is a bc-break as both interfaces (AdvancedNameConverterInterface and the NameConverterInterface) are not compatible and this class is not internal. One could extend the currentCamelCaseToSnakeCaseNameConverter.

soyuka added a commit to soyuka/core that referenced this pull requestMar 21, 2024
@nicolas-grekas
Copy link
Member

Yes it is.@AurelienPillevesse can you please have a look?

@AurelienPillevesse
Copy link
ContributorAuthor

@nicolas-grekas How can we implement this feature without changing the interface ?

@xabbuh
Copy link
Member

I suggest that we add the arguments to theNameConverterInterface (in 8.0) instead:#54531

fabpot added a commit that referenced this pull requestApr 17, 2024
…to NameConverterInterface methods (xabbuh)This PR was merged into the 7.1 branch.Discussion----------[Serializer] add $class, $format and $context arguments to NameConverterInterface methods| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        |Fix#53898 (comment)| License       | MITCommits-------d088047 add $class, $format and $context arguments to NameConverterInterface methods
symfony-splitter pushed a commit to symfony/serializer that referenced this pull requestApr 17, 2024
…to NameConverterInterface methods (xabbuh)This PR was merged into the 7.1 branch.Discussion----------[Serializer] add $class, $format and $context arguments to NameConverterInterface methods| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        |Fixsymfony/symfony#53898 (comment)| License       | MITCommits-------d088047dfa add $class, $format and $context arguments to NameConverterInterface methods
@fabpotfabpot mentioned this pull requestMay 2, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

@OskarStarkOskarStarkAwaiting requested review from OskarStark

Assignees

No one assigned

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

7 participants

@AurelienPillevesse@OskarStark@fabpot@soyuka@nicolas-grekas@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp