[FrameworkBundle] make GetSetMethodNormalizer available by default #12295
Conversation
| @@ -6,6 +6,7 @@ | |||
|
|
|||
| <parameters> | |||
| <parameter key="serializer.class">Symfony\Component\Serializer\Serializer</parameter> | |||
| <parameter key="serializer.normalizer.get_set_method.class">Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer</parameter> | |||
There was a problem hiding this comment.
Afaik it was decided to not use class parameters any more.
There was a problem hiding this comment.
For consistency should I remove existing class parameters (it's a BC)?
Or is mixing class parameters / no class parameters in the same file OK?
There was a problem hiding this comment.
The existing ones must be kept for BC. It's okay to mix them (see for example #12182).
0f18d65 to
885c238
Compare
|
@xabbuh removed class parameter. |
|
Avoiding BC is a bad idea. We want to avoid breaking BC, not keeping it. and making this normalizer run at a high priority is definitely a BC break, as it means it would run before custom normalizers registered the simple way, thus changing the behavior for existing apps |
|
@stof sorry I've misspelled my sentence. I want to keep BC of course :) So my current code is OK (or |
|
@dunglas krsort is meant to sort the high priority first (i.e. sorting in descending order). The effect is to make the high priority win (as the serializer uses the first normalizer supporting the value) |
|
@stof Ok I got it. I've missed the |
885c238 to
7ad35e4
Compare
|
Fixed and squashed. |
|
|
||
| <!-- Normalizer --> | ||
| <service id="serializer.normalizer.get_set_method" class="Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer" public="false"> | ||
| <!-- Arbitrary "low" priority to keep BC --> |
There was a problem hiding this comment.
talking about keeping BC in the comment is the wrong explanation. The reason to make it last (even without BC concern) is that it will report (nearly) any object as supported, so it should give other normalizer a chance to pick the object first.
7ad35e4 to
14a651a
Compare
|
Changed according to @stof comments. |
|
Thank you @dunglas. |
The
GetSetMethodNormalizerhas not been enabled by default in #6815 because it was broken by design. Since #12098 has been merged, circular references are handled and that normalizer cannot breaks applications.This PR makes that normalizer automatically available when the serializer is enabled. To keep BC, I've set a "high" priority for this service (higher than the default
0).