[WIP] Add getSupportedTypes method to Normalizers #5611
Conversation
Add getSupportedTypes method
|
If we're not using the $context in the support method it's quite easy to add in other normalizers, I'm on the symfony slack if you need more informations! |
Co-authored-by: Marco Spengler <marco.spengler@skriptfabrik.com>
| return true; | ||
| } | ||
|
|
||
| public function getSupportedTypes(?string $format) | ||
| { | ||
| return [ |
There was a problem hiding this comment.
I'm really not sure about this modification.
I've tried to understand the purpose of the getSupportedTypes method and adapted it to our class.
I'm waiting for your feedback to continue on the subject.
There was a problem hiding this comment.
This method has the purpose to simplify and improve the build-up of the normalizer/denormalizer cache of the Symfony Serializer.
Symfony < 6.2 used the result of the supportsDenormalization and supportsNormalization to do this, but these methods could heavily rely on the actual data, type, context arguments which could cause significant performance impacts depending on the executed code.
You might wanna take a look at the Symfony blog post and the linked PR.
| public function getSupportedTypes(?string $format) | ||
| { | ||
| return match($format) { | ||
| static::FORMAT => [ |
There was a problem hiding this comment.
I'm really not sure about this modification.
I've tried to understand the purpose of the getSupportedTypes method and adapted it to our class.
I'm waiting for your feedback to continue on the subject.
There was a problem hiding this comment.
See #5611 (comment)
| return [ | ||
| HalItemNormalizer::class => true, | ||
| JsonApiItemNormalizer::class => true, | ||
| JsonLdItemNormalizer::class => true, | ||
| ItemNormalizer::class => true, | ||
| ]; |
There was a problem hiding this comment.
I think this must return ['object' => false].
The description of the getSupportedTypes method indicates a result cannot be cached when it depends on the context or on the data. which the AbstractItemNormalizer does.
Generally, this method should return which objects are supported by the normalizer, and these listed classes here are normalizers on their own which, will never be de/normalized by the ItemNormalizer.
| return match($format) { | ||
| static::FORMAT => [ | ||
| HalCollectionNormalizer::class => true, | ||
| HydraCollectionNormalizer::class => true, | ||
| JsonApiCollectionNormalizer::class => true, | ||
| ], | ||
| default => [], | ||
| }; |
There was a problem hiding this comment.
I think this must return ['*' => false].
The description of the getSupportedTypes method indicates a result cannot be cached when it depends on the context or on the data. which the AbstractCollectionNormalizer does, it verifies that the provided data is iterable.
|
I'm not sure you're going the right way about this - the supported types should be the ApiResources you want the normalizers to support, not nromalizer instances. Depending on how symfony does this, it might be done when initializing operations - you'd register the classes you want supported on the right normalizer? |
|
Someone have an exemple how to remove this depreciation ? Thank in advance, |
|
@john-dufrene-dev this is the best example from the Symfony blog article. The documentation section about performance and custom normalizers is still describing the use of the deprecated methods. This should be a good starting point to remove the deprecation in your own normalizers, for API Platform, at least the item normalizer seems to be a bit more complicated as it discovers the ability to support the provided data by evaluating if it's an API resource. I think this normalizer has either no catchable supports method at the moment, based on this behavior, or another solution could be to discover all API resources in a compiler pass and pass this knowledge to the normalizer to look up, but I am not sure how this will behave if you add or remove an API Resource during development. Also, this could still be a solution for a production environment where this will not change over time. |
|
Okay thanks for your comment, But in some project I don't use my own serialiser just default serializer but I have dépréciation. Just need to wait New release with PR so if I understand ? Sorry for m'y Bad english |
|
@john-dufrene-dev You are right, if you are facing indirect deprecations, from API Platform in this case, you have to wait for a release that fixes those. |
|
Thanks for informations :) |
| @@ -54,11 +57,28 @@ public function supportsNormalization(mixed $data, string $format = null, array | |||
| return static::FORMAT === $format && is_iterable($data); | |||
| } | |||
|
|
|||
| /** | |||
| * @deprecated since Symfony 6.3, use "getSupportedTypes(?string $format)" instead | |||
| public function hasCacheableSupportsMethod(): bool | ||
| { | ||
| trigger_deprecation('symfony/serializer', '6.3', 'The "%s()" method is deprecated, use "getSupportedTypes()" instead.', __METHOD__); |
There was a problem hiding this comment.
| trigger_deprecation('symfony/serializer', '6.3', 'The "%s()" method is deprecated, use "getSupportedTypes()" instead.', __METHOD__); | |
| trigger_deprecation('api-platform/core', '3.2', 'The "%s()" method is deprecated, use "getSupportedTypes()" instead.', __METHOD__); |
Same for all notices (using the right package name) as this is API Platform's code
|
Since api-platform supports symfony/serializer 6.1 and higher, the |
|
hey there @Oipnet , any news on that PR? Is it still ongoing? |
|
I tried working a bit on a separate PR for this since there is no recent activity here. It is still a draft since i haven't looked into testing this correctly, but it seems to do the job for my use...if you're interested i'd gladly take comments : #5672 |
Add getSupportedTypes method
See the article https://symfony.com/blog/new-in-symfony-6-3-performance-improvements#improved-performance-of-serializer-normalizers-denormalizers
In Symfony 6.3, we've added a getSupportedTypes(?string $format): array method to normalizers/denormalizers so they can declare the type of objects that they can handle and whether they are cacheable.
Need help for others Normalizers