Add support for dynamic handlers

[Spea]

This adds support for (De)Serializer handlers which can be used to modify how a class is (de-)serialized. A good example is the uuid package from symfony or ramsey where the class should be serialized to a string and not an object.

This is (somehwat) a proof of concept to get a discussion starting, because I'm not sure if this is the right approach, but there are some use cases where it just does not make sense to add native support in this library, but rather give the user of the library the possibility to add different (de-)serialization.

[dbu]

thank you, this looks actually quite good to me. i like that the resolution which handler to use is done at "compile" time and not while (de)serializing. the approach looks clean enough to me.

how does this relate to when using jms serializer directly? can one do the same there? we should be compatible.

we should document this. and we should mention that virtual properties are easier than writing custom handlers, and when the use case allows for that should therefor be preferred.

btw, have you seen https://symfony.com/doc/current/serializer/streaming_json.html ? i did not dig fully into it, but it seems to be a minimal serializer that is optimized for performance. at symfony live, people told me that it also generates code at cache warmup time. i did not dig into it, but if it works well and solves a similar need, i am tempted to recommend it over the liip/serializer (we don't use it internally anymore at liip, thus its difficult to find time maintaining it)

[Spea]

how does this relate to when using jms serializer directly? can one do the same there? we should be compatible.

In JMS this is also done via subscribing handlers, so we can not be compatible 1:1, as the code in here is created at compile time, and not at run time.

A good example is the SymfonyUidHandler which serializes (u)uids to strings and back to the actual class.

we should document this. and we should mention that virtual properties are easier than writing custom handlers, and when the use case allows for that should therefor be preferred.

Yeah, that I will do. Virtual properties should of course be prefered, but sometimes this is not that easy, especially with existing JMS attributes and third party classes/dependencies.

btw, have you seen symfony.com/doc/current/serializer/streaming_json.html ? i did not dig fully into it, but it seems to be a minimal serializer that is optimized for performance. at symfony live, people told me that it also generates code at cache warmup time. i did not dig into it, but if it works well and solves a similar need, i am tempted to recommend it over the liip/serializer (we don't use it internally anymore at liip, thus its difficult to find time maintaining it)

Yeah I have seen this and looked a bit into it, but it currently only works with classes that have only public properties and no constructor. Since we use a lot of JMS related features, this will be a hug amount of work for us to change. Of course its not impossible, but it takes time.

I'll improve this PR a bit and add some proper documentation around this feature then.
Aside that, do you mind me asking why you are no longer using the liip/serializer internally?

[Spea]

FYI: I added a new section in the README.md and added a new entry in the changelog.

[dbu]

right, now that you mention it, i think the discussion with json stream was to convert entities etc into value objects resp update from value objects. which is a valid concept and sound application design, but a different approach from typical serializers.

i am happy with the code, but looked closely at the documentation and tried to make it as user friendly as possible.

regarding not using internally: we are not working on the project for which we developed this library anymore.

would you be up to get merge & release rights on the 3 repositories for this?

[Spea]

right, now that you mention it, i think the discussion with json stream was to convert entities etc into value objects resp update from value objects. which is a valid concept and sound application design, but a different approach from typical serializers.

Yeah, it looks quite promising though, but for now we will probably stick with the classic serializer approach.

i am happy with the code, but looked closely at the documentation and tried to make it as user friendly as possible.

Really appreciate the thorough code review. I tackled your comments now.

would you be up to get merge & release rights on the 3 repositories for this?

Yeah of course, that would be fine for me 😃 Do you still want to keep reviewing PRs? Otherwise I could also ask my team to look at them when I need a review.

[dbu]

we should do this in the metadata. i did liip/metadata-parser#58

[dbu]

i tagged 2.1.1 of metadata-parser. can you please remove this (and the one in SerializerGenerator) and see if phpstan sees things correctly with the newest parser?

[Spea]

That worked for this case, but now there is an issue in the SerializerGenerator and the DeserializerGenerator

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   DeserializerGenerator.php
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------
  176    Parameter #1 $className of method Liip\MetadataParser\Metadata\ClassDiscriminatorMetadata::getMetadataForClass() expects class-string, string given.
         🪪  argument.type
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   SerializerGenerator.php
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------
  169    Parameter #1 $className of method Liip\MetadataParser\Metadata\ClassDiscriminatorMetadata::getMetadataForClass() expects class-string, string given.
         🪪  argument.type
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------

I think we'd have to map this property to a class-string[] array, but not sure if that is so easy, as this value is actually mapped from the Discriminator attribute and the type in this attribute is also just array<string>.

So I guess we have to "force" a docblock type somewhere. Should I take care of this in the metadata-parser, or rather in this repo?

[Spea]

You can see the necessary changes in this commit: 4f7570e

[dbu]

oh, once we start touching, everything starts to move... i think we should fix it in metadata, that would make the most sense. if you have time to look into it, best would be to test it in context of the serializer to see if we have more places that need to be fixed.

[Spea]

Sure, I'll take care of fixing it in the metadata-parser then

[Spea]

Fixed via liip/metadata-parser#59

[Spea]

Update to latest version via db394fe

[dbu]

yay! https://github.com/liip/serializer/releases/tag/3.2.0