Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Serializer] Deprecate support for returning empty, iterable, countable, raw object when normalizing#42619

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:5.4fromlyrixx:serializer-deprec-nested-object
Aug 23, 2021

Conversation

@lyrixx
Copy link
Member

QA
Branch?5.4
Bug fix?no
New feature?no
Deprecations?yes
Tickets
LicenseMIT
Doc PR

@nicolas-grekas
Copy link
Member

I propose to apply the patch below on top of this PR.

Then we might need a new test case for the legacy code path.

diff --git a/src/Symfony/Component/Serializer/Serializer.php b/src/Symfony/Component/Serializer/Serializer.phpindex 1e4fb793e8..08f8e960ff 100644--- a/src/Symfony/Component/Serializer/Serializer.php+++ b/src/Symfony/Component/Serializer/Serializer.php@@ -169,7 +169,7 @@ class Serializer implements SerializerInterface, ContextAwareNormalizerInterface                 switch (true) {                     case ($context[AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS] ?? false) && \is_object($data):                         if (!$data instanceof \ArrayObject) {-                            trigger_deprecation('symfony/serializer', '5.4', 'The method "%s()" will return an instance of "%s" as of Symfony 6.0 when the object is iteratable, countable and empty.', __METHOD__, \ArrayObject::class);+                            trigger_deprecation('symfony/serializer', '5.4', 'Returning empty object of class "%s" from "%s()" is deprecated unless it extends "ArrayObject".', get_debug_type($data), __METHOD__);                         }                          return $data;diff --git a/src/Symfony/Component/Serializer/Tests/SerializerTest.php b/src/Symfony/Component/Serializer/Tests/SerializerTest.phpindex 2afbd51c22..ab74b06ab0 100644--- a/src/Symfony/Component/Serializer/Tests/SerializerTest.php+++ b/src/Symfony/Component/Serializer/Tests/SerializerTest.php@@ -578,10 +578,7 @@ class SerializerTest extends TestCase         $this->assertSame($expected, $serializer->serialize($data, 'json'));     }-    /**-     * @dataProvider provideObjectOrCollectionTests-     * @group legacy-     */+    /** @dataProvider provideObjectOrCollectionTests */     public function testNormalizePreserveEmptyArrayObject(Serializer $serializer, array $data)     {         $expected = '{"a1":{},"a2":{"k":"v"},"b1":[],"b2":{"k":"v"},"c1":{"nested":{}},"c2":{"nested":{"k":"v"}},"d1":{"nested":[]},"d2":{"nested":{"k":"v"}},"e1":{"map":[]},"e2":{"map":{"k":"v"}},"f1":{"map":{}},"f2":{"map":{"k":"v"}},"g1":{"list":{"list":[]},"settings":[]},"g2":{"list":["greg"],"settings":[]}}';@@ -599,10 +596,7 @@ class SerializerTest extends TestCase         ]));     }-    /**-     * @dataProvider provideObjectOrCollectionTests-     * @group legacy-     */+    /** @dataProvider provideObjectOrCollectionTests */     public function testNormalizeEmptyArrayAsObjectAndPreserveEmptyArrayObject(Serializer $serializer, array $data)     {         $expected = '{"a1":{},"a2":{"k":"v"},"b1":{},"b2":{"k":"v"},"c1":{"nested":{}},"c2":{"nested":{"k":"v"}},"d1":{"nested":{}},"d2":{"nested":{"k":"v"}},"e1":{"map":{}},"e2":{"map":{"k":"v"}},"f1":{"map":{}},"f2":{"map":{"k":"v"}},"g1":{"list":{"list":[]},"settings":{}},"g2":{"list":["greg"],"settings":{}}}';@@ -797,13 +791,15 @@ class Baz     } }-class DummyList implements \Countable, \IteratorAggregate+class DummyList extends \ArrayObject {     public $list;      public function __construct(array $list)     {         $this->list = $list;++        $this->setFlags(\ArrayObject::STD_PROP_LIST);     }      public function count(): int

@lyrixx
Copy link
MemberAuthor

Could we find a better deprecation message ?

Returning empty object of class "%s" from "%s()" is deprecated unless it extends "ArrayObject".

With the tests suite fixtures it gives

Since symfony/serializer 5.4: Returning empty object of class "Symfony\Component\Serializer\Tests\DummyList" from "Symfony\Component\Serializer\Serializer::normalize()" is deprecated unless it extends "ArrayObject".

I'm not sure people understand what it means

@lyrixx
Copy link
MemberAuthor

And BTW, there is something strange here. It's legit to have class thatimplements \Countable, \IteratorAggregate. And it may not be possible to extendsArrayObject. So we are forcing people to do deep change, for something that will be fixed in 6.0.

Finally, I do think this is a very edge case, since the code is ATM broken. So no one could rely on it.

(So... I could update my PR with you patch, since I think nobody will see the message :p)

@nicolas-grekas
Copy link
Member

[...] is deprecated. This class should extend "ArrayObject".?

@lyrixxlyrixxforce-pushed theserializer-deprec-nested-object branch from54a3436 to806bb8fCompareAugust 23, 2021 08:47
@lyrixx
Copy link
MemberAuthor

@nicolas-grekas Should be OK 👍🏼

@fabpot
Copy link
Member

Thank you@lyrixx.

@fabpotfabpot merged commit6d70316 intosymfony:5.4Aug 23, 2021
@lyrixxlyrixx deleted the serializer-deprec-nested-object branchAugust 23, 2021 10:06
fabpot added a commit that referenced this pull requestAug 25, 2021
…jects when PRESERVE_EMPTY_OBJECTS is set (nicolas-grekas)This PR was merged into the 5.4 branch.Discussion----------[Serializer] Return an ArrayObject for empty collection objects when PRESERVE_EMPTY_OBJECTS is set| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -In#42619, we tried adding a BC layer for this behavior change, but when reviewing#42699, I figured out there is no possible BC layer: userland implementations (eg an entity returning an empty doctrine collection) won't extend ArrayObject because we ask them to do so.I therefor propose to return an ArrayObject as of 5.4, relying on the intuition that this should not have much BC impact in practice./cc `@lyrixx`Commits-------7856fe7 [Serializer] Return an ArrayObject for empty collection objects when PRESERVE_EMPTY_OBJECTS is set
This was referencedNov 5, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

4 participants

@lyrixx@nicolas-grekas@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp