Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Serializer] Allow to access extra infos in name converters#27021
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| { | ||
| publicfunctiontestImplem() | ||
| { | ||
| $p =$this->prophesize(AdvancedNameConverterInterface::class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Still WIP I guess 😄
Also we don't use prophecy in symfony core test suite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oops, I don't want to commit this file (yes I know for prophecy :(, it would be the perfect fit here).
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| publicfunctionnormalize($propertyName, ?string$class =null, ?string$format =null,array$context =array()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
the? should be removed, they are duplicate info (same below)
| * | ||
| * @author Kévin Dunglas <dunglas@gmail.com> | ||
| */ | ||
| interface AdvancedNameConverterInterfaceextends NameConverterInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
where is this used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nowhere, it's to be used in userland projects, we don't have this need in Symfony directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I added a test to highlight the expected usage.
b9f10a1 to588b68bCompare588b68b to303dc63Comparedunglas commentedJul 13, 2018
conflict fixed |
fabpot left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think it deserves a related PR for the docs.
| ----- | ||
| *`AbstractNormalizer::handleCircularReference` is now final, and receives two optional extra arguments: the format and the context | ||
| * added`AdvancedNameConverterInterface` to access to the class, the format and the context in a name converter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
...to access the class...
| namespaceSymfony\Component\Serializer\NameConverter; | ||
| /** | ||
| * Gives access to the format and the context in the property name converters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
class is missing here
303dc63 tofe7ff09Comparedunglas commentedSep 4, 2018
@fabpot comments fixed, I'll do the doc PR. |
fe7ff09 to57fe017Comparefabpot commentedSep 4, 2018
Thank you@dunglas. |
…rters (dunglas)This PR was squashed before being merged into the 4.2-dev branch (closes#27021).Discussion----------[Serializer] Allow to access extra infos in name converters| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes<!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | n/a| License | MIT| Doc PR | todoSimilar to#27017 and#27020 but for name converters.ping@meyerbaptisteCommits-------57fe017 [Serializer] Allow to access extra infos in name converters
javiereguiluz commentedSep 7, 2018
I've createdsymfony/symfony-docs#10273 to document this new feature. Please, don't forget to create a doc issue for every new feature. Kévin, if you can't contribute the docs for this, please share some examples of this feature in action or some extended explanation. Thanks! |
…ers (dunglas, javiereguiluz)This PR was merged into the master branch.Discussion----------[Serializer] Allow to access extra infos in name converterssymfony/symfony#27021Commits-------71afdca Minor rewordae63f8a Add a note blockca5e2e0 [Serializer] Allow to access extra infos in name converters
…properties through metadata (fbourigault)This PR was squashed before being merged into the 4.2-dev branch (closes#28505).Discussion----------[Serialized] allow configuring the serialized name of properties through metadata| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#15171| License | MIT| Doc PR |symfony/symfony-docs#10422This leverage the new `AdvancedNameConverterInterface` interface (#27021) to implement a name converter that relies on metadata. The name to use is configured per property using a `@SerializedName` annotation or the `serialized-name` XML attribute or the `serialized_name` key for YAML.This was exposed by@dunglas in#19374 (comment).# Framework integrationFor FramworkBundle integration, a ChainNameConverter could be added to allow users to use this name converter with a custom one.# To do- [x] add a CHANGELOG.md entry.- [x] add a fallback.- [x] add framework integration.- [x] add local caching to `MetadataAwareNameConverter`.- [x] add a doc PR.Commits-------d1d1ceb [Serialized] allow configuring the serialized name of properties through metadata
Uh oh!
There was an error while loading.Please reload this page.
Similar to#27017 and#27020 but for name converters.
ping@meyerbaptiste