schmittjoh / serializer

Library for (de-)serializing data of any complexity (supports JSON, and XML)
http://jmsyst.com/libs/serializer
MIT License
2.32k stars 589 forks source link

Impossible to use discriminator field. Why we need StaticPropertyMetadata ? #705

Open jewome62 opened 7 years ago

jewome62 commented 7 years ago

Hi, i have this error :

The discriminator field name "type" of the base-class "AppBundle\Entity\Message\Message" conflicts with a regular property of the sub-class "AppBundle\Entity\Message\Answer".

into src/JMS/Serializer/Metadata/ClassMetadata.php at line 189

 if (isset($this->propertyMetadata[$this->discriminatorFieldName])
                    && ! $this->propertyMetadata[$this->discriminatorFieldName] instanceof StaticPropertyMetadata) {
                throw new \LogicException(sprintf(
                    'The discriminator field name "%s" of the base-class "%s" conflicts with a regular property of the sub-class "%s".',
                    $this->discriminatorFieldName,
                    $this->discriminatorBaseClass,

I dont understand why StaticPropertyMetadata is required. In my case, this is entity with Inherance Joined table pattern. I have a field type to make discriminator stored into attribute $type into the entity. But the metadata is a instanceof PropertyMetadata

Do you have example? Why it have this constraint ? Thanks

goetas commented 7 years ago

Not everybody is using doctrine or a field for the type.

Using a pure object oriented approach, you do not need a "type field" (as doctrine does) to have an inheritance model; class hierarchy is enough. But if you want to store or to serialize your object graph, then the type field is necessary.

To answer your question, at this point you have to decide if you want to rely on your "database" representation or on your object representation. Both approaches have advantages and disadvantages.

jewome62 commented 7 years ago

Thanks for fast answer. I did not know we can do the second possibility.

I declare my variable messageType into jms Discriminator Annotation.

 * @JMS\Discriminator(field = "messageType", map = {
 *     "question" = "AppBundle\Entity\Message\Question",
 *     "answer" = "AppBundle\Entity\Message\Answer",
 *     "abuse" = "AppBundle\Entity\Message\Abuse",
 *     "comment" = "AppBundle\Entity\Message\Comment",
 *     "review" = "AppBundle\Entity\Message\Review"
 * })

messageType is declared nowhere else But it is not serialized. And that bug in deserialization.

How declare to serialize it ?

Thanks

PS : i use some group for serialization/deserialization

goetas commented 7 years ago

messageType probabily is declared in your abstract class.. is it? have to put @exclude on messageType

jewome62 commented 7 years ago

No it didn't. I can write message_type_blablabla_some_word, It dont be serialized. I have never use @exclude annotation

goetas commented 7 years ago

Can you post the code of the abstract class and of a child class?

jewome62 commented 7 years ago

Abstract Class : http://pastebin.com/SaSWaGap Intermediate abstract class : http://pastebin.com/XXFjnEWT Child class : http://pastebin.com/hirYcMZJ

Into my controller I create a Answer, with parent a question :and send into rabbitMQ

$serializer = $this->get('jms_serializer');
$this->get('old_sound_rabbit_mq.answer_producer')->publish(
$serializer->serialize($answer, 'json', SerializationContext::create()->setGroups(['save'])
));

And when i deserialize

$answer = $this->serializer->deserialize($msg->body, Answer::class, 'json', DeserializationContext::create()->setGroups(['save']));

I have this error :

[LogicException]                                                                                                           
  The discriminator field name "messageType" for base-class "AppBundle\Entity\Message\Message" was not found in input data.  
goetas commented 7 years ago

in answer you should use the abstract class...

$answer = $this->serializer->deserialize($msg->body, Message::class, 'json', DeserializationContext::create()->setGroups(['save']));

Do you have in the JSON serialized representation a filed called message_type ?

jewome62 commented 7 years ago

Data into rabbitMQ ( as a dump)

{
  "content": "Seuls votre pr\u00e9nom et votre pays seront associ\u00e9s \u00e0 votre r\u00e9ponse, les autres informations resteront confidentielles.",
  "author": {
    "personal_data_id": "10E8D408-F7F9-AEC9-0359-C2A8BD706F7A",
    "id": 139060
  },
  "channel": "front",
  "parent": {
    "content": "Est-ce-qu'elles sont id\u00e9ale pour  un travail debout (mise en rayon) ?",
    "author": {
      "personal_data_id": "674E73C5-2E91-0A2C-A6BD-B9D427EF1FD9",
      "id": 139057
    },
    "channel": "front",
    "created_at": "2017-01-20T16:57:52+0100",
    "context": {
      "id": 3
    },
    "status": "pending",
    "topicOrigin": "8342420",
    "category": {
      "id": 18,
      "context": {
        "id": 3
      }
    },
    "topicType": "product",
    "topicInfo": {
      "name": "",
      "code": "8342420",
      "url_photo": "spid-media-cloud\/packshot\/b2\/50\/b250123a189748e0a7268d87c00136da.jpg",
      "brand_name": ""
    }
  },
  "created_at": "2017-01-23T14:46:20+0100",
  "context": {
    "id": 3
  },
  "status": "saved"
}

Note : message_type is a typo with my previous test : in abstract class it is write @JMS\Discriminator(field = "messageType", map = {

goetas commented 7 years ago

as you can see there is not message type serialized in your JSON... this why the deserialization fails.

Which version of the serializer are you using?

jewome62 commented 7 years ago
root@qanda:/var/www/qanda# composer show | grep jms
Do not run Composer as root/super user! See https://getcomposer.org/root for details
jms/metadata                             1.6.0               Class/method/property metadata management in PHP
jms/parser-lib                           1.0.0               A library for easily creating recursive-descent parsers.
jms/serializer                           1.4.2               Library for (de-)serializing data of any complexity; supports XML, JSON, and YAML.
jms/serializer-bundle                    1.1.0               Allows you to easily serialize, and deserialize data of any complexity
goetas commented 7 years ago

ah, yes, you forgot the groups for the discrimintor.

 * @JMS\Discriminator(field = "message_type", map = {
 *     "question" = "AppBundle\Entity\Message\Question",
 *     "answer" = "AppBundle\Entity\Message\Answer",
 *     "abuse" = "AppBundle\Entity\Message\Abuse",
 *     "comment" = "AppBundle\Entity\Message\Comment",
 *     "review" = "AppBundle\Entity\Message\Review"
 * }, groups = {"save"})
jewome62 commented 7 years ago
The annotation @JMS\Discriminator declared on class AppBundle\Entity\Message\Message does not have a property named "groups". Available properties: map, field, disabled
jewome62 commented 7 years ago

I see the group property on master. I think it's for the 1.5.0 ?

goetas commented 7 years ago

it looks so.. https://github.com/schmittjoh/serializer/pull/579 introduced it and was merged on 3 Dec 2016 ... can you try to use the 1.5 rc?

jewome62 commented 7 years ago

I install it. And now that work. Thank you for the time you gave me.

Toilal commented 7 years ago

I'm using 1.6.2, and this feature is really confusing and/or partially broken. I just spent 2 hours to make it work.

Official reference states that type field should not exists. But it seems it has to both exists AND be excluded.

/**
 * @Discriminator(disabled=false, map={
 *     "null": "AppBundle\Model\Query\Value\NullValue",
 *     "string": "AppBundle\Model\Query\Value\StringValue",
 *     "boolean": "AppBundle\Model\Query\Value\BooleanValue",
 *     "integer": "AppBundle\Model\Query\Value\IntegerValue",
 *     "double": "AppBundle\Model\Query\Value\DoubleValue",
 *     "datetime": "AppBundle\Model\Query\Value\DateTimeValue"
 *     })
 */
abstract class ObjectValue
{
    /**
     * @Exclude
     * @var string
     */
    private $type;
}

If I remove @Excluded, deserialization fail with this message :

The discriminator field name "type" of the base-class "AppBundle\Model\Query\Value\ObjectValue" conflicts with a regular property of the sub-class "AppBundle\Model\Query\Value\StringValue".

If I remove $type private field, it fails with this message

Property AppBundle\Model\Query\Value\ObjectValue::$type does not exist

goetas commented 7 years ago

If I remove $type private field, it fails with this message

Property AppBundle\Model\Query\Value\ObjectValue::$type does not exist

you have to clean the cache in this case

mever commented 3 years ago

Not everybody is using doctrine or a field for the type.

Using a pure object oriented approach, you do not need a "type field" (as doctrine does) to have an inheritance model; class hierarchy is enough. But if you want to store or to serialize your object graph, then the type field is necessary.

To answer your question, at this point you have to decide if you want to rely on your "database" representation or on your object representation. Both approaches have advantages and disadvantages.

  • if you rely on database structure, then if you have new requirements at API level, then you have to change the database structure.
  • if you rely on the object graph, you have more flexibility, but a double configuration to maintain.

Can you tell me why we need to choose? If I just remove the exception it works beautifully. Thanks for putting time and effort into this project!

Edit: @goetas ping :)

goetas commented 3 years ago

@mever how would you decide the class to instantiate when deserializing that payload?

A field for the type is used currently to decide which class to instantiate when doing the deserialization. I do not know a better way (that does not require custom glue code to handle it)

mever commented 2 years ago

Hi @goetas,

My apologies it took me long to respond to you. My answer is specific to JSON, but the theory about types is generally applicable (you probably know that already, it is here for the community).

Information about types needs to be complete on the server-side (i.e. to where you're deserializing values). Serialized information can be incomplete. For example:

Information about a value of type A, that's a subtype of type B is always present server-side. I.e.: A being a PHP subclass of B. However, in serialized form, this information can be lost. I.e.: a JSON object does not carry more specific type information besides that the value is an object.

Usually, a field in serialized form has just one possible type, be it a boolean, string, Comment- or Product object. The problem arises when a field can contain stronger or weaker variants of a type (e.g. due to inheritance). For a string, this looks like a longer or short sequence of characters. An object is more difficult because that information may be missing from the serialized form. We solved this problem by including a 'special' field that carries information about the type.

Long story short, you always need to know the type of data (in serialized form or not). When serializing a PHP object this information can be read from the PHP object metadata. When deserializing JSON you need a field to convey this information (or assume one type).

So, to give a concrete answer to your question "how would you decide the class to instantiate when deserializing that payload?": By reading a type field in the JSON object that you're deserializing. Thus, here we agree.

The problem I have is with the exception. My stand is that the discriminator field, the PHP extends keyword, and the field or property of the class or serialized data, are different implementations of the same concept, they convey the same information. Removing the exception proves that it works for us. So the reported conflict is only in implementation and not in concept.

To work around the exception, we made a DoctrineTypeDriver that removes the discriminator field from the property metadata when loading the metadata for the class.