Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store #type in top level objects only #72

Closed
ajgarlag opened this issue Jul 1, 2019 · 7 comments
Closed

Store #type in top level objects only #72

ajgarlag opened this issue Jul 1, 2019 · 7 comments

Comments

@ajgarlag
Copy link
Contributor

ajgarlag commented Jul 1, 2019

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.

@Toflar
Copy link
Contributor

Toflar commented Jul 1, 2019

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 😄

@dunglas
Copy link
Owner

dunglas commented Nov 6, 2019

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.

@ajgarlag
Copy link
Contributor Author

ajgarlag commented Nov 7, 2019

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.

@Toflar
Copy link
Contributor

Toflar commented Nov 7, 2019

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 🤔

@ajgarlag
Copy link
Contributor Author

ajgarlag commented Nov 7, 2019

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 Add option to don't store #type along json data #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.

@SherinBloemendaal
Copy link

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

@dunglas
Copy link
Owner

dunglas commented Jan 16, 2024

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

@dunglas dunglas closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants