BUG: Node move can lead to wrong result with orphans

[kitsunet]

Expected Behavior

The tested scenario is the following:

• Home
  • Page 1
    • Subpage 1
    • Subpage 2
  • Page 2

Now we first move one of the subpages (example "Subpage 1") into "Page 2" then without publishing move "Page 1" into "Page 2" as well. Then "publish all", with the expected outcome:

• Home
  • Page 2
    • Subpage 1
    • Page 1
      • Subpage 2

Current Behavior

See expected behavior for scenario and expected outcome.
Unfortunately this outcome is not guaranteed and an alternative outcome is:

• Home
  • Page 2
    • Page 1
      • Subpage 1
      • Subpage 2

If this wrong outcome happens the child (collection) nodes will be orphaned and actually appear in the DB at the expect place (home -> page 2 -> Subpage 1 -> "childNodeName"), rendering the page unusable because it no longer has content collections.

This only happens when publishing to live due to the conditional code path we folllow for this.

This second outcome is triggered based on the sorting order of the unpublished nodes determined in:

\Neos\ContentRepository\Domain\Service\PublishingService::getUnpublishedNodes

In the wrong case you will see first the moved child node ("Subpage 1" + child nodes) in this list and then the (old) parent "Page 2".

We publish nodes one by one in this process and will (skipping some steps) go through the following code path:

\Neos\ContentRepository\Domain\Model\Workspace::publishNode
\Neos\ContentRepository\Domain\Model\Workspace::replaceNodeData
As we moved Subpage 1 and Page 2 both get to:
\Neos\ContentRepository\Domain\Model\Workspace::moveTargetNodeDataToNewPosition

In there, if we publish to live, instead of using NodeData::move we instead call NodeData::setPath (with the implicit recursive flag true).

Now if the order of nodes is Subpage 1 and then Page 2 we will first move Subpage 1 from it's Live position to it's correctly intended new Position (below Page 1), but as we do not persist in between we will then publish Page 2 and call setPath on Page 2 in live recursively which still includes Subpage 1 (according to the database), consequently moving it again from it's intended position into it's wrong position as a child of Page 2.
If the order is reversed, the result is as intended as we first move subpage 1 together with Page 2 then later in the process move it again to it's intended position resulting in the correct tree position.
The reason why, in the wrong result, only the page and not it's child nodes are moved again, is because the recursive setPath call will call NodeData::getChildNodeData for the next level using the in memory path property of the NodeData object to query. Therefore from "Page 2" asking the database for everything in the target workspace on the next level in the old path of "Page 2" will yield "Subpage 1", but as "Subpage 1" is changed in the unit of work the actual path will be different so at that point any child nodes of "Subpage 1" (this should extend to documents too) is not changed again as the path concatenation will result in the correct new path again.

Removing the special case for live publishing and always going through NodeData::move seems to fix the problem and doesn't seem to leave unwanted orphans behind.

This problem should also trigger in cases the new parents for both the subpage and it's parent are totally different.

Environment

- Flow:
- Neos: 8.3
- PHP:

Anything else?

No response