Bug Fix: DataTransformer returning same class as original should include JSONLD Data #3479
Conversation
|
Scrutinizer does not appear to be related to this PR. Behat test added. If this shouldn't be allowed perhaps it's worth throwing some kind of exception, but it does seem a pretty useful way of using this feature for me and it doesn't seem to cost much to allow this feature to be used in this way. |
|
The latest commit handles both if the dto output class is set as the same as the original resource and if the transformer falls back to returning the original resource. This is done using a serialization context key. As the original resource will use the item normalizer that supports API Platform resources, this context key allows us to prevent an infinite loop. |
|
All test failures appear to be related to the 2.5 branch and not this PR. |
|
@teohhanhui do you have thoughts on this? I think that the use case is real, just not fond of the context to prevent recursion, maybe that there's something better to be done? |
tests/Fixtures/TestBundle/DataTransformer/OutputDtoSameClassTransformer.php
Outdated
Show resolved
Hide resolved
In `ApiPlatform\Core\DataTransformer\DataTransformerInterface` there is a comment that states that DTOs should be allowed to return the same original object if no transformation is done. This resulted in missing LD data. (No @id, @context etc.). ```php /** * Transforms the given object to something else, usually another object. * This must return the original object if no transformation has been done. * * @param object $object * * @return object */ public function transform($object, string $to, array $context = []); ``` This update checks if the output class is the same as the original, and if so populated the extended metadata in the JsonLd\ItemNormalizer as it would not be added using the JsonLd\ObjectNormalizer
When a DataTransformer returns the original resource as a fallback, a context key is set so when repeating the serialization/normalization we can populate the extended jsonld data
Previous push reverted a change which is beneficial to performance. Instead of re-looping over the serializer/normalizers, if the DTO output class is defined as the same as the resource, we can detect this in the 1st round of serialization
…ted class from PropertyInfo extractor
…some modified data if needed
…set no matter how the same class/object is returned
- Prevent BC change on AbstractItemNormalizer::transformOutput - Remove useless documentation
|
Hi, I did make these changes but I think I did something wrong with rebasing and I'm not sure how to notify that the changes requested are done ... so this is me doing that. |
tests/Fixtures/TestBundle/Document/DummyDtoOutputFallbackToSameClass.php
Outdated
Show resolved
Hide resolved
tests/Fixtures/TestBundle/Entity/DummyDtoOutputFallbackToSameClass.php
Outdated
Show resolved
Hide resolved
|
Thank you @vincentchalamon for the review - all changes applied now. I'll wait for core team's opinion. |
`src/Serializer/AbstractItemNormalizer.php`: Update constant name Change author phpdoc on test documents and entities
|
I like this, thanks @vincentchalamon @silverbackdan |
|
@silverbackdan Could you please update your branch from the 2.5 remote branch? |
|
Thanks all! @vincentchalamon - I've just pressed the button to update from 2.5 remote - seems to have been able to merge without conflict. |
|
thanks @silverbackdan |
In
ApiPlatform\Core\DataTransformer\DataTransformerInterfacethere is a comment that states that DTOs should be allowed to return the same original object if no transformation is done. This resulted in missing LD data. (No @id, @context etc.).This update checks if the output class is the same as the original, and if so populated the extended metadata in the JsonLd\ItemNormalizer as it would not be added using the JsonLd\ObjectNormalizer