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

Introduce a marker interface for annotation/attribute classes #1529

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Dec 13, 2023

Q A
Bug fix? yes
New feature? yes
Doc updated no
BC breaks? yes and no...
Deprecations? no
Tests pass? yes
Fixed tickets schmittjoh/JMSSerializerBundle#944, #1525
License MIT

The standalone attribute driver doesn't do any kind of filtering on attribute classes that it tries to use, compared to the deprecated attribute reader decorator (implementing the doctrine/annotations reader interface) which uses a namespace filter. This PR introduces a marker interface for the serializer's annotation/attribute classes and adjusts the Reflection getAttributes() calls to filter down to attribute classes that implement the interface. This solves the issue of trying to handle missing classes or trying to work with incompatible attributes by telling the engine to filter down to only supported classes without the need for other creative checks.

Since #1525 added tests dealing with missing classes, there's no need to add any other test cases as the fixtures already have classes with attributes from mixed providers. If anything, the benchmark slightly improves since it won't try to instantiate unsupported attribute classes anymore (or have to check those in the driver while building the metadata object).

@scyzoryck scyzoryck merged commit 111451f into schmittjoh:master Dec 14, 2023
21 of 23 checks passed
@mbabker mbabker deleted the annotations-marker-interface branch December 14, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants