Skip to content

[Messenger][Serializer] Deprecate "context aware" interfaces #43982

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
Jan 9, 2022

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Nov 9, 2021

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

In supports* methods, we are really often relying on serialization context.
Previously (~2017), in order to have the context in these methods (and be BC compatible), new interfaces were introduced:

  • ContextAwareNormalizerInterface
  • ContextAwareDenormalizerInterface
  • ContextAwareEncoderInterface
  • ContextAwareDecoderInterface

But right now, thanks to the DebugClassLoader, we're able to have an upgrade path where the regular interfaces:

  • NormalizerInterface
  • DenormalizerInterface
  • EncoderInterface
  • DecoderInterface
    can be updated to rely on the serialization context.

Therefore, the "context aware" interfaces could be deprecated in favor of "regular" ones (and then save 4 interfaces in 7.0).

@mtarld mtarld requested a review from dunglas as a code owner November 9, 2021 17:42
@carsonbot carsonbot added this to the 6.0 milestone Nov 9, 2021
@carsonbot carsonbot changed the title [Serializer][Messenger] Deprecate "context aware" interfaces [Messenger][Serializer] Deprecate "context aware" interfaces Nov 9, 2021
@mtarld mtarld force-pushed the fix/deprecate-context-aware-interfaces branch from dd5d7c5 to f671ec2 Compare November 9, 2021 17:44
@mtarld
Copy link
Contributor Author

mtarld commented Nov 9, 2021

fabbot.io isn't happy because I touched to XmlEncoder:

-        throw new NotEncodableValueException('An unexpected value could not be serialized: '.(!\is_resource($data) ? var_export($data, true) : sprintf('%s resource', get_resource_type($data))));
+        throw new NotEncodableValueException('An unexpected value could not be serialized: '.(!\is_resource($data) ? var_export($data, true) : sprintf('"%s" resource', get_resource_type($data))));

But this isn't related to the current PR.

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

LGTM!

@mtarld mtarld changed the base branch from 6.0 to 6.1 December 24, 2021 09:29
@mtarld mtarld force-pushed the fix/deprecate-context-aware-interfaces branch from f671ec2 to 979ec87 Compare December 24, 2021 10:03
@mtarld mtarld force-pushed the fix/deprecate-context-aware-interfaces branch from 979ec87 to b81e087 Compare January 7, 2022 11:40
@mtarld mtarld force-pushed the fix/deprecate-context-aware-interfaces branch from b81e087 to ab72f80 Compare January 7, 2022 12:45
@fabpot
Copy link
Member

fabpot commented Jan 9, 2022

Thank you @mtarld.

@mtarld mtarld deleted the fix/deprecate-context-aware-interfaces branch January 10, 2022 08:33
@fabpot fabpot mentioned this pull request Apr 15, 2022
mbrodala added a commit to mbrodala/PhpEnums that referenced this pull request Jul 5, 2022
ogizanagi added a commit to Elao/PhpEnums that referenced this pull request Jul 11, 2022
This PR was merged into the 1.x-dev branch.

Discussion
----------

Add context to (de)normalizer support check

See symfony/symfony#43982

Commits
-------

ab69eab Add context to (de)normalizer support check
chalasr added a commit that referenced this pull request Aug 21, 2022
…erface` and `ContextAwareDecoderInterface` (nicolas-grekas)

This PR was merged into the 6.1 branch.

Discussion
----------

[Serializer] Revert deprecation of `ContextAwareEncoderInterface` and `ContextAwareDecoderInterface`

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

While reviewing #43231, I figured out that the correct fix for #38270 was to make `ChainEncoder` properly consider `ContextAwareEncoderInterface` when deciding to cache or not.

Since this interface is useful to discriminate the cache/no-cache situations, we have to undeprecate it (from #43982.)

/cc `@Guite` and `@mtarld` FYI

Commits
-------

9ac7fb1 [Serializer] Revert deprecation of `ContextAwareEncoderInterface` and `ContextAwareDecoderInterface`
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.

7 participants