Transformer use Symfony Serializer

[Hanmac]

Subject

This makes the Transformer use Symfony Serializer to normalize/denormalize the Content Array.

Under 4.4 Symfony i had the following restrictions:

• it wasn't easy to exclude attributes, you only could ignore them either global via name, but don't class specific like i wanted, the current solution is to use a group name
• the group name also has the benefit that it orders the attributes, without it, it seems to use the order inside the php class
• You can't denormalize for an Interface, or abstract class. so when it tired to load the block children, it had no idea what to do. And i didn't want to use a discriminator map and type attribute inside the array. I solved this using my Own Denormalizer that is looking for an Interface and Context parameters.

I will add more Testcases soon with doing a functional test using the Symfony Kernel.

I am targeting this branch, because i don't know if that could count as BC or not.

Closes #1503.

Changelog

### Changed
- Uses Symfony Serializer to transform Page and Block Models into array for json

### Deprecated
- Not implementing PageInterface::removeChild
- Not implementing PageInterface::removeBlock
- Not passing Serializer to Transformer as 5th argument

[VincentLanglet]

Adding a mandatory param to a constructor is a BC break.
The BC way is to use null as a default and if null is provided either do new Service(), either use the old logic.

Here I'm not sure it's worth it to provide a BC PR, it might be easier to target directly 4.x
WDYT @jordisala1991 ?

[jordisala1991]

I think it might be nice to have this branch for testing purposes on real apps that are actually using 3.x, but the real PR should target 4.x

[eerison]

Adding a mandatory param to a constructor is a BC break. The BC way is to use null as a default and if null is provided either do new Service(), either use the old logic.

Here I'm not sure it's worth it to provide a BC PR, it might be easier to target directly 4.x WDYT @jordisala1991 ?

I guess just add the serializer as optional, and if it's not passed get from the container, it's fine

[eerison]

I think it might be nice to have this branch for testing purposes on real apps that are actually using 3.x, but the real PR should target 4.x

Do you think it's too much for a minor?

[VincentLanglet]

Adding a mandatory param to a constructor is a BC break. The BC way is to use null as a default and if null is provided either do new Service(), either use the old logic.
Here I'm not sure it's worth it to provide a BC PR, it might be easier to target directly 4.x WDYT @jordisala1991 ?

I guess just add the serializer as optional, and if it's not passed get from the container, it's fine

You dont have the container either in this class

[Hanmac]

For the last issue, should i just suppress it?

if i don't it just complains somewhere else in the Denormalizer

[VincentLanglet]

For the last issue, should i just suppress it?

if i don't it just complains somewhere else in the Denormalizer

The interface ContextAwareDenormalizerInterface is deprecated since Symfony 6.1
https://github.com/symfony/serializer/blob/6.1/Normalizer/ContextAwareDenormalizerInterface.php#L21

Since like it will be added directly to https://github.com/symfony/Serializer/blob/6.1/Normalizer/DenormalizerInterface.php#L59

So I would recommend to not use the interface.
Then we will see the issue you get

[Hanmac]

especially this one i don't know how to fix:

supportsDenormalization() invoked with 4 parameters, 2-3 required.

[VincentLanglet]

For

Method Sonata\PageBundle\Serializer\InterfaceDenormalizer::supportsDenormalization() has parameter $context with no value type specified in iterable type array.

You need to add phpdoc.

For

Method Symfony\Component\Serializer\Normalizer\DenormalizerInterface::supportsDenormalization() invoked with 4 parameters, 2-3 required.
We will ignore the error (@phpstna-ignore-next-line) with a message to explain why

For

@psalm-suppress MoreSpecificImplementedParamType

I think we have to make the serializerAwareInterface generic in Symfony

[VincentLanglet]

@Hanmac there is a DenormalizerAwareInterface.

Dont you think you should both implement DenormalizerAwareInterface and SerializerAwareInterface ?
This will avoid to override the phpdoc of setSerializer and avoid the psalm issue.

[Hanmac]

@Hanmac there is a DenormalizerAwareInterface.

Dont you think you should both implement DenormalizerAwareInterface and SerializerAwareInterface ? This will avoid to override the phpdoc of setSerializer and avoid the psalm issue.

hm i was using ArrayDenormalizer as inspiration, seems it changed after 4.4 for some reason

[Hanmac]

hm where does it want me to do the psalm thing?

[VincentLanglet]

hm where does it want me to do the psalm thing?

I would say it's because you're using \* and not \**

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[Hanmac]

tests are broken, but not by me

[VincentLanglet]

tests are broken, but not by me

What do you mean ? Tests are green

[VincentLanglet]

Just a linter error to fix

[Hanmac]

also happy anniversary for this PR 🥳

[VincentLanglet]

Can you update the ## Changelog part @Hanmac ?

[Hanmac]

i'm not very good at writing such change logs

like do we need to add the other deprecations too?

[VincentLanglet]

i'm not very good at writing such change logs

like do we need to add the other deprecations too?

It would be something like

Changed:
- Use serializer to do ....

Deprecated:
- Not implementing PageInterface::foo
- Not implementing PageInterface::bar
- Not passing Serializer to FooService as 5th argument (or 4th or 3rd...)

[VincentLanglet]

Thanks !

Since I go in vacations at the end of the week, I prefer to delay the release.

[VincentLanglet]

https://github.com/sonata-project/SonataPageBundle/releases/tag/4.6.0