doctrine / DoctrineModule

Doctrine Module for Laminas
http://www.doctrine-project.org/projects/doctrine-module.html
MIT License
398 stars 269 forks source link

Batch insert parent/children regenerate id of each child #708

Closed mdjaman closed 2 years ago

mdjaman commented 4 years ago

Hello maybe i am totally wrong but persisting master and details (OneToMany) resets given id of children. The parent Id is correct but all his ReferenceMany object got new one

update.json [ { "created_at": "2019-02-14T09:57:56-0500", "updated_at": "2020-05-20T11:40:27-0400", "id": "5c6581f45771db0320faf769", "name": "Fiche A", "code": "FNC", "alias": "fiche-a", "description": "Fiche de notification a", "disease": { "id": "5b06a66d5771dba3c7a6a598" }, "template": false, "complete": true, "useAsDefault": false, "items": [ { "created_at": "2019-02-14T09:57:56-0500", "updated_at": "2020-05-27T13:40:27-0400", "id": "5c6581f45771db0320faf76b", "label": "Identifiant Unique", "name": "EPID", "alias": "epid_1", "type": "", "required": false, "readonly": false, "values": [], "position": 1, "tag": "epid", "children": [ { "created_at": "2019-02-14T09:57:56-0500", "updated_at": "2020-05-21T15:05:23-0400", "id": "5c6581f45771db0320faf777", "label": "Code échantillon", "name": "code", "alias": "code_1", "type": "text", "required": false, "readonly": false, "values": [], "position": 1, "children": [], "masks": [], "validators": [] } ], "meta": { "locked": true, "cascadeOperation": false }, "masks": [], "validators": [] } ], "dataTable": { "created_at": "2020-05-27T13:56:38-0400", "id": "5ecea9d6a47e486913436400", "name": "Tableau de données A", "alias": "tableau-de-donnees-a", "columns": [] }, "meta": { "finalResultField": "finalresult" } }]

NotificationFormItem.php (Details/Child)

class NotificationFormItem
{

    /**
     * @var string
     * @ODM\Id
     */
    protected $id;

    /**
     * @var string
     * @ODM\Field(type="string")
     */
    protected $label;

    /**
     * @var string
     * @ODM\Field(type="string")
     */
    protected $name;

    /**
     * @var string
     * @ODM\Field(type="string")
     */
    protected $alias;

    /**
     * @var string
     * @ODM\Field(type="string")
     */
    protected $description;

  /**
     * @var array
     * @ODM\Field(type="collection")
     */
    protected $values = [];

    /**
     * @var NotificationFormInterface
     * @ODM\ReferenceOne(targetDocument="NotificationForm", inversedBy="items", storeAs="id")
     */
    protected $notificationForm;

    /**
     * @var NotificationFormItemInterface
     * @ODM\ReferenceOne(
     *     targetDocument="NotificationFormItem",
     *     storeAs="id",
     *     inversedBy="children",
     *     cascade={"persist", "merge"}
     * )
     */
    protected $parent;

    /**
     * @var Collection|array
     * @ODM\ReferenceMany(
     *     targetDocument="NotificationFormItem",
     *     mappedBy="parent",
     *     sort={"position": "asc"},
     *     cascade={"all"},
     *     orphanRemoval=true
     * )
     */
    protected $children;

    /**
     * NotificationFormItem constructor.
     */
    public function __construct()
    {
        $this->children = new ArrayCollection();
    }

// skipping getters/setters

NotificationForm.php (Master/Parent)

class NotificationForm extends BaseDocument
{
    use BlameableDocument;

    /**
     * @var string
     * @ODM\Id
     */
    protected $id;

    /**
     * @var string
     * @ODM\Field(type="string")
     */
    protected $name;

    /**
     * @var string
     * @ODM\Field(type="string")
    */
    protected $code;

    /**
     * @var string
     * @ODM\Field(type="string")
     */
    protected $alias;

    /**
     * @var string
     * @ODM\Field(type="string")
     */
    protected $description;

    /**
     * @var Collection|array
     * @ODM\ReferenceMany(
     *     targetDocument="NotificationFormItem",
     *     mappedBy="notificationForm",
     *     storeAs="id",
     *     sort={"position": "asc"},
     *     cascade={"all"}
     * )
     */
    protected $items;

