-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Serializer] Properties extractor implementations #30980
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
[Serializer] Properties extractor implementations #30980
Conversation
a6349e8
to
ed46093
Compare
67af691
to
64458eb
Compare
Max Depth Handling is tricky to handle with that interface as it's not only about removing the property with max depth when normalizing but also about changing the value, there is some ways to do that but not sure which is one is better:
Do you have some input /cc @dunglas @soyuka @fbourigault @dbu @jewome62 @dmaicher ? |
i am not sure i understand the architecture of this. each extractor seems to build a list of properties - is that only a flat list of names, and recursion happens on a higher level? and are you going to intersect the results of the different extractors, or how will a combination of e.g. ignored and group be possible? for the liip serialilzer generator, we built a tool that builds the metadata of our models with all information we can find (and is extensible to add your own custom annotations). then, in a second step, we can reason on that metadata tree to filter out things. that would allow to cache the metadata once, and reuse it for different group configuration or excluded field lists. (in our use case, its also very useful to generate the serializer code, because the metadata holds all information about how to access the property) |
Each extractor build / filter a flat list of properties given a sub extractor and a context key.
Yes it will be possible, since each extractor rely on a sub extractor you can have something like this: Allowed -> Ignored -> Groups -> (Reflection| PhpDoc | Doctrine | ...)
This is the goal here, we can, like explained in #30818 add a Then we can have a CacheExtractor that cache a list of properties for a given class. One of the advantage of this design, is, by having the same interface from the beginning to the end, put our cache where we want. Sometimes i know that a for specific domain of my application i will always have the same groups / allowed properties / ignored one etc .... So in this case, i can put my cache in front of my filters. When i'm having a too dynamic setup i can put my cache in front of the "real" extractors so filtering happens after the cache. One other thing that this design allow is the ability to completely deactive some behaviour, for instance i never use allowed and ignored features of the serializer, having the possibility to simply remove this class will certainly offer a nice performance gain. |
in liip/metadata-parser, we combine the metadata information available from reflection & phpdoc & various annotation sources. its quite tricky to pick the right name for things, because annotations can specify the name to use vs the php fields and then naming strategies (convert camelCase to camel_case etc). we need to be sure that the process is well understood so the users can understand how that works. will it be possible to combine input from several sources or do i have to chose between annotations and reflection? when its only the serializer annotations, that extractor could decorate the reflection extractor. but when you also want to take information from doctrine annotations and whatnot, the decorating becomes problematic, you need something to enrich / combine information. how will the tools in this PR interact with something like property access that not only needs to know what exists but also how to access it? it feels redundant to me to process all kind of metadata to produce a string array of property names and then process more metadata to determine how to read/write each of those properties. |
It's already the case as the actual implementation use this: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/PropertyInfo/PropertyInfoExtractor.php which have several extractors possible
I don't see this problematic, decorating is a detail of implementation, you can use a combine strategy that use this interface or anything else. Current implementations use the
That precisely the point of #30704 so we can ensure we got properties list and how to read / write to them with the same implementation. |
*/ | ||
final class CheckCircularReferenceNormalizer implements NormalizerInterface, DenormalizerInterface, CacheableSupportsMethodInterface, NormalizerAwareInterface, DenormalizerAwareInterface, SerializerAwareInterface | ||
{ | ||
public const CIRCULAR_REFERENCE_LIMIT = 'circular_reference_limit'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure to duplicate const.
I think, for the moment. you can referrer to original const as :
public const CIRCULAR_REFERENCE_LIMIT = AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT
When the deprecated on AbstractNormalizer will be add, we will can revert the relation with
public const CIRCULAR_REFERENCE_LIMIT = CheckCircularReferenceNormalizer::CIRCULAR_REFERENCE_LIMIT
on AbstractNormalizer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jewome62 The const value can't be changed for BC, so it's alright.
That's chaining, not combining. There needs to be a way to get better information by combining raw information from multiple sources. |
Any news regarding this PR? |
We've just moved away from |
Related to #30818
This is an example of some property list extractor implementations, things that need to be done to complete this PR: