Changing the name of a node results in deletion not move

[uwej711]

I found this while trying to rename a route (from the RoutingBundle) with setName, the cause is a bit difficult to describe ...

First you need a parent with mapped children, say /cms/routes/zn, the child /cms/routes/zn/page-1 will be renamed to page-2 with setName.

The UnitOfWork correctly detects the move of /cms/routes/zn/page-1 to /cms/routes/zn/page-2. But it also creates a change for /cms/routes/zn because it adds an empty reordering in https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/UnitOfWork.php#L1422 or https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/UnitOfWork.php#L1424. the resulting executeUpdates for /cms/routes/zn triggers another computeChangeset for that node in https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/UnitOfWork.php#L2482, which then results in a deletion of /cms/routes/zn/page-1.

Only adding the reodering if the array in https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/UnitOfWork.php#L1420 has count > 0 solves that very special case for me but if your parent node has other changes it will break anyway.

Hope this is somehow clear ...

Cheers
Uwe

[dbu]

i guess we should not add the empty reordering, it makes no sense. but for when other changes do exist, we have to sort out the ordering issue then.

[uwej711]

The setup to demonstrate this seems a bit more complicated than I thought ...

[dantleech]

So this UOW is moving the node, then recalculating the changeset, and then registering a delete for the "missing" node and then deleting it because executeRemoves is after executeUpdates ?

• Could we calculate the delete based on UUID instead of node names? (only if we were to force UUIDs in a next major version).

[uwej711]

This is a bit more complicated, see the test in #661.
Because there is an event listener computeChangeset is called twice but the second computation messes up the child renaming. In #661 I restricted this changeset computation to fields only bit that leaves us with a failing test that want's to revert a child reorder in the preUpdate event.

[uwej711]

Just bumped into that again. I think I will test my change in #661 in production now ...

[dbu]

if you find a way to get the build green on #661 and have an improvement that has no regressions, i am happy to merge that pull request. even if it does not fix all problems around the topic - re-reading it i feel like we got lost in the realm of possible fixes and improvements.

[uwej711]

As already mentioned in #661 there remains an issue with undoing an reordering of children in preUpdate which keeps the tests failing. If I find the time I will investigate that again.