[Serializer] fixed object normalizer for a class with cancel method
#56868
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
This comment has been minimized.
This comment has been minimized.
|
I can see that unit tests failures come from totally different area and the same failures repeat across different PRs. How should we proceed? |
carsonbot
changed the title
cancel methodcancel method
src/Symfony/Component/Serializer/Normalizer/ObjectNormalizer.php
Outdated
Show resolved
Hide resolved
You can ignore them for now |
src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php
Show resolved
Hide resolved
| if (str_starts_with($name, 'get') || str_starts_with($name, 'has') || str_starts_with($name, 'can')) { | ||
| if ( | ||
| (str_starts_with($name, 'get') || str_starts_with($name, 'has') || str_starts_with($name, 'can')) | ||
| && ctype_upper($name[3] ?? '') |
There was a problem hiding this comment.
As you said in the description methods are case insensitive in PHP, so this looks too brittle to me.
There was a problem hiding this comment.
I barely see any other option that won't imply having a list of exclusions. If you have any better idea, share please.
During the debug of quite big application I found out that at unrelated edge cases a class got called
cancelmethod. It turned out that as a part of outbox pattern, Serializer kicked in to producefailedqueue message. And eventually, found out that attributes list ofObjectNormalizercontains a fieldcelthat didn't exist anywhere.Eventually, it turned out that for default
ObjectNormalizerconfiguration, getters are also utilized to fetch list of attributes. But since Symfony 6.1cannerswere introduced, alongside withissersandhassers. Butcanprefix is also applicable to a wordcanCeland provided PHP methods are case-insensitive, getting list of attributes caused accidental call ofcancelmethod breaking business logic.See attached unit test for better explanation.