    /**
     * NotificationForm constructor.
     */
    public function __construct()
    {
        $this->items = new ArrayCollection();
    }
mdjaman commented 4 years ago

Maybe an issue with the hydrator

malarzm commented 4 years ago

@mdjaman could you please prepare a failing test case and send it as a PR? That will greatly help us identifying your issue. Here you will find an example of such tests: https://github.com/doctrine/mongodb-odm/blob/master/tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/GH2157Test.php

mdjaman commented 4 years ago

Basically

use DoctrineModule\Stdlib\Hydrator\DoctrineObject as Hydrator;

$record = json_decode($jsonVar);
$hydrator = new Hydrator($objectManager);
$entity = new NotificationForm();
$document = $hydrator->hydrate($record, $entity);

//Playing with new object all children have a null id
var_dump($document->getItems()[0]->getChildren()[0]);
$objectManager->persist($document);
// persisting them and new ids are generated
var_dump($document->getItems()[0]->getChildren()[0]);

Edited for formatting (@alcaeus)

malarzm commented 3 years ago
//Playing with new object all children have a null id
var_dump($document->getItems()[0]->getChildren()[0]);
$objectManager->persist($document);
// persisting them and new ids are generated
var_dump($document->getItems()[0]->getChildren()[0]);

This is expected behaviour - you are using autogenerated ids and ODM is creating them (if they don't exist) during initial persist.

mdjaman commented 3 years ago

They exist

[
  {
    "created_at": "2019-02-14T09:57:56-0500",
    "updated_at": "2020-05-20T11:40:27-0400",
    "id": "5c6581f45771db0320faf769",
    "name": "Fiche A",
    "code": "FNC",
    "alias": "fiche-a",
    "description": "Fiche de notification a",
    "disease": { "id": "5b06a66d5771dba3c7a6a598" },
    "template": false,
    "complete": true,
    "useAsDefault": false,
    "items": [
      {
        "created_at": "2019-02-14T09:57:56-0500",
        "updated_at": "2020-05-27T13:40:27-0400",
        "id": "5c6581f45771db0320faf76b",
        "label": "Identifiant Unique",
        "name": "EPID",
        "alias": "epid_1",
        "type": "",
        "required": false,
        "readonly": false,
        "values": [],
        "position": 1,
        "tag": "epid",
        "children": [
          {
            "created_at": "2019-02-14T09:57:56-0500",
            "updated_at": "2020-05-21T15:05:23-0400",
            "id": "5c6581f45771db0320faf777",
            "label": "Code échantillon",
            "name": "code",
            "alias": "code_1",
            "type": "text",
            "required": false,
            "readonly": false,
            "values": [],
            "position": 1,
            "children": [],
            "masks": [],
            "validators": []
          }
        ],
        "meta": { "locked": true, "cascadeOperation": false },
        "masks": [],
        "validators": []
      }
    ],
    "dataTable": {
      "created_at": "2020-05-27T13:56:38-0400",
      "id": "5ecea9d6a47e486913436400",
      "name": "Tableau de données A",
      "alias": "tableau-de-donnees-a",
      "columns": []
    },
    "meta": { "finalResultField": "finalresult" }
  }
]
mdjaman commented 3 years ago

But the hydrator removed them while processing, i don't know why. That happens only in one to many relationship

malarzm commented 3 years ago

Ah so they do exist in JSON but are lost when using hydrator. I'm afraid I need to move your issue to the DoctrineModule as this seems to be related with how the hydrator itself is working and not related to ODM.

TomHAnderson commented 3 years ago

If I understand this correctly you are using auto generated ids but you are supplying the ids and the problem is your supplied ids are being replaced with auto generated ids. Have I got that right?

If I am right then why don't you change the ids to not be auto generated and see if that fixes it?

mdjaman commented 3 years ago

Can u tell me if I keep the auto generated why I can't set the Id. I think this issue comes from the generated hydrator

mdjaman commented 3 years ago

I suspect that doctrine hydrator doesn't correctly handle the onetomany relationship since it ignored the given id

malarzm commented 3 years ago

FWIW ODM-wise it's perfectly fine to have an id before persisting - that will cause an upsert operation

TomHAnderson commented 3 years ago

So, my question still stands. If you turn off auto generated ids does it work?

mdjaman commented 3 years ago

Same result seems that items in array collection lost their ids

TomHAnderson commented 3 years ago
doctrine/doctrine-module/src (master)]$ grep -rli hydrator *

I do not think the problem is coming from this repository because this repository does not have a hydrator. Please check your code and find out what hydrator object is doing your hydration then we can go from there.

I know you came from the odm module so I'm sorry if this feels like the run-around. Let's figure out what file exactly is causing your problem.

mdjaman commented 3 years ago

Sorry i clicked the wrong button 😅. Is this class DoctrineModule\Stdlib\Hydrator\DoctrineObject responsible of creating hydrated object ? 🤔

TomHAnderson commented 3 years ago

Yes, you're right. Thank you for helping me with that.

I'll look through the hydrator soon. We're going to need a unit test for this sooner than later.

TomHAnderson commented 3 years ago

@mdjaman there is a newer hydrator than the one supplied with this module. https://github.com/doctrine/doctrine-laminas-hydrator

If I'm going to investigate a hydrator I'd rather investigate that one. You may have the same error with this new hydrator because it was based on this one. But just to cover all the bases will you install that repo and test with that new hydrator?

mdjaman commented 3 years ago

When i tried to hydrate the object from the same database everything went well but when i switched to another database i got a different id. I really don't know what happening here 😑

TomHAnderson commented 3 years ago

Well, because of the lack of others with the same issue your issue sounds unique. Which often points to a user error. So, in order to be able to help you with this, I'm going to need a unit test from you which duplicates the problem.

Your success and failures also point to the same conclusion. Please author a unit test or even a whole application with which I can duplicate your issue locally.