[Serializer] ObjectNormalizer #13257

dunglas · 2015-01-04T21:41:13Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
License	MIT
Doc PR	not yet

PropertyAccessNormalizer is a new normalizer leveraging the PropertyAccess Component. It is able to handle classes containing both public properties and properties only accessibles trough getters / setters / issers / hassers...

As it extends AbstractNormalizer, it supports circular reference handling, name converters and existing object population.
What do you think about making this new normalizer the default one as it's the most convenient to use and the most consistent with the behavior of other components.
#13120, #13252 and #13255 need to be merged to make this PR working.

dunglas · 2015-01-06T22:10:13Z

If you want to play with this new normalizer (or hack it), I've created a branch including all Serializer PR waiting to be merged (including this one): https://github.com/dunglas/symfony/tree/new_serializer

To use it in a standard edition install, add the following lines to your composer.json:

    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/dunglas/symfony"
        }
    ],

Then replace with Symfony line to use this experimental branch:

        "symfony/symfony": "dev-new_serializer as 2.7.0",

mickaelandrieu · 2015-01-06T22:41:10Z

hi @dunglas, can you provide a practical sample of what this feature can do ?

dunglas · 2015-01-06T23:06:33Z

class MyObj
{
    public $foo = 'foo';

    public getBar()
    {
       return 'bar';
    }

    public isBaz()
    {
        return true;
    }
}

$normalizer = new \Symfony\Component\Serializer\Normalizer\PropertyAccessNormalizer();
var_dump($normalizer->normalize(new MyObj()));
// Should output something like ['foo' => 'foo', 'bar' => 'bar', 'baz' => true]

dunglas · 2015-01-06T23:14:42Z

