Skip to content

[FrameworkBundle][Serializer] Allow serializer default context configuration #38542

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
Oct 27, 2021
Merged

[FrameworkBundle][Serializer] Allow serializer default context configuration #38542

merged 1 commit into from
Oct 27, 2021

Conversation

soyuka
Copy link
Contributor

@soyuka soyuka commented Oct 13, 2020

Q A
Branch? 5.x
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR todo if we go with it

This allow to configure the context for normalizer's on a high level:

serializer:
    default_context:
        enable_max_depth: true

This feature was asked in api-platform/core#1764 (comment) and I thought it could be in symfony instead. The messenger component has the same configuration.
This re-opens #28927 (for discussion). I'm still unsure about the discussion in #28927 (comment) and what solution I should implement. Please let me know if you come to an agreement.

@soyuka soyuka requested a review from dunglas as a code owner October 13, 2020 09:35
@nicolas-grekas nicolas-grekas added this to the 5.x milestone Oct 13, 2020
@maxhelias maxhelias mentioned this pull request Jan 16, 2021
Closed
@soyuka soyuka mentioned this pull request Feb 19, 2021
Closed
@stof
Copy link
Member

stof commented Sep 20, 2021

@soyuka what is the state of this PR ?

@soyuka
Copy link
Contributor Author

soyuka commented Oct 22, 2021 •
edited
Loading

@stof I'm wondering as well, I'm really not sure that we've come to an agreement on what needs to be done. If the community knows exactly how we can go on, it'll be my pleasure to continue working on it!

Do we actually want this feature?

@norkunas
Copy link
Contributor

I'd like to use this feature. Currently to always expose extended datime format I override datetime normalizer context in compiler pass.

@dunglas
Copy link
Member

dunglas commented Oct 23, 2021

I'm in favor of merging this feature

@lyrixx
Copy link
Member

lyrixx commented Oct 23, 2021

Me too. @soyuka do you have time to finish it, or do you need help?

@soyuka
Copy link
Contributor Author

soyuka commented Oct 25, 2021

No problem I'll finish this. About the code I should use setBinding then? Won't that be a possible conflict with userland? If someone declares a $defaultContext it'll overwrite ours right?

@lyrixx
Copy link
Member

lyrixx commented Oct 25, 2021

No problem I'll finish this. About the code I should use setBinding then? Won't that be a possible conflict with userland? If someone declares a $defaultContext it'll overwrite ours right?

yes 👍🏼

@soyuka soyuka force-pushed the configure-serializer-context branch 5 times, most recently from 53e430b to 919719f Compare October 26, 2021 12:05
@soyuka soyuka force-pushed the configure-serializer-context branch from 919719f to 43d1ca5 Compare October 26, 2021 12:27
lyrixx
lyrixx approved these changes Oct 26, 2021
View reviewed changes
Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@carsonbot carsonbot changed the title Allow serializer default context configuration [FrameworkBundle][Serializer] Allow serializer default context configuration Oct 26, 2021
@fabpot
Copy link
Member

fabpot commented Oct 27, 2021

Thank you @soyuka.

@fabpot fabpot mentioned this pull request Oct 27, 2021
Closed
@fabpot fabpot merged commit cddd1ff into symfony:5.4 Oct 27, 2021
@soyuka soyuka deleted the configure-serializer-context branch October 27, 2021 11:56
This was referenced Nov 5, 2021
Merged
Merged
@ro0NL ro0NL mentioned this pull request Jan 9, 2022
Closed
@bigfoot90 bigfoot90 mentioned this pull request Jan 12, 2022
Open
@chalasr chalasr mentioned this pull request Jan 26, 2022
Open
16 tasks
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

@dunglas dunglas dunglas approved these changes

@lyrixx lyrixx lyrixx approved these changes

@chalasr chalasr chalasr approved these changes

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

Successfully merging this pull request may close these issues.
10 participants
@soyuka @stof @norkunas @dunglas @lyrixx @fabpot @chalasr @nicolas-grekas @derrabus @carsonbot