Proposal to change transformer for symfony serializer

[eerison]

The idea of this proposal is keep the code simple and using more Symfony features.

The idea of Transformer is convert the object to json or the other way round, But IMO It can be handled "easily" for symfony serializer componente

[VincentLanglet]

Do you plan to try a PR ?

[Hanmac]

sounds good, do you want to try it?
how exactly does doctrine type="json" work there? you probably don't need to serialize the object yourself, but just need the normalizer part into an array, so doctrine can encode it into json, right?

[eerison]

Do you plan to try a PR ?

Yep, if we agree I can provide a PR!

the idea is: first coverage the class, to make sure that we not forget anything, Definitely we need this before touch in Transformer, almost no test: https://app.codecov.io/gh/sonata-project/SonataPageBundle/blob/4.x/src/Entity/Transformer.php

Sounds good, do you want to try it?

Well I can try it, But if you want to do, I'm fine with this too, I can check others things that are necessary for 4.0 release. (I'm still on #1497 😄 )

how exactly does doctrine type="json" work there? you probably don't need to Serialize the object yourself, but just need the Normalize part into an array, so doctrine can encode it into json, right?

well we can do like this, convert to array and let the database handle this for json, maybe we can check this with Phpstan somehow too, But well we need to test :)

[Hanmac]

if really needed, i saw https://symfony.com/doc/4.4/components/serializer.html#built-in-normalizers
especially JsonSerializableNormalizer

This means that nested JsonSerializable classes will also be normalized.

it might help us to normalize the Block with the Child Blocks?

EDIT: but it doesn't support denormalize :(

[eerison]

if really needed, i saw https://symfony.com/doc/4.4/components/serializer.html#built-in-normalizers especially JsonSerializableNormalizer

This means that nested JsonSerializable classes will also be normalized.

it might help us to normalize the Block with the Child Blocks?

EDIT: but it doesn't support denormalize :(

Well I use this to parse object -> json and json -> object, and it works quite well

something like this: https://symfony.com/doc/4.4/components/serializer.html#deserializing-in-an-existing-object

[Hanmac]

if really needed, i saw https://symfony.com/doc/4.4/components/serializer.html#built-in-normalizers especially JsonSerializableNormalizer

This means that nested JsonSerializable classes will also be normalized.

it might help us to normalize the Block with the Child Blocks?
EDIT: but it doesn't support denormalize :(

Well I use this to parse object -> json and json -> object, and it works quite well

something like this: https://symfony.com/doc/4.4/components/serializer.html#deserializing-in-an-existing-object

you don't want to deserialize but to just denormalize, and i don't know if that option exist there too

EDIT: i stand corrected, it exist for https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php too

[eerison]

if really needed, i saw https://symfony.com/doc/4.4/components/serializer.html#built-in-normalizers especially JsonSerializableNormalizer

This means that nested JsonSerializable classes will also be normalized.

it might help us to normalize the Block with the Child Blocks?
EDIT: but it doesn't support denormalize :(

Well I use this to parse object -> json and json -> object, and it works quite well
something like this: https://symfony.com/doc/4.4/components/serializer.html#deserializing-in-an-existing-object

you don't want to deserialize but to just denormalize, and i don't know if that option exist there too

EDIT: i stand corrected, it exist for https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php too

it's enough, isn't it?

[Hanmac]

my thought was that you don't need the serializer when you just normalizing/denormalizing

[eerison]

my thought was that you don't need the serializer when you just normalizing/denormalizing

well But I need to convert from
object to array -> save into the database
and array to object -> to use into the code

or am i missing something?

[eerison]

my thought was that you don't need the serializer when you just normalizing/denormalizing

or do you mean the Selializer class?

[Hanmac]

i meant the serializer class, but we still might use it for the nice functions

object to array -> save into the database
and array to object -> to use into the code

is what the normalizer/denormalizer does

[eerison]

i meant the serializer class, but we still might use it for the nice functions

object to array -> save into the database
and array to object -> to use into the code

is what the normalizer/denormalizer does

well maybe you could provider a simple example how we should use this!

[Hanmac]

there is an example of just using normalizer with denormalize https://symfony.com/doc/4.4/components/serializer.html#camelcase-to-snake-case

[Hanmac]

also should we use make own Serializer instance or should we use SerializerInterface and let Symfony define it?
https://symfony.com/doc/current/serializer.html#using-the-serializer-service

[eerison]

hmmm good question!

I would use SymfonyInterface and pass the instace that we configurate! why?

because we use snake_case but the guy configure CamelCase in his config, then it'll convert to propertyName instead of property_name

[eerison]

there is an example of just using normalizer with denormalize https://symfony.com/doc/4.4/components/serializer.html#camelcase-to-snake-case

