[Serializer] Add `name_converter` flag and use an array of name converters #35374

alanpoulain · 2020-01-17T17:23:44Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	yes
Tickets	N/A
License	MIT
Doc PR	To do

The aim of this PR is to bring the possibility to modify the name converter to use at runtime.

Imagine you use the classical serializer for your (de)normalization needs (if you use some DTOs for instance). You also need to call an API using snake case and you want to use the serializer too.

In the current state, you need to do something like this:

services:
    app.serializer.normalizer.object.snake_case:
        class: Symfony\Component\Serializer\Normalizer\ObjectNormalizer
        autoconfigure: false
        arguments:
            $nameConverter: '@serializer.name_converter.camel_case_to_snake_case'

    app.serializer.denormalizer.array.snake_case:
        class: Symfony\Component\Serializer\Normalizer\ArrayDenormalizer
        autoconfigure: false

    app.serializer.normalizer.datetime.snake_case:
        class: Symfony\Component\Serializer\Normalizer\DateTimeNormalizer
        autoconfigure: false

    app.serializer.encoder.json.snake_case:
        class: Symfony\Component\Serializer\Encoder\JsonEncoder
        autoconfigure: false

    app.serializer.snake_case:
        class: Symfony\Component\Serializer\Serializer
        autoconfigure: false
        arguments:
            - ['@app.serializer.normalizer.datetime.snake_case', '@app.serializer.normalizer.object.snake_case', '@app.serializer.denormalizer.array.snake_case']
            - ['@app.serializer.encoder.json.snake_case']

You have to reconfigure all the (de)normalizers because if you use the ones from Symfony in the declaration of your snake case serializer, they can use the snake case serializer instead of the classical one when you (de)normalize your data (mainly because of this:

symfony/src/Symfony/Component/Serializer/Serializer.php

Lines 77 to 87 in af4035d

    
           if ($normalizer instanceof SerializerAwareInterface) { 
        
               $normalizer->setSerializer($this); 
        
           } 
        
           if ($normalizer instanceof DenormalizerAwareInterface) { 
        
               $normalizer->setDenormalizer($this); 
        
           } 
        
           if ($normalizer instanceof NormalizerAwareInterface) { 
        
               $normalizer->setNormalizer($this); 
        
           }

).

With this PR, you can just use the classical serializer with a flag given to the context:

$data = $this->normalizer->normalize($object, null, [
    AbstractNormalizer::NAME_CONVERTER => CamelCaseToSnakeCaseNameConverter::class,
]);

alanpoulain · 2020-01-17T23:39:35Z

src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php

    /**
     * Sets the {@link ClassMetadataFactoryInterface} to use.
     */
-    public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, NameConverterInterface $nameConverter = null, array $defaultContext = [])
+    public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, /* array */ $nameConverter/*s*/ = [], array $defaultContext = [])


I'm not sure if it's possible to change or deprecate the name of a constructor argument since it could be used by DI with a named argument.

alanpoulain · 2020-01-17T23:41:20Z

src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php

+            $this->nameConverter = $nameConverter;
+        } else {
+            foreach ($nameConverter as $nameConverterInstance) {
+                $this->nameConverters[\get_class($nameConverterInstance)] = $nameConverterInstance;


Is it a good idea to use the FQCN for referencing a name converter?
We could also use the service id.

nope, it's not: it prevents passing two differently configured instances of the same class
instead, why not pass a service locator?

We could use a service locator too.
We would keep the name converter and we would add one more argument, am I right?
Or maybe we can replace the name converter with the service locator. I'm not sure to like it since it means not having a clear dependency between the normalizer and the name converter and having to use a default name converter service id.

alanpoulain · 2020-01-17T23:44:02Z

src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php

-                $key = $this->nameConverter ? $this->nameConverter->normalize($paramName, $class, $format, $context) : $paramName;
+                $key = $paramName;
+                if (\count($this->nameConverters) > 0) {
+                    $nameConverter = $this->nameConverters[$context[static::NAME_CONVERTER] ?? null] ?? reset($this->nameConverters);


If the given name converter in context doesn't correspond to one of the name converters, the first name converter is used instead.
The user is not warned, but I think it's the right approach. WDYT?

an exception should be thrown instead to me
having a service locator would naturally throw a service-bot-found exception btw.

alanpoulain · 2020-01-17T23:47:40Z

Low deps tests are failing in Travis but it seems logical to me (since the modified configuration of the serializer cannot be applied to an old version of the code).
Failures in AppVeyor doesn't seem related.

nicolas-grekas

Did you consider passing a name converter instance as a context option?

nicolas-grekas · 2020-01-20T11:56:21Z

src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php

+            $this->nameConverter = $nameConverter;
+        } else {
+            foreach ($nameConverter as $nameConverterInstance) {
+                $this->nameConverters[\get_class($nameConverterInstance)] = $nameConverterInstance;


nope, it's not: it prevents passing two differently configured instances of the same class
instead, why not pass a service locator?

nicolas-grekas · 2020-01-20T11:56:44Z

src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php

-                $key = $this->nameConverter ? $this->nameConverter->normalize($paramName, $class, $format, $context) : $paramName;
+                $key = $paramName;
+                if (\count($this->nameConverters) > 0) {
+                    $nameConverter = $this->nameConverters[$context[static::NAME_CONVERTER] ?? null] ?? reset($this->nameConverters);


an exception should be thrown instead to me
having a service locator would naturally throw a service-bot-found exception btw.

nicolas-grekas · 2020-01-20T11:58:09Z

src/Symfony/Component/Serializer/Tests/Normalizer/GetSetMethodNormalizerTest.php

@@ -323,7 +390,7 @@ protected function getNormalizerForMaxDepth(): NormalizerInterface
    protected function getDenormalizerForObjectToPopulate(): DenormalizerInterface
    {
        $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
-        $normalizer = new GetSetMethodNormalizer($classMetadataFactory, null, new PhpDocExtractor());
+        $normalizer = new GetSetMethodNormalizer($classMetadataFactory, [], new PhpDocExtractor());


null should still be accepted, no need to deprecate it.

Why would you want to keep passing null? What would it mean?

unnecessary deprecations should be avoided, that's my point.

nicolas-grekas · 2020-01-20T11:58:26Z

UPGRADE-5.1.md

+Serializer
+----------
+
+ * Deprecated passing a name converter directly to the second argument of the constructor of `AbstractNormalizer`, pass an array of name converters instead.


why? it looks fine to me

alanpoulain · 2020-01-20T13:07:42Z

I did consider passing an instance in the context but it would not seem clean to me. It has the advantage of not deprecating anything though.

nicolas-grekas · 2020-01-20T14:17:27Z

I did consider passing an instance in the context but it would not seem clean to me. It has the advantage of not deprecating anything though.

Actually it would be cleaner: the current approach requires some "long-distance" correlation between what is provided to the constructor and what string is passed in the context. That's quite fragile, with the same drawbacks as documented in "service locators are bad" articles.

But then, there is this cacheability consideration. To be investigated.

dunglas · 2020-01-22T13:18:53Z

For such cases, instead of introducing such complexity, wouldn't it be better to use several Serializer instances? One instance to serialize your internal data structure, one other to deserialize the data coming from an external API?

I would like to have a factory mechanism to create as many serializers as needed in FrameworkBundle, as we do for Cache. It would also allow to improve the overall performance of the Serializer by registering less useless normalizers for a given instance.

alanpoulain · 2020-01-22T13:50:21Z

@dunglas That's the issue here. Defining a specific serializer is cumbersome and can lead to many issues if not defined properly (the main issue being the necessity to redefine all the normalizers and encoders you need to use, see my explanation in the description of the PR).

Moreover I think it's not very clear for a newcomer to understand how changing the name converter can be done (the name converter is not defined in the serializer but in the object normalizer).

dunglas · 2020-02-05T01:14:32Z

What you're trying to achieve here can already be done user-land by creating a "smart" name converter implementing the AdvancedNameConverterInterface. It can use a chain of responsibility pattern or something similar to select how to normalize the property names depending of the context.

Also, I've the feeling that it's not the good way to fix this issue. What we need is an easy way to create several serializer instances, not introducing unneeded complexity in the component.

I'm closing for now. Thanks for your PR anyway. Don't hesitate to reopen if I'm missing the point.

krewetka · 2021-02-24T09:15:04Z

@dunglas :

What we need is an easy way to create several serializer instances, not introducing unneeded complexity in the component.

is there maybe any way to achive it already now in newest version of symfony?

I also have use case with 2-3 separate serializers needed ( with different name converters) -> one for consuming one external API, another for another integration + another one for exposing our own API

This PR was squashed before being merged into the 7.2 branch. Discussion ---------- [Serializer] Introduce named serializers | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT The idea behind this PR is to allow configuring multiple serializer instances with different default contexts, name converters, and sets of normalizers and encoders. This is useful when your application is communicating with multiple APIs, each with different rules. Similar ideas have been mentioned [before](#35374 (comment)). ```yaml serializer: named_serializers: api1: name_converter: 'serializer.name_converter.camel_case_to_snake_case' default_context: enable_max_depth: true api2: default_context: enable_max_depth: false ``` The different serializers can be injected using named aliases: ```php #[AsController] class HomeController { #[Route('/', name: 'app_home')] public function index( SerializerInterface $serializer, // Default serializer SerializerInterface $api1Serializer, // api1 serializer #[Target('api2.serializer')] // api2 serializer SerializerInterface $someName, ) { // ... } } ``` Multiple normalizer/encoder instances with different arguments are created as child services of the default ones. ~~I also ensured that the same normalizer/encoder instances are reused between different serializers that have the same default context and name converter to minimize the number of child services created.~~ Custom normalizers/encoders can target specific serializers using the `serializer` tag attribute: ```yaml get_set_method_normalizer: class: Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer autoconfigure: false # This is needed so that it's not included in the default serializer tags: # single serializer - serializer.normalizer: { serializer: 'api1' } # or multiple ones - serializer.normalizer: { serializer: [ 'api1', 'api2' ] } # use * to include the service in all serializers including the default one - serializer.normalizer: { serializer: '*' } ``` For BC reasons, not setting the `serializer` tag attribute is the same as setting it to the default one: ```yaml tags: - serializer.normalizer # same as - serializer.normalizer: { serializer: 'default' } ``` The profiler has been updated to support multiple serializer instances: ![image](https://github.com/symfony/symfony/assets/2445045/0468ccf3-8d5a-4c60-a639-09cfe918b163) All normalizers/encoders are tagged with additional named serializer specific tags to help with debugging. To get the priority of normalizers/encoders for a certain serializer, use the `serializer.normalizer.<name>` or `serializer.encoder.<name>` tags: ```shell $ bin/console debug:container --tag serializer.normalizer.default Symfony Container Services Tagged with "serializer.normalizer.default" Tag ========================================================================== ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- Service ID priority Class name ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- serializer.denormalizer.unwrapping 1000 Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer serializer.normalizer.flatten_exception -880 Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer ... serializer.denormalizer.array -990 Symfony\Component\Serializer\Normalizer\ArrayDenormalizer serializer.normalizer.object -1000 Symfony\Component\Serializer\Normalizer\ObjectNormalizer ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- $ bin/console debug:container --tag serializer.normalizer.api1 Symfony Container Services Tagged with "serializer.normalizer.api1" Tag ======================================================================= ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- Service ID priority Class name ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- serializer.denormalizer.unwrapping 1000 Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer get_set_method_normalizer Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer serializer.normalizer.flatten_exception -880 Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer ... serializer.denormalizer.array -990 Symfony\Component\Serializer\Normalizer\ArrayDenormalizer serializer.normalizer.object -1000 Symfony\Component\Serializer\Normalizer\ObjectNormalizer ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- $ bin/console debug:container --tag serializer.normalizer Symfony Container Services Tagged with "serializer.normalizer" Tag ================================================================== ------------------------------------------------- ---------- ---------- ----------------- ------------------------------------------------------------------------------------------- Service ID standard priority serializer Class name ------------------------------------------------- ---------- ---------- ----------------- ------------------------------------------------------------------------------------------- serializer.denormalizer.unwrapping 1 1000 Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer get_set_method_normalizer ["api1","api2"] Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer serializer.normalizer.flatten_exception 1 -880 Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer ... serializer.denormalizer.array 1 -990 Symfony\Component\Serializer\Normalizer\ArrayDenormalizer serializer.normalizer.object 1 -1000 Symfony\Component\Serializer\Normalizer\ObjectNormalizer ------------------------------------------------- ---------- ---------- ----------------- ------------------------------------------------------------------------------------------- ``` Since Symfony comes with some pre-registered normalizers and encoders, I added options to exclude those in case someone wants to use only custom ones: ```yaml serializer: named_serializers: api1: include_built_in_normalizers: false include_built_in_encoders: true name_converter: 'serializer.name_converter.camel_case_to_snake_case' default_context: enable_max_depth: true ``` TBH, I have doubts about the usefulness of this, please let me know your thoughts. I've split the PR into two commits to ease reviewing: - the first commit only rearranges the `SerializerPass` without adding any features - the second commit implements the feature Commits ------- 29bd8ce [Serializer] Introduce named serializers

This PR was squashed before being merged into the 7.2 branch. Discussion ---------- [Serializer] Introduce named serializers | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT The idea behind this PR is to allow configuring multiple serializer instances with different default contexts, name converters, and sets of normalizers and encoders. This is useful when your application is communicating with multiple APIs, each with different rules. Similar ideas have been mentioned [before](symfony/symfony#35374 (comment)). ```yaml serializer: named_serializers: api1: name_converter: 'serializer.name_converter.camel_case_to_snake_case' default_context: enable_max_depth: true api2: default_context: enable_max_depth: false ``` The different serializers can be injected using named aliases: ```php #[AsController] class HomeController { #[Route('/', name: 'app_home')] public function index( SerializerInterface $serializer, // Default serializer SerializerInterface $api1Serializer, // api1 serializer #[Target('api2.serializer')] // api2 serializer SerializerInterface $someName, ) { // ... } } ``` Multiple normalizer/encoder instances with different arguments are created as child services of the default ones. ~~I also ensured that the same normalizer/encoder instances are reused between different serializers that have the same default context and name converter to minimize the number of child services created.~~ Custom normalizers/encoders can target specific serializers using the `serializer` tag attribute: ```yaml get_set_method_normalizer: class: Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer autoconfigure: false # This is needed so that it's not included in the default serializer tags: # single serializer - serializer.normalizer: { serializer: 'api1' } # or multiple ones - serializer.normalizer: { serializer: [ 'api1', 'api2' ] } # use * to include the service in all serializers including the default one - serializer.normalizer: { serializer: '*' } ``` For BC reasons, not setting the `serializer` tag attribute is the same as setting it to the default one: ```yaml tags: - serializer.normalizer # same as - serializer.normalizer: { serializer: 'default' } ``` The profiler has been updated to support multiple serializer instances: ![image](https://github.com/symfony/symfony/assets/2445045/0468ccf3-8d5a-4c60-a639-09cfe918b163) All normalizers/encoders are tagged with additional named serializer specific tags to help with debugging. To get the priority of normalizers/encoders for a certain serializer, use the `serializer.normalizer.<name>` or `serializer.encoder.<name>` tags: ```shell $ bin/console debug:container --tag serializer.normalizer.default Symfony Container Services Tagged with "serializer.normalizer.default" Tag ========================================================================== ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- Service ID priority Class name ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- serializer.denormalizer.unwrapping 1000 Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer serializer.normalizer.flatten_exception -880 Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer ... serializer.denormalizer.array -990 Symfony\Component\Serializer\Normalizer\ArrayDenormalizer serializer.normalizer.object -1000 Symfony\Component\Serializer\Normalizer\ObjectNormalizer ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- $ bin/console debug:container --tag serializer.normalizer.api1 Symfony Container Services Tagged with "serializer.normalizer.api1" Tag ======================================================================= ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- Service ID priority Class name ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- serializer.denormalizer.unwrapping 1000 Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer get_set_method_normalizer Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer serializer.normalizer.flatten_exception -880 Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer ... serializer.denormalizer.array -990 Symfony\Component\Serializer\Normalizer\ArrayDenormalizer serializer.normalizer.object -1000 Symfony\Component\Serializer\Normalizer\ObjectNormalizer ------------------------------------------------- ---------- ------------------------------------------------------------------------------------------- $ bin/console debug:container --tag serializer.normalizer Symfony Container Services Tagged with "serializer.normalizer" Tag ================================================================== ------------------------------------------------- ---------- ---------- ----------------- ------------------------------------------------------------------------------------------- Service ID standard priority serializer Class name ------------------------------------------------- ---------- ---------- ----------------- ------------------------------------------------------------------------------------------- serializer.denormalizer.unwrapping 1 1000 Symfony\Component\Serializer\Normalizer\UnwrappingDenormalizer get_set_method_normalizer ["api1","api2"] Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer serializer.normalizer.flatten_exception 1 -880 Symfony\Component\Messenger\Transport\Serialization\Normalizer\FlattenExceptionNormalizer ... serializer.denormalizer.array 1 -990 Symfony\Component\Serializer\Normalizer\ArrayDenormalizer serializer.normalizer.object 1 -1000 Symfony\Component\Serializer\Normalizer\ObjectNormalizer ------------------------------------------------- ---------- ---------- ----------------- ------------------------------------------------------------------------------------------- ``` Since Symfony comes with some pre-registered normalizers and encoders, I added options to exclude those in case someone wants to use only custom ones: ```yaml serializer: named_serializers: api1: include_built_in_normalizers: false include_built_in_encoders: true name_converter: 'serializer.name_converter.camel_case_to_snake_case' default_context: enable_max_depth: true ``` TBH, I have doubts about the usefulness of this, please let me know your thoughts. I've split the PR into two commits to ease reviewing: - the first commit only rearranges the `SerializerPass` without adding any features - the second commit implements the feature Commits ------- 29bd8ce6c2 [Serializer] Introduce named serializers

Use an array of name converters instead of a single one

196c846

alanpoulain requested a review from dunglas as a code owner January 17, 2020 17:23

carsonbot added Status: Needs Review Serializer Feature Deprecation labels Jan 17, 2020

alanpoulain commented Jan 17, 2020

View reviewed changes

nicolas-grekas added this to the next milestone Jan 20, 2020

nicolas-grekas reviewed Jan 20, 2020

View reviewed changes

nicolas-grekas added Status: Needs Work and removed Status: Needs Review labels Jan 27, 2020

dunglas closed this Feb 5, 2020

alanpoulain mentioned this pull request Mar 18, 2020

[Serializer] Misconfiguration when multiple serializers are defined #36113

Closed

nicolas-grekas modified the milestones: next, 5.1 May 4, 2020

HypeMC mentioned this pull request May 20, 2024

[Serializer] Introduce named serializers #56823

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serializer] Add `name_converter` flag and use an array of name converters #35374

[Serializer] Add `name_converter` flag and use an array of name converters #35374

alanpoulain commented Jan 17, 2020

alanpoulain Jan 17, 2020

alanpoulain Jan 17, 2020

nicolas-grekas Jan 20, 2020

alanpoulain Jan 20, 2020

alanpoulain Jan 17, 2020

nicolas-grekas Jan 20, 2020

alanpoulain commented Jan 17, 2020

nicolas-grekas left a comment

nicolas-grekas Jan 20, 2020

nicolas-grekas Jan 20, 2020

nicolas-grekas Jan 20, 2020

alanpoulain Jan 20, 2020

nicolas-grekas Jan 20, 2020 •

edited

Loading

alanpoulain Jan 20, 2020

nicolas-grekas Jan 20, 2020

alanpoulain commented Jan 20, 2020

nicolas-grekas commented Jan 20, 2020 •

edited

Loading

dunglas commented Jan 22, 2020 •

edited

Loading

alanpoulain commented Jan 22, 2020

dunglas commented Feb 5, 2020

krewetka commented Feb 24, 2021

	if ($normalizer instanceof SerializerAwareInterface) {
	$normalizer->setSerializer($this);
	}

	if ($normalizer instanceof DenormalizerAwareInterface) {
	$normalizer->setDenormalizer($this);
	}

	if ($normalizer instanceof NormalizerAwareInterface) {
	$normalizer->setNormalizer($this);
	}

[Serializer] Add name_converter flag and use an array of name converters #35374

[Serializer] Add name_converter flag and use an array of name converters #35374

Conversation

alanpoulain commented Jan 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanpoulain commented Jan 17, 2020

nicolas-grekas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas Jan 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanpoulain commented Jan 20, 2020

nicolas-grekas commented Jan 20, 2020 • edited Loading

dunglas commented Jan 22, 2020 • edited Loading

alanpoulain commented Jan 22, 2020

dunglas commented Feb 5, 2020

krewetka commented Feb 24, 2021

[Serializer] Add `name_converter` flag and use an array of name converters #35374

[Serializer] Add `name_converter` flag and use an array of name converters #35374

nicolas-grekas Jan 20, 2020 •

edited

Loading

nicolas-grekas commented Jan 20, 2020 •

edited

Loading

dunglas commented Jan 22, 2020 •

edited

Loading