[Serializer] Allow to add groups to SerializedName annotation/attribute #46432

alamirault · 2022-05-21T18:55:41Z

Q	A
Branch?	7.2
Bug fix?	no
New feature?	yes
Deprecations?	yes
Tickets	Fix #30483
License	MIT
Doc PR	symfony/symfony-docs#...

This PR allow to configure serialization groups for annotation SerializedName. Inspired by PR #37903.

Before we can only define #[SerializedName('nameOne')]. Now, name can be different depending of group

Attributes

#[SerializedName('nameOne')]             <-- Applied when no groups are provided or when group no match
#[SerializedName('nameTwo', ['a', 'b'])] <-- Applied if group is a or b
#[SerializedName('nameThree', ['c'])]    <-- Applied if group is c
public $foo;

Annotations

/**
 * @SerializedName("nameOne")
 * @SerializedName("nameTwo", groups={"a", "b"})
 * @SerializedName("nameThree", groups="c")
 */
public $foo;

YAML

attributes:
  foo:
    serialized_names:
      nameOne: ['a', 'b']
      nameTwo: 'c'

XML

<attribute name="foo">
    <serialized_name name="nameOne">
        <group>a</group>
        <group>b</group>
    </serialized_name>
    <serialized_name name="nameTwo">
        <group>c</group>
    </serialized_name>
</attribute>

src/Symfony/Component/Serializer/Mapping/Loader/XmlFileLoader.php

src/Symfony/Component/Serializer/Mapping/Loader/YamlFileLoader.php

carsonbot · 2022-05-22T14:56:57Z

Hey!

I think @mtarld has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

src/Symfony/Component/Serializer/CHANGELOG.md

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

src/Symfony/Component/Serializer/Mapping/Loader/XmlFileLoader.php

src/Symfony/Component/Serializer/NameConverter/MetadataAwareNameConverter.php

alamirault · 2022-05-24T12:11:21Z

@mtarld I can't "Re request review" but changes are done

mtarld

Almost good!

src/Symfony/Component/Serializer/CHANGELOG.md

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

src/Symfony/Component/Serializer/Mapping/AttributeMetadataInterface.php

src/Symfony/Component/Serializer/Mapping/Loader/YamlFileLoader.php

src/Symfony/Component/Serializer/Mapping/Loader/XmlFileLoader.php

src/Symfony/Component/Serializer/Tests/Mapping/Loader/AnnotationLoaderTest.php

src/Symfony/Component/Serializer/Tests/Mapping/Loader/YamlFileLoaderTest.php

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

src/Symfony/Component/Serializer/Mapping/Loader/XmlFileLoader.php

src/Symfony/Component/Serializer/Mapping/Loader/YamlFileLoader.php

mtarld

LGTM then! 🙂

alamirault · 2022-07-26T18:48:18Z

(Rebased on current 6.2)

nicolas-grekas

Thanks for the PR. Here are some comments to help move forward.

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

src/Symfony/Component/Serializer/Mapping/Loader/YamlFileLoader.php

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

src/Symfony/Component/Serializer/Mapping/AttributeMetadataInterface.php

alamirault · 2022-07-29T20:49:18Z

@nicolas-grekas I added layer and trigger deprecation (like urlMatcher)

However, deprecation seems trigger even if methods are not called

  1x: The "Symfony\Component\Serializer\Mapping\AttributeMetadata::setSerializedName()" method will require a new "string[] $groups" argument in the next major version of its interface "Symfony\Component\Serializer\Mapping\AttributeMetadataInterface", not defining it is deprecated.
    1x in AttributeMetadataTest::testInterface from Symfony\Component\Serializer\Tests\Mapping

    public function testInterface()
    {
        $attributeMetadata = new AttributeMetadata('name');
        $this->assertInstanceOf(AttributeMetadataInterface::class, $attributeMetadata);
    }

chalasr · 2022-07-30T14:14:24Z

Can you try documenting the param explicitly the same way as the interface? It may be enough to avoid the notice.

alamirault · 2022-07-30T16:18:29Z

Indeed, It's enough, thank you @chalasr

src/Symfony/Component/Serializer/CHANGELOG.md

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

alamirault · 2022-08-29T11:47:16Z

Friendly ping for re-review @nicolas-grekas

nicolas-grekas

Here are some more BC notes. Please rebase also.

UPGRADE-6.2.md

...ymfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/config/serialization.yaml

src/Symfony/Component/Serializer/Annotation/SerializedName.php

nicolas-grekas · 2022-08-31T09:06:54Z

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

     */
-    public $serializedName;
+    public $serializedName = [];


For maximum BC, I think we should keep this as a string when possible, and use arrays only when actually required.

@nicolas-grekas , Tu be sure, somthing like this ?

/** * string|array<string, string|null> */ public $serializedName;

And deal with string and array in every methods, is it right ?

nicolas-grekas · 2022-08-31T09:08:53Z

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php

+     *
+     * @param string[] $groups
+     */
+    public function getSerializedName(/* array $groups = [] */): ?string


Why pass an array here? Shouldn't we accept string|null instead?

Used by MetadataAwareNameConverter
https://github.com/symfony/symfony/pull/46432/files#diff-87ffe277c0213fca753a8e0b26cc8f4436e255b9776e5e08852c403df247c5b9R121

Serializer group context can be multiple. So returned SerializerName is the first matching group list.

Should be done in MetadataAwareNameConverter ?

zairigimad · 2022-10-26T14:31:23Z

Hello @alamirault, this is a great feature I was waiting for it for so long, do you you need help on this PR to resolve the latest feedbacks ? thanks

qdequippe · 2023-03-29T14:23:48Z

Is this PR will be included in 6.2.X? or in 6.3?

Renrhaf · 2023-08-04T13:00:06Z

thank you for the contribution !
would love to see this shipped in the next version :)

sigbits · 2023-10-23T15:28:10Z

Will this one still be released?

andriusvo · 2024-01-08T00:53:08Z

Any plans to make it released?

nesl247 · 2024-07-16T20:19:50Z

We just ran into this need. We need to deserialize from xml we don't control, and serialize to json. Doing this any other way would be extremely complex as we're already dealing with a few hundred objects. Duplicating all of that to control it differently is not an option.

nesl247 · 2024-08-06T14:46:37Z

Would it be hard to add to SerializedPath as well?

mikevrind · 2024-09-11T11:30:02Z

I really appreciate the works that people have put into this feature. Would be valuable to see this being merged in the near future because this gives us more control (and less unwanted/unexpected behaviour) within de Serializer.

derrabus · 2024-09-11T11:35:37Z

Somebody would need to pick up the work and rebase the changes onto 7.2.

daniser · 2024-09-20T09:10:21Z

Sorry guys, I completely missed this PR and discussion and created my own PR (#58236) 🤦‍♂️.
On the good side, I've also implemented logic for SerializedPath grouping.

alamirault requested a review from dunglas as a code owner May 21, 2022 18:55

carsonbot added Status: Needs Review Deprecation Feature Serializer labels May 21, 2022

carsonbot added this to the 6.2 milestone May 21, 2022

alamirault commented May 21, 2022

View reviewed changes

src/Symfony/Component/Serializer/Mapping/Loader/XmlFileLoader.php Show resolved Hide resolved

alamirault commented May 21, 2022

View reviewed changes

src/Symfony/Component/Serializer/Mapping/Loader/YamlFileLoader.php Outdated Show resolved Hide resolved

mtarld suggested changes May 23, 2022

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels May 23, 2022

mtarld suggested changes May 24, 2022

View reviewed changes

mtarld suggested changes May 26, 2022

View reviewed changes

src/Symfony/Component/Serializer/Mapping/AttributeMetadata.php Outdated Show resolved Hide resolved

src/Symfony/Component/Serializer/Mapping/Loader/XmlFileLoader.php Show resolved Hide resolved

src/Symfony/Component/Serializer/Mapping/Loader/YamlFileLoader.php Show resolved Hide resolved

mtarld approved these changes May 26, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels May 26, 2022

alamirault force-pushed the feature-30483-serialized-names-group branch from d01bf55 to 83941cd Compare July 26, 2022 18:43

nicolas-grekas reviewed Jul 29, 2022

View reviewed changes

alamirault force-pushed the feature-30483-serialized-names-group branch 2 times, most recently from afb3e7a to 035b0dd Compare July 29, 2022 20:31

nicolas-grekas reviewed Aug 1, 2022

View reviewed changes

alamirault requested a review from nicolas-grekas August 2, 2022 20:32

carsonbot added Status: Needs Review and removed Status: Reviewed labels Aug 2, 2022

nicolas-grekas reviewed Aug 31, 2022

View reviewed changes

nicolas-grekas mentioned this pull request Aug 31, 2022

[Serializer] SerializedName based on groups #30483

Open

alamirault force-pushed the feature-30483-serialized-names-group branch 2 times, most recently from 922aabe to 03d905e Compare October 26, 2022 20:10

[Serializer] Allow to add groups to SerializedName annotation/attribute

03d905e

nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022

nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023

nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023

xabbuh modified the milestones: 7.1, 7.2 May 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serializer] Allow to add groups to SerializedName annotation/attribute #46432

