[Serializer] Named serializer should get correct normalizer instance #59627
Conversation
Named serializers were introduced in symfony#56823 and they work great. We noticed a small bug when using custom name convertors. The MimeMessageNormalizer holds a reference to `serializer.normalizer.property`. But when using named serializers, it would use the specific child normalizer instead. With this change, we fix this problem for any service that starts with `serializer.normalizer.`.
93ca946 to
e0caeb7
Compare
| foreach ($container->getDefinition($id)->getArguments() as $index => $argument) { | ||
| if ($argument instanceof Reference && self::NAME_CONVERTER_METADATA_AWARE_ID === (string) $argument) { | ||
| $definition->replaceArgument($index, new Reference($config['name_converter'])); | ||
| } elseif ($argument instanceof Reference && str_starts_with((string) $argument, 'serializer.normalizer.')) { |
There was a problem hiding this comment.
@ruudk This won't work since the service is not tagged with serializer.normalizer:
symfony/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.php
Lines 136 to 146 in ac26e1b
Without the tag, the child service will never be created. As far as I can tell, the service was intentionally not tagged, but I could be wrong, see #37847.
There was a problem hiding this comment.
Weird because this fix does solve the problem.
There was a problem hiding this comment.
Are you sure? I've tested the fix on a clean install and got an error:
framework:
serializer:
named_serializers:
foo:
name_converter: serializer.name_converter.camel_case_to_snake_caseThe service "serializer.normalizer.mime_message.foo" has a dependency on a non-existent service "serializer.normalizer.property.foo".
There was a problem hiding this comment.
Oops, you're right. For some reason, the project where we experience this problem, had the following compiler pass:
$container->addCompilerPass(
new class() implements CompilerPassInterface {
public function process(ContainerBuilder $container) : void
{
$container->getDefinition('serializer.normalizer.property')
->addTag('serializer.normalizer', [
'priority' => -990,
]);
}
},
priority: 10,
);I cannot recall why we added this, but probably to make sure serialization worked the same way as JMS Serializer.
There was a problem hiding this comment.
I know why we added this, because we want to use the PropertyNormalizer to make sure no getters are called while serializing. We only want properties to be serialized.
Is there a better way to enable this? Or is the compiler pass above the recommended way to use this normalizer?
Maybe we can check in this PR if the serializer.normalizer.*.$serializer_name exists, and then replace it?
Named serializers were introduced in #56823 and they work great.
We noticed a small bug when using custom name convertors.
The MimeMessageNormalizer holds a reference to
serializer.normalizer.property.But when using named serializers, it should use the specific child normalizer instead.
With this change, we fix this problem for any service that starts with
serializer.normalizer../cc @HypeMC