This normalizer also supports denormalization (for gettters / setters and properties), serialization groups (http://symfony.com/blog/new-in-symfony-2-7-serialization-groups), name converters (symfony/symfony-docs#4692), circular references handlers (symfony/symfony-docs#4299) and existing object population (#13252).

fabpot · 2015-01-07T06:51:26Z

👍

The only "issue" I see is the name; I understand that you named your class after the property access component but I find it confusing as at first, I thought it was about "just" supporting class properties. If it becomes the default one (which I think is a good idea), we should find a better name.

dunglas · 2015-01-07T07:10:31Z

I agree, the name is confusing. What about ObjectNormalizer?

fabpot · 2015-01-07T07:17:49Z

@dunglas If this is the "definitive" serializer, that sounds much better to me.

mickaelandrieu · 2015-01-07T09:15:03Z

totaly agree with @fabpot , this should become the default serializer!

xabbuh · 2015-01-07T13:22:51Z

src/Symfony/Component/Serializer/composer.json

@@ -28,7 +28,8 @@
        "doctrine/annotations": "For using the annotation mapping. You will also need doctrine/cache.",
        "doctrine/cache": "For using the default cached annotation reader and metadata cache.",
        "symfony/yaml": "For using the default YAML mapping loader.",
-        "symfony/config": "For using the XML mapping loader."
+        "symfony/config": "For using the XML mapping loader.",
+        "symfony/property-access": "For using the property access normalizer."


Shouldn't this be moved to require if it becomes the default normalizer?

This should be the default serializer in the full stack framework, but please don't add more "hard" dependencies between (standalone) components !

@mickaelandrieu 👍!

@mickaelandrieu good point

dunglas · 2015-01-17T09:10:20Z

What do you think about moving the code listing accessible "properties" (trough public properties and methods) - https://github.com/dunglas/symfony/blob/seriaizer_property_access_normalizer/src/Symfony/Component/Serializer/Normalizer/ObjectNormalizer.php#L62-L89 - in the PropertyAccess Component?

It can be useful in other contexts (in fact, I've an use case in a private project right now). cc @fabpot @webmozart

stof · 2015-01-18T17:41:12Z

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

+use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory;
+use Symfony\Component\Serializer\Tests\Fixtures\GroupDummy;
+
+require_once __DIR__.'/../../Annotation/Groups.php';


you should rely on autoloading instead

dunglas · 2015-02-04T13:01:16Z

Rebased. Should be ready for the merge now.

mickaelandrieu · 2015-02-04T13:51:00Z

src/Symfony/Component/Serializer/composer.json

@@ -28,7 +29,8 @@
        "doctrine/annotations": "For using the annotation mapping. You will also need doctrine/cache.",
        "doctrine/cache": "For using the default cached annotation reader and metadata cache.",
        "symfony/yaml": "For using the default YAML mapping loader.",
-        "symfony/config": "For using the XML mapping loader."
+        "symfony/config": "For using the XML mapping loader.",
+        "symfony/property-access": "For using the ObjectNormalizer."


👍 thanks a lot!

dunglas · 2015-02-22T17:57:11Z

I've fixed tests with components=low. Is there any more work to be done before getting this PR merged?

jderusse · 2015-02-22T21:20:54Z

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

+                continue;
+            }
+
+            $attributeValue = $this->propertyAccessor->getValue($object, $attribute);


Is it on purpose you don't use an else block on the next statement? It can avoid a call to the this->propertyAccessor->getValue

@jeremy-derusse I don't understand what you mean.

You can replace

$attributeValue = $this->propertyAccessor->getValue($object, $attribute); if (array_key_exists($attribute, $this->callbacks)) { $attributeValue = call_user_func($this->callbacks[$attribute], $attributeValue); }

by

if (array_key_exists($attribute, $this->callbacks)) { $attributeValue = call_user_func($this->callbacks[$attribute], $attributeValue); } else { $attributeValue = $this->propertyAccessor->getValue($object, $attribute); }

and avoid to call $this->propertyAccessor->getValue($object, $attribute); when array_key_exists($attribute, $this->callbacks) is false.

No because $attributeValue in $attributeValue = call_user_func($this->callbacks[$attribute], $attributeValue); will be undefined.

Or worse, the value of the previous loop iteration.

Ok I got it. But still no because the attribute value will never be retrieved. We need it even if a callback is not registered.

dunglas · 2015-03-03T14:59:42Z

Rebased.

dunglas · 2015-03-03T17:59:18Z

ping @symfony/deciders

This PR was merged into the 2.7 branch. Discussion ---------- [Serializer] ObjectNormalizer | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT | Doc PR | not yet `PropertyAccessNormalizer` is a new normalizer leveraging the PropertyAccess Component. It is able to handle classes containing both public properties and properties only accessibles trough getters / setters / issers / hassers... As it extends `AbstractNormalizer`, it supports circular reference handling, name converters and existing object population. What do you think about making this new normalizer the default one as it's the most convenient to use and the most consistent with the behavior of other components. #13120, #13252 and #13255 need to be merged to make this PR working. Commits ------- 0050bbb [Serializer] Introduce ObjectNormalizer

mickaelandrieu · 2015-03-06T11:09:50Z

👍 thanks for this improvement @dunglas !

…nnotations (dunglas) This PR was merged into the 2.7 branch. Discussion ---------- [Serializer] Supports hassers and setters for groups annotations | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a For more coherence with the new `ObjectNormalizer` (#13257), this PR allows to add `@Groups` annotations on hasser and setter methods too. Commits ------- 9c87ecc [Serializer] Supports hassers and setters for groups annotations

xabbuh reviewed Jan 7, 2015
View reviewed changes

fabpot force-pushed the 2.7 branch from 77b6cca to dcfc338 Compare January 8, 2015 13:06

fabpot added the Serializer label Jan 16, 2015

stof reviewed Jan 18, 2015
View reviewed changes

dunglas mentioned this pull request Jan 22, 2015

[Serializer] Manage php JsonSerializable #13496

Closed

dunglas force-pushed the seriaizer_property_access_normalizer branch 4 times, most recently from b344af7 to 2f69022 Compare February 3, 2015 07:10

dunglas changed the title ~~[Serializer] PropertyAccessNormalizer~~ [Serializer] ObjectNormalizer Feb 3, 2015

dunglas force-pushed the seriaizer_property_access_normalizer branch 4 times, most recently from 7531332 to e1cf645 Compare February 4, 2015 12:59

mickaelandrieu reviewed Feb 4, 2015
View reviewed changes

jderusse reviewed Feb 22, 2015
View reviewed changes

dunglas force-pushed the seriaizer_property_access_normalizer branch from a562e42 to a4b6421 Compare March 1, 2015 17:04

dunglas force-pushed the seriaizer_property_access_normalizer branch 3 times, most recently from fca94f2 to 6a12f1e Compare March 3, 2015 14:59

dunglas mentioned this pull request Mar 3, 2015

[FrameworkBundle] Serializer groups support #13107

Closed

[Serializer] Introduce ObjectNormalizer

0050bbb

dunglas force-pushed the seriaizer_property_access_normalizer branch from 6a12f1e to 0050bbb Compare March 6, 2015 10:53

dunglas merged commit 0050bbb into symfony:2.7 Mar 6, 2015

dunglas mentioned this pull request Apr 2, 2015

[Serializer] Supports hassers and setters for groups annotations #14187

Merged

dunglas deleted the seriaizer_property_access_normalizer branch December 5, 2015 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serializer] ObjectNormalizer #13257

[Serializer] ObjectNormalizer #13257

dunglas commented Jan 4, 2015

dunglas commented Jan 6, 2015

mickaelandrieu commented Jan 6, 2015

dunglas commented Jan 6, 2015

dunglas commented Jan 6, 2015

fabpot commented Jan 7, 2015

dunglas commented Jan 7, 2015

fabpot commented Jan 7, 2015

mickaelandrieu commented Jan 7, 2015

xabbuh Jan 7, 2015

mickaelandrieu Jan 7, 2015

dunglas Jan 7, 2015

xabbuh Jan 8, 2015

dunglas commented Jan 17, 2015

stof Jan 18, 2015

dunglas commented Feb 4, 2015

mickaelandrieu Feb 4, 2015

dunglas commented Feb 22, 2015

jderusse Feb 22, 2015

dunglas Mar 1, 2015

jderusse Mar 1, 2015

dunglas Mar 1, 2015

dunglas Mar 1, 2015

dunglas Mar 1, 2015

dunglas commented Mar 3, 2015

dunglas commented Mar 3, 2015

mickaelandrieu commented Mar 6, 2015

[Serializer] ObjectNormalizer #13257

[Serializer] ObjectNormalizer #13257

Conversation

dunglas commented Jan 4, 2015

dunglas commented Jan 6, 2015

mickaelandrieu commented Jan 6, 2015

dunglas commented Jan 6, 2015

dunglas commented Jan 6, 2015

fabpot commented Jan 7, 2015

dunglas commented Jan 7, 2015

fabpot commented Jan 7, 2015

mickaelandrieu commented Jan 7, 2015

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 Jan 17, 2015

Choose a reason for hiding this comment

dunglas commented Feb 4, 2015

Choose a reason for hiding this comment

dunglas commented Feb 22, 2015

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 commented Mar 3, 2015

dunglas commented Mar 3, 2015

mickaelandrieu commented Mar 6, 2015