Skip to content

[Serializer] Fix support of plain object types denormalization #52804

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
Dec 1, 2023
Merged

[Serializer] Fix support of plain object types denormalization #52804

merged 1 commit into from
Dec 1, 2023
+18 −1

Conversation

andersonamuller
Copy link
Contributor

@andersonamuller andersonamuller commented Nov 29, 2023 •
edited by nicolas-grekas
Loading

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

When denormalizing properties with a plain object type without a class the ObjectNormalizer would fail with a TypeError.

@carsonbot carsonbot added this to the 5.4 milestone Nov 29, 2023
@andersonamuller andersonamuller changed the title Fix support of plain object types denormalization [Serializer] Fix support of plain object types denormalization Nov 29, 2023
@OskarStark
Copy link
Contributor

Can you please add test case to avoid further regressions? Thank you!

@andersonamuller
Copy link
Contributor Author

Can you please add test case to avoid further regressions? Thank you!

I added a test case

@OskarStark
Copy link
Contributor

friendly ping @mtarld as you are deeply involved in the serializer component, thanks!

mtarld
mtarld reviewed Dec 1, 2023
View reviewed changes
Copy link
Contributor

@mtarld mtarld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd do instead:

if (Type::BUILTIN_TYPE_OBJECT === $builtinType && null !== $class) {

By doing that, it'll fallback to:

if (('is_'.$builtinType)($data)) {

And IMHO, it might be better to handle a raw object as a builtin type. WDYT?

@andersonamuller
Copy link
Contributor Author

I'd do instead:

if (Type::BUILTIN_TYPE_OBJECT === $builtinType && null !== $class) {

By doing that, it'll fallback to:

if (('is_'.$builtinType)($data)) {

And IMHO, it might be better to handle a raw object as a builtin type. WDYT?

Makes sense, I have pushed the change.

@andersonamuller
Fix support to denormalize plain object types
4a728f7 
When denormalizing properties with a plain `object` type without a class the `ObjectNormalizer` would fail with a `TypeError`.
@carsonbot carsonbot changed the title [Serializer] Fix support of plain object types denormalization Fix support of plain object types denormalization Dec 1, 2023
@carsonbot carsonbot changed the title Fix support of plain object types denormalization [Serializer] Fix support of plain object types denormalization Dec 1, 2023
@nicolas-grekas
Copy link
Member

Thank you @andersonamuller.

@nicolas-grekas nicolas-grekas merged commit 410490c into symfony:5.4 Dec 1, 2023
This was referenced Dec 1, 2023
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtarld mtarld mtarld approved these changes

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

Assignees
No one assigned
Labels
Bug Serializer Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
5 participants
@andersonamuller @OskarStark @nicolas-grekas @mtarld @carsonbot