schmittjoh / serializer

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

Is it possible to set entity relationships just with the id when unserialize? #801

Open devnix opened 7 years ago

devnix commented 7 years ago

I'm trying to unserialize to persist an entity with multiple relationships.

With jms_serializer.doctrine_object_constructor.class I've been able to assign different child entities to a parent entity.

For example (a cart entity):

{
  "articles": [
    {
       "id": 1
    },
    {
       "id": 2
    },
    {
       "id": 3
    }
  ],
  "client": {
    "id": 43
  }
}

It works flawlessly. The problem is that I must update the articles entity, including his relationships, without modifying the child entities.

As far as I tested, this would modify the articles matching with their id.

{
  "articles": [
    {
       "id": 1,
       "name": "foo"
    },
    {
       "id": 2,
       "name": "foo"
    },
    {
       "id": 3,
       "name": "test"
    }
  ],
  "client": {
    "id": 43
  }
}

As I see in JMS\Serializer\Construction\DoctrineObjectConstructor line 82

{
  "articles": [1, 2, 3 ],
  "client": 43
}

should set the relationship using the numbers as references instead of the whole object as I understand, but this feature is not working. Is this correct?

bmeynell commented 7 years ago

@DevNIX - https://github.com/schmittjoh/serializer/pull/793/files was a fix related to Doctrine object refs. Give that a shot if you haven't already.

devnix commented 7 years ago

@bmeynell I'm using the latest release, and that PR seems integrated.

I've just found another StackOverflow, but it doesn't work too. https://stackoverflow.com/questions/14534234/deserializing-doctrine-references-with-jmsserializer

The error right now would be something like

Invalid data \"1\"(integer), expected \"FooBundle\\Entity\\Client\
goetas commented 7 years ago

if you have a look here https://github.com/schmittjoh/serializer/blob/master/src/JMS/Serializer/Construction/DoctrineObjectConstructor.php#L82 it is implemented as you need.

can you share your object metadata?

devnix commented 7 years ago

@goetas I will try to test carefully a new pair of entities on a different environment tonight at home, I'm struggling at work with urgent things :(

Edit: That's what I was understanding about https://github.com/schmittjoh/serializer/blob/master/src/JMS/Serializer/Construction/DoctrineObjectConstructor.php#L82, okay!

scaytrase commented 7 years ago

@DevNIX it's possible to write a custom handler for relations, like this

https://github.com/bankiru/jsonrpc-server-bundle/blob/release/2.0/Adapters/JMS/RelationsHandler.php

Currently this code serializes relation as ID or array of ID for *toMany relations and deserializes them back

sky003 commented 6 years ago

I have the same problem.

https://github.com/schmittjoh/serializer/blob/master/src/JMS/Serializer/Construction/DoctrineObjectConstructor.php#L82 ObjectConstructor works good, but looks like Visitor doesn't know enything about an object (entity) created by ObjectConstructor.

(1/1) RuntimeExceptionInvalid data "311"(string), expected "Entity\ORM\Organization".
--
in GenericDeserializationVisitor.php (line 170)
at GenericDeserializationVisitor->visitProperty(object(PropertyMetadata), 311, object(DeserializationContext))in GraphNavigator.php (line 261)
at GraphNavigator->accept(311, array('name' => 'Entity\\ORM\\Organization', 'params' => array()),object(DeserializationContext))in GenericDeserializationVisitor.php (line 181)

Bug?

goetas commented 6 years ago
{
  "articles": [
    {
       "id": 1
    },
    {
       "id": 2
    },
    {
       "id": 3
    }
  ],
  "client": {
    "id": 43
  }
}

works already and

{
  "articles": [1, 2, 3 ],
  "client": 43
}

can be achieved by just adding an empty handler callback on the Article and Client object,


class Article {
   /**
     * @HandlerCallback("json", direction = "deserialization")
     */
    public function dummy()
    {
        // no custom logic here
    }
}
sky003 commented 6 years ago

can be achieved by just adding an empty handler callback on the Article and Client object,

class Article {
   /**
     * @HandlerCallback("json", direction = "deserialization")
     */
    public function dummy()
    {
        // no custom logic here
    }
}

I just added an empty handler and deserialization doesn't work at all. Maybe I missed something?

goetas commented 6 years ago

I just added an empty handler and deserialization doesn't work at all. Maybe I missed something?

can you elaborate?

goetas commented 6 years ago
{
  "articles": [1, 2, 3 ],
  "client": 43
}

will not work out of the box (different hacks and tricks are necessary to make it work...) I see this can be a common use case and might be nice to have some functionaluity to make it work of the box...

I'm open for suggestions.

My first idea was:

class Site
{
    /**
     * @Serializer\Type("array<JMS\Serializer\Tests\Fixtures\Doctrine\Article>")
     * @Serializer\Options("deserialization", {"do_not_visit_result":"true"})
     */
    protected $articles;

