Skip to content

[Serializer] Named serializer should get correct normalizer instance #59627

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

Open
wants to merge 1 commit into
base: 7.2
Choose a base branch
from
+36 −13
Open

[Serializer] Named serializer should get correct normalizer instance #59627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions src/Symfony/Component/Serializer/DependencyInjection/SerializerPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,24 +185,17 @@ private function buildChildDefinitions(ContainerBuilder $container, string $seri

$definition = $container->registerChild($childId, (string) $id);

if (null !== $nameConverterIndex = $this->findNameConverterIndex($container, (string) $id)) {
$definition->replaceArgument($nameConverterIndex, new Reference($config['name_converter']));
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.')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@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

->set('serializer.normalizer.property', PropertyNormalizer::class)
->args([
service('serializer.mapping.class_metadata_factory'),
service('serializer.name_converter.metadata_aware'),
service('property_info')->ignoreOnInvalid(),
service('serializer.mapping.class_discriminator_resolver')->ignoreOnInvalid(),
null,
])
->set('serializer.denormalizer.array', ArrayDenormalizer::class)

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird because this fix does solve the problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

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_case
The service "serializer.normalizer.mime_message.foo" has a dependency on a non-existent service "serializer.normalizer.property.foo".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

$definition->replaceArgument($index, new Reference((string) $argument.'.'.$serializerName));
}
}

$id = new Reference($childId);
}

return $services;
}

private function findNameConverterIndex(ContainerBuilder $container, string $id): int|string|null
{
foreach ($container->getDefinition($id)->getArguments() as $index => $argument) {
if ($argument instanceof Reference && self::NAME_CONVERTER_METADATA_AWARE_ID === (string) $argument) {
return $index;
}
}

return null;
}
}
30 changes: 30 additions & 0 deletions src/Symfony/Component/Serializer/Tests/DependencyInjection/SerializerPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,4 +637,34 @@ public function testNormalizersAndEncodersAreDecoratedAndOrderedWhenCollectingDa
$this->assertEquals(new Reference('serializer.data_collector'), $traceableEncoderDefinition->getArgument(1));
$this->assertSame('api', $traceableEncoderDefinition->getArgument(2));
}

public function testPropertyNormalizerIsCorrectInstanceWhenUsingNamedSerializer()
{
$container = new ContainerBuilder();

$container->setParameter('kernel.debug', false);
$container->setParameter('.serializer.named_serializers', [
'api' => [],
]);

$container->register('serializer')->setArguments([null, null]);
$container->register('n0')
->setArguments([
new Reference('serializer.normalizer.property'),
])
->addTag('serializer.normalizer', ['serializer' => 'default']);
$container->register('n1')
->setArguments([
new Reference('serializer.normalizer.property'),
])
->addTag('serializer.normalizer', ['serializer' => 'api']);

$container->register('e1')->addTag('serializer.encoder', ['serializer' => ['*']]);

$serializerPass = new SerializerPass();
$serializerPass->process($container);

$this->assertEquals(new Reference('serializer.normalizer.property'), $container->getDefinition('n0')->getArgument(0));
$this->assertEquals(new Reference('serializer.normalizer.property.api'), $container->getDefinition('n1.api')->getArgument(0));
}
}