8000 [Serializer] Add support for preserving empty object in object property by lyrixx · Pull Request #42240 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Serializer] Add support for preserving empty object in object property #42240

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 u 8000 p 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
Jul 25, 2021
Merged

[Serializer] Add support for preserving empty object in object property #42240

merged 1 commit into from
Jul 25, 2021

Conversation

lyrixx
Copy link
Member
@lyrixx lyrixx commented Jul 23, 2021
Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations?
Tickets Fix #38192
License MIT
Doc PR

This PR leverage https://symfony.com/blog/new-in-symfony-5-3-inlined-serialization-context to fix #38192.

Example:

class MyDto
{
    public function __construct(
        #[Context([AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true ])]
        public array $mapWithOption = [],
        #[Context([AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true ])]
        public array $mapWithOptionAndData = ['foo' => 'bar'],
        public array $mapWithoutOption = [],
        public array $mapWithoutOptionAndData = ['foo' => 'bar'],
    ) {
    }
}

Will produce:

{"mapWithOption":{},"mapWithOptionAndData":{"foo":"bar"},"mapWithoutOption":[],"mapWithoutOptionAndData":{"foo":"bar"}}

@lyrixx lyrixx requested a review from dunglas as a code owner July 23, 2021 13:27
chalasr
chalasr approved these changes Jul 23, 2021
View reviewed changes
Copy link
Member
@chalasr chalasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Nyholm
Nyholm approved these changes Jul 24, 2021
View reviewed changes
Copy link
Member
@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the PR title and I was mentally preparing for reviewing a 500+ lines PR of complicated Serializer code.

This looks great. Well done.

Maybe we should take a short moment to discuss the name PRESERVE_EMPTY_OBJECTS. Im not sure it is perfect, but I don't have any suggestions Im more comfortable with.
Maybe FORCE_EMPTY_ARRAYS_AS_OBJECT.. Or EMPTY_MAP_AS_OBJECT... hm..

Did you consider anything else bef 8000 ore deciding on PRESERVE_EMPTY_OBJECTS?

@lyrixx
Copy link
Member Author
lyrixx commented Jul 25, 2021 •
edited
Loading

Did you consider anything else before deciding on PRESERVE_EMPTY_OBJECTS?

I did not introduce this constant. It exists for ages, with this exact purpose. 😁

@Nyholm
Copy link
Member
Nyholm commented Jul 25, 2021

Oh. Sorry. You are correct. Im all 👍🏽

@fabpot
Copy link
Member
fabpot commented Jul 25, 2021

Thank you @lyrixx.

@fabpot fabpot mentioned this pull request Jul 25, 2021
Closed
@fabpot fabpot merged commit 01eb18d into symfony:5.4 Jul 25, 2021
@lyrixx lyrixx deleted the serializer-map branch July 25, 2021 15:45
@snoob snoob mentioned this pull request Jul 26, 2021
Closed
@Foxprodev
Copy link
Contributor
Foxprodev commented Jul 26, 2021 •
edited
Loading

Well. It's weird, starting from 5.4 I have to move from

class A {
    #[Context(normalizationContext: [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true])]
    public ArrayObject $map;
    public function __construct()
    {
        $this->map = new ArrayObject();
        $this->map->append(new B());
    }
}
class B {
    public array $list = [];

    public array $anotherListMayBeEmpty;
    public function __construct()
    {
        $this->anotherListMayBeEmpty[] = new C();
    }
}
class C {
    public ArrayObject $index;

    public ArrayObject $settings;

    public ArrayObject $andSoOnObjects;

    public function __construct()
    {
        $this->index = new ArrayObject();
        $this->settings = new ArrayObject();
        $this->andSoOnObjects = new ArrayObject();
    }
}

to

class A {
    #[Context(normalizationContext: [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true])]
    public ArrayObject $map;
    public function __construct()
    {
        $this->map = new ArrayObject();
        $this->map->append(new B());
    }
}
class B {
    #[Context(normalizationContext: [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => false])]
    public array $list = [];

    #[Context(normalizationContext: [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => false])]
    public array $anotherListMayBeEmpty;

    public function __construct()
    {
        $this->anotherListMayBeEmpty[] = new C();
    }
}
class C {
    #[Context(normalizationContext: [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true])]
    public ArrayObject $index;

    #[Context(normalizationContext: [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true])]
    public ArrayObject $settings;

    #[Context(normalizationContext: [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true])]
    public ArrayObject $andSoOnObjects;

    public function __construct()
    {
        $this->index = new ArrayObject();
        $this->settings = new ArrayObject();
        $this->andSoOnObjects = new ArrayObject();
    }
}

Is it an expected complication? @lyrixx @chalasr @Nyholm
It was pretty expected behavior for me to separate list array's from indexed array's using OOP
That's why PRESERVE_EMPTY_OBJECTS was introduced ignoring just empty arrays.

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

Lines 161 to 164 in cd12afe

if (is_iterable($data)) {
if (($context[AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS] ?? false) === true && $data instanceof \Countable && 0 === $data->count()) {
return $data;
}

This option is even named PRESERVE not FORCE like JSON_FORCE_OBJECT

P.S. I am using api-platform and there are so much serializeable nested classes in my projects...

@Foxprodev Foxprodev mentioned this pull request Jul 27, 2021
Closed
@lyrixx
Copy link
Member Author
lyrixx commented Jul 27, 2021

I'll answer in the issue.

This was referenced Nov 5, 2021
Merged
Merged
@alanpoulain alanpoulain mentioned this pull request Jul 13, 2022
Merged
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Aug 8, 2022
@javiereguiluz
minor #16977 [Serializer] fix(serializer): missing empty_array_as_obj…
90ff119 
…ect in example (alanpoulain)

This PR was merged into the 5.4 branch.

Discussion
----------

[Serializer] fix(serializer): missing empty_array_as_object in example

This PR:
- Replaces `empty_iterable_as_object` by `preserve_empty_objects`, like it should be.
- Adds `empty_array_as_object` to example in Serializer Context.
- Removes the mention `by default`, since it's `false` by default.

Related:
- #15554
- #15580
- symfony/symfony#42240
- symfony/symfony#42297

Commits
-------

cc54517 fix(serializer): missing empty_array_as_object in example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@Nyholm Nyholm Nyholm approved these changes

@chalasr chalasr chalasr approved these changes

@faizanakram99 faizanakram99 faizanakram99 approved these changes

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Feature Serializer Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Serializer] Add a annotation (or something) to serialize empty collection as an object and not an array
7 participants
@lyrixx @Nyholm @fabpot @Foxprodev @chalasr @faizanakram99 @carsonbot
0