    /**
     * @Serializer\Type("JMS\Serializer\Tests\Fixtures\Doctrine\Client")
     * @Serializer\Options("deserialization", {"do_not_visit_result":"true"})
     */
    protected $client;
}

adding some kind of "options" that can be used by visitors to do something special... (in this case to stop the deserialization process and return the current result)

to be honest I do not linke the idea of adding a generic "options" array, in almost all the cases it ends un putting it it all the trash and edge-cases I can immagine... and that will make the jms/serializer an un-maintainable project...

Suggestions?

MadeRelevant commented 5 years ago

This issue is almost a year old and I experience the same issue. As much as I like to create a PR, I don't think I am experienced enough with this library to do so in the near future. Until this is fixed it seems like I need to go with a custom handler on the child deserialized objects. Any suggestions on deserializing these specific entities because setting a custom handler destroys the purpose of automatic deserialization imo.

@goetas care to share how you solved this issue maybe? There are two cases I'm especially interested in:

goetas commented 5 years ago

@MadeRelevant Thanks for spending your time to comment on this issue.

As this in an open-source project, developed in my free time, I try to do my best to keep it running.

Some people have proposed a solution, but nobody actually tried to do propose an implementation.

At the moment this issue can be workaround-ed using a custom handler, but of course is somebody is willing to provide a better solution I will be happy to review that PR.

supersmile2009 commented 5 years ago

@DevNIX @MadeRelevant If you want to make sure that when posting data like this:

{
  "articles": [
    {
       "id": 1,
       "name": "foo"
    },
    {
       "id": 2,
       "name": "foo"
    },
    {
       "id": 3,
       "name": "test"
    }
  ],
  "client": {
    "id": 43
  }
}

Serializer just fetches relations from DB (articles and client in this case) and does not modify those entities ("foo" names from example don't override original article names), you can simply use serialization groups:

class ShoppingCart
{
    /**
     * @Serializer\Type("array<App\Entity\Article>")
     * @Serializer\Groups({"shoppingCart:write", "shoppingCart:read"})
     */
    protected $articles;

    /**
     * @Serializer\Type("array<App\Entity\Client>")
     * @Serializer\Groups({"shoppingCart:write", "shoppingCart:read"})
     */
    protected $client;
}

Make sure you don't have "shoppingCart:write" group inside those related entities and run deserialization with this group. It will fetch relations from the DB, but will not touch any properties in them since they don't have "shoppingCart:write" group.

And if you need to serialize the ShoppingCart for the response, use "shoppingCart:read" group. You can also add this group into related entities on properties which you want to serialize .

We've been using this approach for ages in our project, works perfectly. Hope this helps you guys :)

P. S. I think any additional solutions like

     * @Serializer\Options("deserialization", {"do_not_visit_result":"true"})

will be redundant in this case. Just my 2 cents :)

mangalores commented 4 years ago

encountered the same issue when looking to replace Symfony:Form with JMS solution and finding code already in the bundle. The main break point is that on scalar the Doctrine Creator returns a reference and then JMS proceeds to try to go through all properties of the scalar instead of stopping the investigation as the reference is kind of the only thing to do.

is there a solution for loading entity for a property by scalar id as in examples above: { "articles": [1, 2, 3 ], "client": 43 }

Currently I don't see how this reference creation in the doctrine object creator is ever of use.

Is the above solution with groups kind of the accepted way to do it? Testing a bit more that seems to be a viable option.

scaytrase commented 4 years ago

Using this example https://stackoverflow.com/a/36199209/1361089 you can easily tune it to filter out non-id parts of relation object if present (there is an example how to access id fieldnames). You should also apply naming strategy there for sure. But in general this should work both for { "articles": [1,2,3], "client": 43} OOB and for {"client": {"name": "whatever", "id": 42}} if tuned.

So the key is to handle own type here manually. Actually I'm reusing this solution for years now, works ok.

mangalores commented 4 years ago

Currently implementing a ParamConverter scheme where I want to use automatic resolution of Doctrine. Until now it required Symfony/Form for merging nested dependencies so this problem has been bugging me for a while as I am looking for a generic way for simplistic Rest endpoints and abusing Symfony form for Rest does not make anyone happy because of its arcane nature and being for actual form rendering, including myself.

Currently seeing ExclusionStrategy as another way to automatically cease traversing a scalar property if its type resolved to a valid Doctrine reference.

gremo commented 1 year ago

@scaytrase thank you, your solution works pretty fine! Apart from the decision of using getReference instead of find: when the entity doesn't exist in the database, getReference still returns something, and the persist would fail because of the foreign key check.

Better to stick with findthat returns null, and kicks-in the Symfony validation!