[Serializer] Object class resolver #28669

alanpoulain · 2018-10-01T16:07:41Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

When normalizing an object, it could be useful to use a custom method to resolve the class name of it instead of using get_class.
For instance, Doctrine is using proxy classes for lazy-loading and we usually want the real class and not the proxied one.
That's why we are using this trait in API Platform: https://github.com/api-platform/core/blob/master/src/Util/ClassInfoTrait.php
With this feature, we could solve an issue in API Platform with the JSON-LD normalizer when the eager fetching is disabled.

nicolas-grekas · 2018-10-01T19:56:09Z

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

@@ -50,7 +51,12 @@
     */
    protected $classDiscriminatorResolver;

-    public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, NameConverterInterface $nameConverter = null, PropertyTypeExtractorInterface $propertyTypeExtractor = null, ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null)
+    /**
+     * @var ObjectClassResolverInterface|null


can't we make it a callable and remove the interface instead?

nicolas-grekas · 2018-10-01T19:56:22Z

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

+    /**
+     * @var ObjectClassResolverInterface|null
+     */
+    protected $objectClassResolver;


should be private

And the docblock can be removed

Can't it be useful to have it in the child classes?

it's still catchable by overriding the constructor in child classes
but we don't want to promise BC regarding any changes that could happen after instantiation.

dunglas · 2018-10-01T20:21:03Z

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

+    /**
+     * @var ObjectClassResolverInterface|null
+     */
+    protected $objectClassResolver;


And the docblock can be removed

dunglas · 2018-10-01T20:23:30Z

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

+     */
+    protected $objectClassResolver;
+
+    public function __construct(ClassMetadataFactoryInterface $classMetadataFactory = null, NameConverterInterface $nameConverter = null, PropertyTypeExtractorInterface $propertyTypeExtractor = null, ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, ObjectClassResolverInterface $objectClassResolver = null)


If you switch to a callable, you can just set '\get_class' as default value, then you can remove the ternary operator later.

Although that might be slower

alanpoulain · 2018-10-02T09:02:00Z

I have used a callable instead of an interface.
For the sake of performance, I have kept the ternary.

nicolas-grekas

(with minor comment)

nicolas-grekas · 2018-10-02T09:07:39Z

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

@@ -86,7 +88,7 @@ public function normalize($object, $format = null, array $context = array())
        $data = array();
        $stack = array();
        $attributes = $this->getAttributes($object, $format, $context);
-        $class = \get_class($object);
+        $class = $this->objectClassResolver ? \call_user_func($this->objectClassResolver, $object) : \get_class($object);


could be written as ($this->objectClassResolver)($object)

I've copied what was done for the maxDepthHandler.

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

Line 100 in 9610d10

$attributeValue = \call_user_func($this->maxDepthHandler, $attributeValue, $object, $attribute, $format, $context);

It seems to me there is no important difference between the two ways of doing it: https://stackoverflow.com/questions/1596221/php-call-user-func-vs-just-calling-function/35031466

($this->objectClassResolver)($object) is more elegant, but indeed being consistent is good

dunglas · 2018-10-03T20:34:56Z

Thanks @alanpoulain for working on this feature, this is much appreciated.

This PR was squashed before being merged into the 4.2-dev branch (closes #28669). Discussion ---------- [Serializer] Object class resolver | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | When normalizing an object, it could be useful to use a custom method to resolve the class name of it instead of using `get_class`. For instance, Doctrine is using proxy classes for lazy-loading and we usually want the real class and not the proxied one. That's why we are using this trait in API Platform: https://github.com/api-platform/core/blob/master/src/Util/ClassInfoTrait.php With this feature, we could solve an issue in API Platform with the JSON-LD normalizer when the eager fetching is disabled. Commits ------- 18d2143 [Serializer] Object class resolver

Taluu · 2018-11-03T15:37:15Z

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

@@ -60,6 +61,7 @@ public function __construct(ClassMetadataFactoryInterface $classMetadataFactory
            $classDiscriminatorResolver = new ClassDiscriminatorFromClassMetadata($classMetadataFactory);
        }
        $this->classDiscriminatorResolver = $classDiscriminatorResolver;
+        $this->objectClassResolver = $objectClassResolver;


I know it's late, but how about $this->objectClassResolver = $objectClassResolver >: '\get_class'; or something like that ? So that we don't need to test if it is not null on each call

@Taluu : You should provide a PR :)

alanpoulain requested a review from dunglas as a code owner October 1, 2018 16:07

carsonbot added Status: Needs Review Serializer Feature BC Break labels Oct 1, 2018

nicolas-grekas reviewed Oct 1, 2018

View reviewed changes

nicolas-grekas removed the BC Break label Oct 1, 2018

nicolas-grekas added this to the next milestone Oct 1, 2018

dunglas requested changes Oct 1, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Oct 1, 2018

alanpoulain force-pushed the serializer-object-class-resolver branch from fc6cb7d to eb5d8d4 Compare October 2, 2018 08:58

carsonbot removed the Status: Needs Work label Oct 2, 2018

dunglas approved these changes Oct 2, 2018

View reviewed changes

carsonbot added the Status: Reviewed label Oct 2, 2018

nicolas-grekas approved these changes Oct 2, 2018

View reviewed changes

[Serializer] Object class resolver

18d2143

dunglas force-pushed the serializer-object-class-resolver branch from eb5d8d4 to 18d2143 Compare October 3, 2018 20:34

dunglas merged commit 18d2143 into symfony:master Oct 3, 2018

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

This was referenced Nov 3, 2018

Release v4.2.0-BETA1 #29072

Closed

Release v4.2.0-BETA1 #29073

Merged

Taluu reviewed Nov 3, 2018

View reviewed changes

alanpoulain mentioned this pull request Nov 28, 2018

Graphql : Cannot return null for non-nullable field api-platform/api-platform#902

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serializer] Object class resolver #28669

[Serializer] Object class resolver #28669

alanpoulain commented Oct 1, 2018 •

edited by nicolas-grekas

Loading

nicolas-grekas Oct 1, 2018

nicolas-grekas Oct 1, 2018

dunglas Oct 1, 2018

alanpoulain Oct 2, 2018

nicolas-grekas Oct 2, 2018

dunglas Oct 1, 2018

dunglas Oct 1, 2018 •

edited

Loading

nicolas-grekas Oct 1, 2018

alanpoulain commented Oct 2, 2018

nicolas-grekas left a comment

nicolas-grekas Oct 2, 2018

alanpoulain Oct 2, 2018

dunglas Oct 2, 2018

dunglas commented Oct 3, 2018

Taluu Nov 3, 2018

ogizanagi Nov 3, 2018

[Serializer] Object class resolver #28669

[Serializer] Object class resolver #28669

Conversation

alanpoulain commented Oct 1, 2018 • edited by nicolas-grekas Loading

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

dunglas Oct 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanpoulain commented Oct 2, 2018

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

dunglas commented Oct 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanpoulain commented Oct 1, 2018 •

edited by nicolas-grekas

Loading

dunglas Oct 1, 2018 •

edited

Loading