Skip to content

[Serializer] Deprecate support for abstract uid denormalization in UidNormalizer #44721

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

Merged
merged 1 commit into from
Feb 23, 2022

Conversation

fancyweb
Copy link
Contributor

Q A
Branch? 6.1
Bug fix? no
New feature? no
Deprecations? yes
Tickets -
License MIT
Doc PR -

Continuation of #44600. All abstract classes should not be denormalized to Uuid::class. We need a concrete class. Instead of penalizing everyone with reflection in supportsDenormalization(), let's just bubble up the PHP \Error (we are going to do the same in UidValueResolver).

@nicolas-grekas
Copy link
Member

(rebase needed)

@fancyweb fancyweb force-pushed the serializer/deprec-abstract-uid branch from 09000e1 to 8661042 Compare December 27, 2021 09:13
@fancyweb fancyweb force-pushed the serializer/deprec-abstract-uid branch 2 times, most recently from 38431ca to 4075944 Compare December 31, 2021 08:04
@fancyweb fancyweb force-pushed the serializer/deprec-abstract-uid branch from 4075944 to b0c07f8 Compare January 13, 2022 16:59
Comment on lines +11 to +28
* Deprecate supporting denormalization for `AbstractUid` in `UidNormalizer`, use one of `AbstractUid` child class instead
* Deprecate denormalizing to an abstract class in `UidNormalizer`
Copy link
Contributor

Choose a reason for hiding this comment

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

to me this signals the same deprecation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is one deprecation for supportsDenormalization() that will return false instead of true for AbstractUid::class specifically.
And another for denormalize() that will throw an \Error if the class is abstract (instead of fallbacking to Uuid::class).

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

rebase needed

@fancyweb fancyweb force-pushed the serializer/deprec-abstract-uid branch 2 times, most recently from 793a7b2 to 6ed7a68 Compare February 14, 2022 09:09
@fancyweb fancyweb force-pushed the serializer/deprec-abstract-uid branch from 6ed7a68 to 6fc4287 Compare February 23, 2022 13:12
@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

@nicolas-grekas nicolas-grekas merged commit a14eb3f into symfony:6.1 Feb 23, 2022
@fancyweb fancyweb deleted the serializer/deprec-abstract-uid branch February 23, 2022 13:21
@fabpot fabpot mentioned this pull request Apr 15, 2022
nicolas-grekas added a commit that referenced this pull request Jun 5, 2023
…(fancyweb)

This PR was merged into the 7.0 branch.

Discussion
----------

[Serializer] Remove abstract uid denormalization code

| Q             | A
| ------------- | ---
| Branch?       | 7.0
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Removes the code that was deprecated in #44721

Commits
-------

79028f9 [Serializer] Remove abstract uid denormalization code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants