Skip to content

[Serializer] do not transform empty \Traversable to Array #36601

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

Jump to bottom
Merged
merged 1 commit into from
May 1, 2020
Merged

[Serializer] do not transform empty \Traversable to Array #36601

merged 1 commit into from
May 1, 2020

Conversation

soyuka
Copy link
Contributor

@soyuka soyuka commented Apr 27, 2020 •
edited
Loading

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets na
License MIT
Doc PR na

Today, using PRESERVE_EMPTY_OBJECTS (introduced in 4.0), the JSON serialization of:

<?php
$object = [];
$object['foo'] = new \ArrayObject();
$object['bar'] = new \ArrayObject(['notempty']);
$object['baz'] = new \ArrayObject(['nested' => new \ArrayObject()]);

Outputs:

{"foo":[],"bar":["notempty"],"baz":{"nested":[]}}

Instead of the expected:

{"foo":{},"bar":["notempty"],"baz":{"nested":{}}}

This issue comes from the Serializer that transforms Traversable to an Array here. Also, the AbstractObjectNormalizer doesn't support Traversable, but he allows to preserve empty objects.

I propose this patch where the fix doesn't transform a Traversable to an Array. I see another way to patch this in which we could allow empty Traversable in the AbstractObjectNormalizer (not sure it's better though). See attached other-fix.patch to see the alternative patch.

@soyuka soyuka requested a review from dunglas as a code owner April 27, 2020 13:22
@soyuka soyuka changed the base branch from master to 4.4 April 27, 2020 13:22
@soyuka soyuka force-pushed the fix/serializer-preserve-empty-objects branch from 05c76bb to 8d2411b Compare April 27, 2020 13:28
@soyuka soyuka force-pushed the fix/serializer-preserve-empty-objects branch from 8d2411b to 1c979c0 Compare April 27, 2020 13:31
@soyuka soyuka force-pushed the fix/serializer-preserve-empty-objects branch from 1c979c0 to e5c2029 Compare April 27, 2020 14:09
@nicolas-grekas nicolas-grekas added this to the 4.4 milestone May 1, 2020
@nicolas-grekas nicolas-grekas changed the title Fix serializer do not transform empty \Traversable to Array [Serializer] do not transform empty \Traversable to Array May 1, 2020
@nicolas-grekas
Copy link
Member

Thank you @soyuka.

@nicolas-grekas nicolas-grekas merged commit 4528c11 into symfony:4.4 May 1, 2020
@soyuka soyuka deleted the fix/serializer-preserve-empty-objects branch May 4, 2020 07:11
This was referenced May 31, 2020
Merged
Merged
@Gemorroj
Copy link
Contributor

Gemorroj commented Jun 1, 2020 •
edited
Loading

@nicolas-grekas
but why!? it's bc changes. Now empty doctrine collections casts to object instead of empty array

@Gemorroj Gemorroj mentioned this pull request Jun 1, 2020
Closed
@soyuka
Copy link
Contributor Author

soyuka commented Jun 1, 2020

but why!? it's bc changes. Now empty doctrine collections casts to object instead of empty array

You should not expose ArrayCollection but use ArrayCollection->getValues instead, which will get you a real array (also fixes an index issue if you delete somthing from this ArrayCollection). ArrayCollection is an object and it's correct that it gets normalized to an object imo. Please open an issue if we should discuss this further.

@Plopix
Copy link
Contributor

Plopix commented Jun 2, 2020

@Gemorroj issue fixed ;) #37061

soyuka added a commit to soyuka/core that referenced this pull request Aug 31, 2020
@soyuka
Use symfony/serializer >= 4.4.9 to fix issues:
a9e0a30 
symfony/symfony#34455
symfony/symfony#36601
soyuka added a commit to soyuka/core that referenced this pull request Aug 31, 2020
@soyuka
Use symfony/serializer >= 4.4.9 to fix issues:
73affae 
symfony/symfony#34455
symfony/symfony#36601
soyuka added a commit to soyuka/core that referenced this pull request Aug 31, 2020
@soyuka
Use symfony/serializer >=4.4.9-5.0.9 to fix issues
173ff43 
symfony/symfony#34455
symfony/symfony#36601
soyuka added a commit to api-platform/core that referenced this pull request Sep 1, 2020
@soyuka
Fix tests (#3698)
2c87089 
* Use symfony/serializer >=4.4.9-5.0.9 to fix issues

symfony/symfony#34455
symfony/symfony#36601

* Lowest + legacy test suite missing git

* fix Symfony 5 router generate with reference type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalasr chalasr chalasr approved these changes

@dunglas dunglas dunglas approved these changes

Assignees
No one assigned
Labels
Bug Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
7 participants
@soyuka @nicolas-grekas @Gemorroj @Plopix @dunglas @chalasr @carsonbot