[Serializer] Allow to add groups to SerializedName annotation/attribute #46432

alamirault commented May 21, 2022 •

edited by OskarStark

Loading

carsonbot commented May 22, 2022

alamirault commented May 24, 2022

mtarld left a comment

mtarld left a comment

alamirault commented Jul 26, 2022

nicolas-grekas left a comment

alamirault commented Jul 29, 2022 •

edited

Loading

chalasr commented Jul 30, 2022 •

edited

Loading

alamirault commented Jul 30, 2022

alamirault commented Aug 29, 2022

nicolas-grekas left a comment

nicolas-grekas Aug 31, 2022

alamirault Oct 26, 2022 •

edited

Loading

nicolas-grekas Aug 31, 2022

alamirault Oct 26, 2022

zairigimad commented Oct 26, 2022

qdequippe commented Mar 29, 2023

Renrhaf commented Aug 4, 2023

sigbits commented Oct 23, 2023

andriusvo commented Jan 8, 2024

nesl247 commented Jul 16, 2024

nesl247 commented Aug 6, 2024

mikevrind commented Sep 11, 2024

derrabus commented Sep 11, 2024

daniser commented Sep 20, 2024

[Serializer] Allow to add groups to SerializedName annotation/attribute #46432

Are you sure you want to change the base?

[Serializer] Allow to add groups to SerializedName annotation/attribute #46432

Conversation

alamirault commented May 21, 2022 • edited by OskarStark Loading

Attributes

Annotations

YAML

XML

carsonbot commented May 22, 2022

alamirault commented May 24, 2022

mtarld left a comment

Choose a reason for hiding this comment

mtarld left a comment

Choose a reason for hiding this comment

alamirault commented Jul 26, 2022

nicolas-grekas left a comment

Choose a reason for hiding this comment

alamirault commented Jul 29, 2022 • edited Loading

chalasr commented Jul 30, 2022 • edited Loading

alamirault commented Jul 30, 2022

alamirault commented Aug 29, 2022

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas Aug 31, 2022

Choose a reason for hiding this comment

alamirault Oct 26, 2022 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Aug 31, 2022

Choose a reason for hiding this comment

alamirault Oct 26, 2022

Choose a reason for hiding this comment

zairigimad commented Oct 26, 2022

qdequippe commented Mar 29, 2023

Renrhaf commented Aug 4, 2023

sigbits commented Oct 23, 2023

andriusvo commented Jan 8, 2024

nesl247 commented Jul 16, 2024

nesl247 commented Aug 6, 2024

mikevrind commented Sep 11, 2024

derrabus commented Sep 11, 2024

daniser commented Sep 20, 2024

alamirault commented May 21, 2022 •

edited by OskarStark

Loading

alamirault commented Jul 29, 2022 •

edited

Loading

chalasr commented Jul 30, 2022 •

edited

Loading

alamirault Oct 26, 2022 •

edited

Loading