hmmm nice, maybe we could depends of ObjectNormalizerInterface and not the SerializerInterface.

[Hanmac]

the DenormalizerInterface probably doesn't exist as service like the SerializerInterface does

[eerison]

the DenormalizerInterface probably doesn't exist as service like the SerializerInterface does

well I would choose the option to have this injected in the service instead of use the new ObjectNormalizer into the class/code!

[Hanmac]

@eerison if you can try to make an MR for the coverage, then i can later try to make an MR for the serializer (if you didn't already)

[eerison]

@eerison if you can try to make an MR for the coverage, then i can later try to make an MR for the serializer (if you didn't already)

well I'm still working on #1497

[Hanmac]

@VincentLanglet assuming i want to write testcases for the Transformer functions:
Using the Transformer as real objects and the manager as mocks, do i need to mock the Page/Block/Snapshot entities too?
Or can i use the tests/App/Entity ones to check if the Snapshot has the same values as the Page/Block?

[eerison]

I guess you can mock the DB, you just need to check if after convert from object to array (and the other way round 😄 ), it's as expected!

Maybe you can create one db test, just to make sure that passing array to content, it'll be saved as json

[Hanmac]

for the (first) transformer test, i don't need the DB, i just mock the manager, i was just thinking about using the fake Entity objects so i don't need to mock getContent and setContent stuff and all the other getter/setter when the Transformer is coping from Page to Snapshot and vice versa.

[eerison]

for the (first) transformer test, i don't need the DB, i just mock the manager, i was just thinking about using the fake Entity objects so i don't need to mock getContent and setContent stuff and all the other getter/setter when the Transformer is coping from Page to Snapshot and vice versa.

well if you can cover the functionality with a fake entity I'm fine with this :)

[VincentLanglet]

@VincentLanglet assuming i want to write testcases for the Transformer functions: Using the Transformer as real objects and the manager as mocks, do i need to mock the Page/Block/Snapshot entities too? Or can i use the tests/App/Entity ones to check if the Snapshot has the same values as the Page/Block?

i dunno this bundle to give good advice without a PR to rely on.
You can start with the simpler solution you have in mind

[eerison]

Hey @Hanmac I don't know if it helps somehow, But into the block bundle there is a definition for serializer, maybe it help somehow: https://github.com/sonata-project/SonataBlockBundle/blob/4.x/src/Resources/config/serializer/Model.BaseBlock.xml

or maybe we should define a new group there for fake/group block(container block), I guess make sense it be defined in block-bundle because we are handle block normalizer and denormalizer, what do you think?

[Hanmac]

Hey @Hanmac I don't know if it helps somehow, But into the block bundle there is a definition for serializer, maybe it help somehow: https://github.com/sonata-project/SonataBlockBundle/blob/4.x/src/Resources/config/serializer/Model.BaseBlock.xml

or maybe we should define a new group there for fake/group block(container block), I guess make sense it be defined in block-bundle because we are handle block normalizer and denormalizer, what do you think?

i will check this out later, first i do am MR for Transformer testcases

[Hanmac]

there was https://github.com/sonata-project/SonataPageBundle/blob/3.x/src/Resources/config/serializer/Model.Block.xml
and https://github.com/sonata-project/SonataPageBundle/blob/3.x/src/Resources/config/serializer/Model.Page.xml
i might recycle for my possible change

[jordisala1991]

AFAIK , those are for the JMS serializer, used on 3.x for the API (already removed on 4.x). Not sure if that works for symfony/serializer.

[Hanmac]

@eerison you use this Snapshots right? can you explain to me what the SnapshotPageProxy is for?

and also if it would be okay to change the format of the Json Content? For example, the https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php may use a different format for the DateTime values. or should i try to keep the U format?

[eerison]

@eerison you use this Snapshots right? can you explain to me what the SnapshotPageProxy is for?

and also if it would be okay to change the format of the Json Content? For example, the https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php may use a different format for the DateTime values. or should i try to keep the U format?

Hey @Hanmac I use snapshot because it's a base feature from sonata page bundle, But I do not know why exactly we need the SnapshotProxy, But as it's a proxy I assume that the main idea behind this is intercept the snapshot behaviour, But it's just my felling!

and about the DatetimeNormalizer, which field are datetime? create and updated at? maybe it's not used ?! But if we don't know if it's used or not we should keep the same behaviour of 3.x, or provide a PR in 3.x saying that it will be changed in 4.x and need to be update!

But maybe you can create a custom Normalizer for datetime I guess!

[Hanmac]

I made a first try of the Serializer change: #1522

you guys need to check if that counts as BC or not

i kept the Datetime format and the order of the elements, so the Testdata didn't need to change

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Hanmac]

shush git bot, i work on the PR

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Hanmac]

The PR still exist