Skip to content

[Serializer] Add context for CamelCaseToSnakeCaseNameConverter #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.

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
Merged
merged 1 commit into from
Mar 17, 2024
Merged

[Serializer] Add context for CamelCaseToSnakeCaseNameConverter #53898

merged 1 commit into from
Mar 17, 2024

Conversation

AurelienPillevesse
Copy link
Contributor

@AurelienPillevesse AurelienPillevesse commented Feb 10, 2024 •
edited
Loading

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues
License MIT

Currently, when we use a Symfony 7.0 project (or 6.4 for example) with the symfony/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 two POST requests :

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

They will be handled exactly the same.

The idea is to add a context CamelCaseToSnakeCaseNameConverter::REQUIRE_SNAKE_CASE_PROPERTIES which can be used in #[MapRequestPayload] attribute to only accept the POST request with last_name attribute body.

Example :

class MyRouteInput
{
    public string $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 with PartialDenormalizationException.
If for you it can added in 6.4 and 7.0 instead of 7.1, i'm OK.

@carsonbot carsonbot added Status: Needs Review Feature RFC RFC = Request For Comments (proposals about features that you want to be discussed) Serializer labels Feb 10, 2024
@carsonbot carsonbot added this to the 7.1 milestone Feb 10, 2024
@carsonbot carsonbot changed the title [RFC][Serializer] Add context for CamelCaseToSnakeCaseNameConverter [Serializer] Add context for CamelCaseToSnakeCaseNameConverter Feb 10, 2024
@OskarStark
Copy link
Contributor

Please add a test case to avoid further regression, thanks

@AurelienPillevesse AurelienPillevesse force-pushed the feature/context-force-snake-case branch 2 times, most recently from 1373073 to 45edbbb Compare February 10, 2024 21:15
@OskarStark OskarStark changed the title [Serializer] Add context for CamelCaseToSnakeCaseNameConverter [Serializer] Add context for CamelCaseToSnakeCaseNameConverter Feb 10, 2024
@AurelienPillevesse AurelienPillevesse force-pushed the feature/context-force-snake-case branch 3 times, most recently from 796cf00 to 6bf22cf Compare February 10, 2024 21:43
@OskarStark OskarStark removed the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Feb 10, 2024
@AurelienPillevesse AurelienPillevesse force-pushed the feature/context-force-snake-case branch 2 times, most recently from 4569651 to b2c5e57 Compare February 10, 2024 21:51
@AurelienPillevesse AurelienPillevesse force-pushed the feature/context-force-snake-case branch 2 times, most recently from f871772 to 75b0805 Compare February 11, 2024 08:36
@AurelienPillevesse AurelienPillevesse force-pushed the feature/context-force-snake-case branch from 75b0805 to 04e22ec Compare March 17, 2024 15:33
@fabpot
Copy link
Member

fabpot commented Mar 17, 2024

Thank you @AurelienPillevesse.

@fabpot fabpot merged commit a5c0b0c into symfony:7.1 Mar 17, 2024
@fabpot fabpot mentioned this pull request Mar 17, 2024
Closed
@AurelienPillevesse AurelienPillevesse deleted the feature/context-force-snake-case branch March 17, 2024 17:04
@AurelienPillevesse AurelienPillevesse mentioned this pull request Mar 17, 2024
Merged
fabpot added a commit that referenced this pull request Mar 17, 2024
@fabpot
minor #54316 Add missing changelog for PR53898 (AurelienPillevesse)
099daf5 
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       | MIT

Add missing changelog in my PR merged today : #53898

Commits
-------

c86f689 add missing changelog for PR 53898
@soyuka
Copy link
Contributor

soyuka commented Mar 21, 2024

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 current CamelCaseToSnakeCaseNameConverter.

soyuka added a commit to soyuka/core that referenced this pull request Mar 21, 2024
@soyuka
test: use our name converter
7c1336f 
symfony/symfony#53898 breaks our interface
@nicolas-grekas
Copy link
Member

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

@AurelienPillevesse
Copy link
Contributor Author

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

@xabbuh xabbuh mentioned this pull request Apr 9, 2024
Merged
@xabbuh
Copy link
Member

xabbuh commented Apr 9, 2024

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

fabpot added a commit that referenced this pull request Apr 17, 2024
@fabpot
minor #54531 [Serializer] add $class, $format and $context arguments …
22cbf8f 
…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       | MIT

Commits
-------

d088047 add $class, $format and $context arguments to NameConverterInterface methods
symfony-splitter pushed a commit to symfony/serializer that referenced this pull request Apr 17, 2024
@fabpot
minor #54531 [Serializer] add $class, $format and $context arguments …
309af65 
…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 symfony/symfony#53898 (comment)
| License       | MIT

Commits
-------

d088047dfa add $class, $format and $context arguments to NameConverterInterface methods
@fabpot fabpot mentioned this pull request May 2, 2024
Merged
@mtarld mtarld mentioned this pull request Sep 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

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

@OskarStark OskarStark Awaiting requested review from OskarStark

Assignees
No one assigned
Labels
Feature Serializer Status: Reviewed
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