Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serializer] Fix object normalizer when properties has the same name as their accessor #54148

Merged
merged 1 commit into from Mar 17, 2024

Conversation

NeilPeyssard
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #54109
License MIT

Hello,

This PR fix a bug in object normalization when properties has the same name as their accessor. The current behavior mess up class metadata between properties and methods so the keys in the normalized format does not match the object properties, and metadata are not correctly applied (SerializedName in our exemple).

This bug also affects versions 6.4 and 7+, but I'm not sure if we can merge this PR in these branches without refactoring. Let me know if another PR should be open for version 6.4 and 7.

@NeilPeyssard
Copy link
Contributor Author

Fabbot doesn't seems happy, but it complains about things which are not in my changes... Should I update it too ?

@nicolas-grekas
Copy link
Member

Fabbot doesn't seems happy

You can ignore it

@fabpot
Copy link
Member

fabpot commented Mar 17, 2024

Thank you @NeilPeyssard.

@fabpot fabpot merged commit 8391d6d into symfony:5.4 Mar 17, 2024
8 of 12 checks passed
This was referenced Apr 2, 2024
@michaljusiega
Copy link
Contributor

Hi @fabpot @nicolas-grekas @NeilPeyssard

I think this PR could make BC-Break. In my case in my project (with SF 6.4), I have a lot base-code where a property have the same name as accessor-method.

For example:

<?php

class MyObject {
    public function __construct(
        private readonly bool|null $isVerified,
    ) {
    }

    public function isVerified(): bool|null
    {
        return $this->isVerified;
    }
}

returns before 6.4.6:
['verified' => true];

returns after6.4.6:
['isVerified' => true];

This situation means that if "keys" are sent via the API in the original version, it now has to be fixed, which is a difficult work.

At the moment, only way is to add conflict in composer.json like
"symfony/serializer": ">= 6.4.6"

In my opinion, you have a right - this was bug but this should be fixed in deprecation-way with some warnings. WDYT ?

nicolas-grekas added a commit that referenced this pull request Apr 17, 2024
… has the same name as their accessor" - it was a BC Break (NeilPeyssard)

This PR was merged into the 5.4 branch.

Discussion
----------

[Serializer] Revert "Fix object normalizer when properties has the same name as their accessor" - it was a BC Break

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #54488
| License       | MIT

Revert changes from this PR #54148 (BC Break).

Commits
-------

f6b4c65 [Serializer] Revert #54488 to fix BC Break
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants