Skip to content

Add preserve_empty_objects support#4363

Closed
Foxprodev wants to merge 1 commit into
api-platform:mainfrom
Foxprodev:preserve_empty_objects_support
Closed

Add preserve_empty_objects support#4363
Foxprodev wants to merge 1 commit into
api-platform:mainfrom
Foxprodev:preserve_empty_objects_support

Conversation

@Foxprodev

@Foxprodev Foxprodev commented Jul 21, 2021

Copy link
Copy Markdown
Q A
Branch? 2.6
Tickets #38192 ?
License MIT
Doc PR

CollectionNormalizer currently replacing ArrayObject with an empty array

More about main idea discussed here symfony/symfony#28363

Feel free to help me with test case and test location. Thank you 😄

@Foxprodev

Copy link
Copy Markdown
Author

Or maybe CollectionNormalizers should stop supporting empty Countables

@Foxprodev
Foxprodev changed the base branch from main to 2.6 July 22, 2021 07:08
@Foxprodev
Foxprodev force-pushed the preserve_empty_objects_support branch from de05be2 to b0d548a Compare July 22, 2021 07:10
@snoob

snoob commented Jul 26, 2021

Copy link
Copy Markdown
Contributor

+1 for this :)

WDTY about calling the normalizer on normalizeRawCollection and fully delegate the "PRESERVE_EMPTY_OBJECTS" management to Symfony ?
Once fixed, we will be able to use this symfony/symfony#42240 on symfony 5.4

return $this->normalizer->normalize($this->normalizeRawCollection($object, $format, $context));

I have found a temporary workaround. Add "PRESERVE_EMPTY_OBJECTS" in your entity normalizationContext and declare your property as stdClass instead of array. It will be serialized as empty object instead of empty array.

@Foxprodev

Copy link
Copy Markdown
Author

Once fixed, we will be able to use this symfony/symfony#42240 on symfony 5.4

Wow this PR seems terribly bad for me. It breaks list array at all. I am currently using PRESERVE_EMPTY_OBJECTS in global context and this behaviour feels expected for me

@snoob

snoob commented Jul 27, 2021

Copy link
Copy Markdown
Contributor

Wow this PR seems terribly bad for me. It breaks list array at all. I am currently using PRESERVE_EMPTY_OBJECTS in global context and this behaviour feels expected for me

About this PR, you shouldn't worry about it :) think it won't break your current behavior. You will just be able to override this for a specific property.

@Foxprodev

Foxprodev commented Jul 27, 2021

Copy link
Copy Markdown
Author

About this PR, you shouldn't worry about it :) think it won't break your current behavior. You will just be able to override this for a specific property.

According to PR ObjectNormalizer will convert all empty arrays to objects, when PRESERVE_EMPTY_OBJECTS enabled. It sounds weird for me and breaks BC.
But yes it's not about my current PR 😄

@Foxprodev

Foxprodev commented Jul 28, 2021

Copy link
Copy Markdown
Author

symfony/symfony#42240 irrelevant. Maybe I should rework this PR to support symfony/symfony#42297 in advance
All should work fine

@Foxprodev
Foxprodev force-pushed the preserve_empty_objects_support branch from b0d548a to 60b38c4 Compare August 3, 2021 07:58
@Foxprodev

Copy link
Copy Markdown
Author

Added tests

@Foxprodev
Foxprodev force-pushed the preserve_empty_objects_support branch 2 times, most recently from 58ade62 to fdbd75a Compare August 4, 2021 06:58
@soyuka

soyuka commented Aug 10, 2021

Copy link
Copy Markdown
Member

Could you target main?

@Foxprodev
Foxprodev changed the base branch from 2.6 to main August 10, 2021 18:40
@Foxprodev
Foxprodev marked this pull request as draft August 10, 2021 18:41
@Foxprodev
Foxprodev force-pushed the preserve_empty_objects_support branch 2 times, most recently from 44116fd to 2235366 Compare August 10, 2021 18:49
@Foxprodev
Foxprodev marked this pull request as ready for review August 10, 2021 18:49
@Foxprodev

Copy link
Copy Markdown
Author

@soyuka done

@snoob

snoob commented Aug 10, 2021

Copy link
Copy Markdown
Contributor

You must rebase your branch there are conflicts

@Foxprodev
Foxprodev force-pushed the preserve_empty_objects_support branch from 2235366 to a40edca Compare August 10, 2021 18:57
@snoob

snoob commented Aug 10, 2021

Copy link
Copy Markdown
Contributor

Thank :)

b2p-fred added a commit to b2p-fred/core that referenced this pull request Jan 28, 2022
@alanpoulain

Copy link
Copy Markdown
Member

Superseded by #4999, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants