Add support for serializing stdClass objects #33894
Conversation
| @@ -81,7 +82,7 @@ public function testNormalizeNoMatch() | |||
| { | |||
| $this->expectException('Symfony\Component\Serializer\Exception\UnexpectedValueException'); | |||
| $serializer = new Serializer([$this->getMockBuilder('Symfony\Component\Serializer\Normalizer\CustomNormalizer')->getMock()]); | |||
| $serializer->normalize(new \stdClass(), 'xml'); | |||
There was a problem hiding this comment.
This change looks like a tiny BC break, on the other hand this test was relying on an (undocumented) edge case. WDYT @symfony/mergers?
There was a problem hiding this comment.
What’s the change? That previously you would get an exception with stdClass and now it would work? Or am I missing the point?
There was a problem hiding this comment.
I guess that test covers the case when we have no matching normalizer for the class that is not normalizable. So its not really important which not-normalizable class we are trying to normalize
There was a problem hiding this comment.
I think what @dunglas means is that currently an instance of stdClass can be properly serialized if you register a custom normalizer that supports it, with this PR such normalizer wouldn't be called anymore.
IMHO it's ok but only if we don't match classes extending stdClass (yes, one can do that..), I suggest using \stdClass::class === \get_class($data) instead of instanceof.
There was a problem hiding this comment.
Why not making it part of the object normalizer instead? It has a very low priority that would mitigate this issue.
There was a problem hiding this comment.
@ogizanagi thanks, good idea. Will check that tomorrow, I think that will work
|
@dunglas @chalasr please have a look at my implementation of @ogizanagi suggestion. Thanks! |
henry2778
force-pushed
the
feature/serailizer-std-class-support
branch
from
3ffe9d6 to
31b83fe
Compare
henry2778
force-pushed
the
feature/serailizer-std-class-support
branch
from
31b83fe to
7a06dbf
Compare
|
Any update on this MR? This is a quite annoying bug. |
|
ping @dunglas |
|
The new implementation doesn't look good to me. It doesn't support any of the feature of |
|
I just opened an alternative PR with what I've in mind: #35596 |
|
Thank you for pushing this @henry2778. |
This PR was merged into the 5.1-dev branch. Discussion ---------- [Serializer] Add support for stdClass | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #33047, #33894 | License | MIT | Doc PR | n/a Add support for `stdClass`. Alternative to #33894. Commits ------- d7bca80 [Serializer] Add support for stdClass
Updated PR for issue #33047 :) Adding support for serializing stdClass objects, because it was missing. Thanks!