Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serializer] Allow to access extra infos in name converters #27021

Merged
merged 1 commit into from Sep 4, 2018

Conversation

@dunglas
Copy link
Member

dunglas commented Apr 23, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR symfony/symfony-docs#10317

Similar to #27017 and #27020 but for name converters.

ping @meyerbaptiste

{
public function testImplem()
{
$p = $this->prophesize(AdvancedNameConverterInterface::class);

This comment has been minimized.

Copy link
@ogizanagi

ogizanagi Apr 23, 2018

Member

Still WIP I guess 😄
Also we don't use prophecy in symfony core test suite.

This comment has been minimized.

Copy link
@dunglas

dunglas Apr 23, 2018

Author Member

Oops, I don't want to commit this file (yes I know for prophecy :(, it would be the perfect fit here).

/**
* {@inheritdoc}
*/
public function normalize($propertyName, ?string $class = null, ?string $format = null, array $context = array());

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jun 19, 2018

Member

the ? should be removed, they are duplicate info (same below)

*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
interface AdvancedNameConverterInterface extends NameConverterInterface

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jun 19, 2018

Member

where is this used?

This comment has been minimized.

Copy link
@dunglas

dunglas Jun 30, 2018

Author Member

Nowhere, it's to be used in userland projects, we don't have this need in Symfony directly.

This comment has been minimized.

Copy link
@dunglas

dunglas Jun 30, 2018

Author Member

I added a test to highlight the expected usage.

@dunglas dunglas force-pushed the dunglas:serializer-name-converter-extra branch 5 times, most recently from b9f10a1 to 588b68b Jun 30, 2018
@dunglas dunglas force-pushed the dunglas:serializer-name-converter-extra branch from 588b68b to 303dc63 Jul 13, 2018
@dunglas

This comment has been minimized.

Copy link
Member Author

dunglas commented Jul 13, 2018

conflict fixed

Copy link
Member

fabpot left a comment

I think it deserves a related PR for the docs.

@@ -5,6 +5,7 @@ CHANGELOG
-----

* `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

This comment has been minimized.

Copy link
@fabpot

fabpot Jul 13, 2018

Member

...to access the class...

namespace Symfony\Component\Serializer\NameConverter;

/**
* Gives access to the format and the context in the property name converters.

This comment has been minimized.

Copy link
@fabpot

fabpot Jul 13, 2018

Member

class is missing here

@dunglas dunglas force-pushed the dunglas:serializer-name-converter-extra branch from 303dc63 to fe7ff09 Sep 4, 2018
@dunglas

This comment has been minimized.

Copy link
Member Author

dunglas commented Sep 4, 2018

@fabpot comments fixed, I'll do the doc PR.

@fabpot
fabpot approved these changes Sep 4, 2018
@fabpot fabpot force-pushed the dunglas:serializer-name-converter-extra branch from fe7ff09 to 57fe017 Sep 4, 2018
@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Sep 4, 2018

Thank you @dunglas.

@fabpot fabpot merged commit 57fe017 into symfony:master Sep 4, 2018
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details
fabpot added a commit that referenced this pull request Sep 4, 2018
…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     <!-- see https://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        | todo

Similar to #27017 and #27020 but for name converters.

ping @meyerbaptiste

Commits
-------

57fe017 [Serializer] Allow to access extra infos in name converters
@javiereguiluz

This comment has been minimized.

Copy link
Member

javiereguiluz commented Sep 7, 2018

I've created symfony/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!

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Sep 11, 2018
…ers (dunglas, javiereguiluz)

This PR was merged into the master branch.

Discussion
----------

[Serializer] Allow to access extra infos in name converters

symfony/symfony#27021

Commits
-------

71afdca Minor reword
ae63f8a Add a note block
ca5e2e0 [Serializer] Allow to access extra infos in name converters
dunglas added a commit that referenced this pull request Oct 5, 2018
…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#10422

This 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 integration
For 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
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018
This was referenced Nov 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.