Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Serializer] AddSerializedPath annotation to flatten nested attributes#43534

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

Merged

Conversation

@boenner
Copy link
Contributor

@boennerboenner commentedOct 15, 2021
edited
Loading

QA
Branch?6.2
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#32080
LicenseMIT
Doc PR#17396

As suggested by@derrabus in#32080, I'm creating a PR for this.

In order to normalize/denormalize nested attributes, the@SerializedPath annotation can be used:

class NestedDummy{/**     * @SerializedPath("[one][two][three]")     */public$foo;/**     * @SerializedPath("[one][four]")     */public$bar;}

with

$data = ['one' => ['two' => ['three' =>'foo',        ],'four' =>'bar',    ],];$normalizer =newAbstractObjectNormalizerWithMetadata();$normalizer->denormalize($data,    NestedDummy::class,'any');

The annotations needs to be used with a validPropertyPath string for this to work.

Open todos:

  • tests for new feature
  • update documentation

derrabus, Guuzen, elliotbruneel, yceruto, kadarsys, friedrj, piotrooo, lyrixx, valtzu, and BafS reacted with thumbs up emojikadarsys, nikophil, and imdhemy reacted with rocket emoji
@carsonbotcarsonbot added this to the5.4 milestoneOct 15, 2021
@carsonbotcarsonbot changed the title[WIP] [Serializer] Flatten nested attributes[Serializer] [WIP] Flatten nested attributesOct 15, 2021
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (seehttps://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
  • Features and deprecations must be submitted against the 5.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@carsonbot
Copy link

Hey!

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

Cheers!

Carsonbot

@OskarStarkOskarStark changed the title[Serializer] [WIP] Flatten nested attributes[Serializer] [WIP] Flatten nested attributesOct 17, 2021
@boennerboennerforce-pushed theserializer-flatten-nested-attributes branch from2758fde tofe5d453CompareOctober 28, 2021 12:55
@fabpotfabpot modified the milestones:5.4,6.1Oct 29, 2021
@boennerboennerforce-pushed theserializer-flatten-nested-attributes branch 2 times, most recently from02e59c6 toa824bd0CompareNovember 2, 2021 15:18
@boennerboennerforce-pushed theserializer-flatten-nested-attributes branch fromfc0d799 to9e6dd9aCompareNovember 17, 2021 10:19
@boenner
Copy link
ContributorAuthor

I tried rebasing this from 5.4 to 6.0 by following thecontribution guidelines but it seems like I screwed that up as things got a little messy ... is there any way I can salvage this PR or do I need to create a new one based on the 6.0 branch?

@boennerboennerforce-pushed theserializer-flatten-nested-attributes branch 2 times, most recently froma15a534 tofdcc545CompareNovember 17, 2021 16:17
@chalasrchalasr changed the base branch from5.4 to6.0November 17, 2021 20:42
@chalasrchalasrforce-pushed theserializer-flatten-nested-attributes branch froma15a534 to992a563CompareNovember 17, 2021 20:43
@chalasr
Copy link
Member

@boenner You only forgot to change the base branch of the pull request. I did it for you and squashed your commits to trigger the CI.

boenner reacted with heart emoji

@boennerboennerforce-pushed theserializer-flatten-nested-attributes branch fromc210064 to5d84bf2CompareSeptember 21, 2022 15:08
@nicolas-grekasnicolas-grekasforce-pushed theserializer-flatten-nested-attributes branch 2 times, most recently frombd4277c toef74981CompareOctober 18, 2022 16:49
@nicolas-grekas
Copy link
Member

@boenner I just force-pushed on your fork to fix all comments from@mtarld but one. Can you please have a look?

boenner and nikophil reacted with heart emoji

@nicolas-grekasnicolas-grekas added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelOct 19, 2022
@boenner
Copy link
ContributorAuthor

The review is finished and theSerializedPath annotation now uses thePropertyAccess component. Thanks for your help with the code changes,@mtarld!

mtarld reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

Can you please update the PR's description if it's not in sync?

boenner reacted with thumbs up emoji

@nicolas-grekasnicolas-grekasforce-pushed theserializer-flatten-nested-attributes branch fromceff3bd to08a1119CompareOctober 22, 2022 13:22
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment
edited
Loading

Choose a reason for hiding this comment

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

I force-pushed some CS changes. Failures are unrelated or false-positives (fabbot).

boenner reacted with thumbs up emoji
@fabpot
Copy link
Member

Thank you@boenner.

boenner and DemigodCode reacted with hooray emoji

@fabpotfabpot merged commit5131ec3 intosymfony:6.2Oct 23, 2022
@fabpotfabpot mentioned this pull requestOct 24, 2022
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestNov 3, 2022
This PR was merged into the 6.2 branch.Discussion----------[Serializer] SerializedPath documentationAdds documentation for the `SerializedPath` annotation (symfony/symfony#43534). Thisfixes#17389.Commits-------375c089 Documentation for the SerializedPath definition
eliashaeussler added a commit to eliashaeussler/composer-unused that referenced this pull requestDec 1, 2022
Sincesymfony/symfony#43534 (released with `symfony/serializer` 6.2.0),Symfony's ObjectNormalizer has a hard dependency to the`symfony/property-access` package. As we're using the ObjectNormalizer,we must ensure to meet this requirement by explicitly adding thepackage as requirement in `composer.json`.
icanhazstring pushed a commit to composer-unused/composer-unused that referenced this pull requestDec 1, 2022
Sincesymfony/symfony#43534 (released with `symfony/serializer` 6.2.0),Symfony's ObjectNormalizer has a hard dependency to the`symfony/property-access` package. As we're using the ObjectNormalizer,we must ensure to meet this requirement by explicitly adding thepackage as requirement in `composer.json`.
nicolas-grekas added a commit that referenced this pull requestMar 31, 2023
…enner)This PR was merged into the 6.2 branch.Discussion----------[Serializer] Fix serialized path for non-scalar values| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#49494| License       | MIT| Doc PR        | noThis relates to#49494 and#49225. When non-scalar values are normalized, they are normalized twice in the `normalize()` function:```phpif (null !== $attributeValue && !\is_scalar($attributeValue)) {$stack[$attribute] = $attributeValue;}$data = $this->updateData($data, $attribute, $attributeValue, $class, $format, $attributeContext, $attributesMetadata, $classMetadata);```and a bit later:```phpforeach ($stack as $attribute => $attributeValue) {...$data = $this->updateData($data, $attribute, $this->serializer->normalize($attributeValue, $format, $childContext), $class, $format, $attributeContext, $attributesMetadata, $classMetadata);}```For non-scalar values with a `SerializedPath` annotation this leads to an exception because the serializer is trying to re-populate the path. Running `updateData()` only once fixes this, but breaks a couple of tests across the components as it changes the order of elements in the serialized string (non-scalar values will be pushed to the end). Other than the string comparisons, nothing seems to break. This was also an issue while reviewing the PR for the `SerializedPath` annotation (#43534 (comment)) and got reverted because of the potential BC break.I'm not sure what benefit normalizing twice brings, so I added the test from `@HonzaMatosik`, changed the behavior in the normalizer and fixed the broken tests. If that's not the preferred solution here, I'd be ok with just eleminating the "The element you are trying to set is already populated" exception in the `SerializedPath` and allow overwriting values.Commits-------d82ec41 [Serializer] Fix serializedpath for non scalar types
symfony-splitter pushed a commit to symfony/serializer that referenced this pull requestMar 31, 2023
…enner)This PR was merged into the 6.2 branch.Discussion----------[Serializer] Fix serialized path for non-scalar values| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       | Fix #49494| License       | MIT| Doc PR        | noThis relates tosymfony/symfony#49494 andsymfony/symfony#49225. When non-scalar values are normalized, they are normalized twice in the `normalize()` function:```phpif (null !== $attributeValue && !\is_scalar($attributeValue)) {$stack[$attribute] = $attributeValue;}$data = $this->updateData($data, $attribute, $attributeValue, $class, $format, $attributeContext, $attributesMetadata, $classMetadata);```and a bit later:```phpforeach ($stack as $attribute => $attributeValue) {...$data = $this->updateData($data, $attribute, $this->serializer->normalize($attributeValue, $format, $childContext), $class, $format, $attributeContext, $attributesMetadata, $classMetadata);}```For non-scalar values with a `SerializedPath` annotation this leads to an exception because the serializer is trying to re-populate the path. Running `updateData()` only once fixes this, but breaks a couple of tests across the components as it changes the order of elements in the serialized string (non-scalar values will be pushed to the end). Other than the string comparisons, nothing seems to break. This was also an issue while reviewing the PR for the `SerializedPath` annotation (symfony/symfony#43534 (comment)) and got reverted because of the potential BC break.I'm not sure what benefit normalizing twice brings, so I added the test from `@HonzaMatosik`, changed the behavior in the normalizer and fixed the broken tests. If that's not the preferred solution here, I'd be ok with just eleminating the "The element you are trying to set is already populated" exception in the `SerializedPath` and allow overwriting values.Commits-------d82ec41d18 [Serializer] Fix serializedpath for non scalar types
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@OskarStarkOskarStarkOskarStark left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@mtarldmtarldmtarld approved these changes

@chalasrchalasrchalasr approved these changes

@jderussejderusseAwaiting requested review from jderusse

@lyrixxlyrixxAwaiting requested review from lyrixx

@wouterjwouterjAwaiting requested review from wouterj

@xabbuhxabbuhAwaiting requested review from xabbuh

@ycerutoycerutoAwaiting requested review from yceruto

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

Assignees

No one assigned

Labels

FeatureSerializer❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: Reviewed

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

[Serializer] @SerializedName flattening nested attributes

8 participants

@boenner@carsonbot@chalasr@dunglas@nicolas-grekas@fabpot@OskarStark@mtarld

[8]ページ先頭

©2009-2025 Movatter.jp