dunglas/doctrine-json-odm

Store `#type` in top level objects only

ajgarlag opened this issue ยท 7 comments

Can we store the #type key in top level objects only?

For any other object in the graph, we should leverage the standard symfony serializer ability to extract the the object class reading the phpdoc or type-hint.

Should be doable, yes. Although we need to make sure it can still handle the nested ones for BC reasons. I don't feel like migrating all the old data ๐Ÿ˜„

It's not totally doable. One great feature of this tool is that you can store totally dynamic data (mixed in PHP). For instance, let's imagine the following object graph:

class X {
     public $rel;
}
class Foo {}
class Bar {}

$x1 = new X();
$x1->rel = new Foo();

$x2 = new X();
$x2->rel = new Bar();

$dataToStore = [$x1, $x2];

This type of very dynamic graph cannot be handled without storing #type at every level.

But for that type of very dynamic graph, you can write a custom normalizer, or implement Symfony\Component\Serializer\Normalizer\DenormalizableInterface.

I think we can have an option (not enabled by default) to choose if we want to store the #type in top level objects only and use the standard serializer extension points to inject custom (de)normalizers if needed.

Don't we usually know the top level type already? In most cases for me that looks either like that:

    /**
     * @var Entity
     *
     * @ORM\Column(type="json_document")
     */
    private $entity;

or like so:

    /**
     * @var array<Entity>|Entity[]
     *
     * @ORM\Column(type="json_document")
     */
    private $entities;

So I think either we always store the #type with it or we don't need it at all ๐Ÿค”

I see three different strategies here:

  1. To store #type in any level: (Current behaviour) Implemented as a custom DBAL type that passes the class to deserialize as an empty string, and a custom serializer that adds the #type key into all serialized objects.
  2. Not to store any #type key: Proposed in #63, can be implemented with a custom DBAL type that must guess the classname of the object to deserialize. We can use the PropertyInfo component, or add a new option to the annotation to guess the classname. To denormalize complex or dynamic object graphs, the user should inject custom denormalizers into the serializer.
  3. To store the #type in top level only: Proposed here, can be implemented using the current DBAL type, but with a different custom serializer that adds the #type key in top level object only. To denormalize complex or dynamic object graphs, the user should inject custom denormalizers into the serializer. It is a compromise between options 1 and 2.

PHP supports union types since 8.0 so storing a type is very useful in my opinion :)

Closing, we don't intend to change the current behavior for the reasons exposed, and because of union types.