BUGFIX: Copy node respect tethered nodes correctly #5350

In case a leaf node that is tethered is copied, we un-tether it.
A migration `flow migrateevents:migratecopytetherednode` fixes this for previous cases.

In case a tethered node is attempted to be copied which has tethered childnodes determined by the parent nodetype we fail. This is not possible. What we dont do yet is determine this correctly and there are false positives:

> we assume here that the child node is tethered because the grandparent specifies that.
> this is not always fully correct and we could loosen the constraint by checking the node type schema

to correctly determine this, we have to evaluate the node type schema which is not available currently.

Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/NodeCopying/CopyNode_ConstraintChecks_TetheredNodes.feature

@@ -0,0 +1,51 @@
+@contentrepository @adapters=DoctrineDBAL
+Feature: Copy nodes (without dimensions)
+
+  Background:
+    Given using no content dimensions
+    And using the following node types:
+    """yaml
+    'Neos.ContentRepository.Testing:Tethered': []
+    'Neos.ContentRepository.Testing:TetheredDocument':
+      childNodes:
+        tethered:
+          type: 'Neos.ContentRepository.Testing:Tethered'
+    'Neos.ContentRepository.Testing:Document':
+      childNodes:
+        tethered-document:
+          type: 'Neos.ContentRepository.Testing:TetheredDocument'
+    'Neos.ContentRepository.Testing:DocumentWithoutTetheredChildren': []
+    """
+    And using identifier "default", I define a content repository
+    And I am in content repository "default"
+    And I am user identified by "initiating-user-identifier"
+    And the command CreateRootWorkspace is executed with payload:
+      | Key                | Value           |
+      | workspaceName      | "live"          |
+      | newContentStreamId | "cs-identifier" |
+    And I am in workspace "live" and dimension space point {}
+    And the command CreateRootNodeAggregateWithNode is executed with payload:
+      | Key             | Value                         |
+      | nodeAggregateId | "lady-eleonode-rootford"      |
+      | nodeTypeName    | "Neos.ContentRepository:Root" |
+    And the following CreateNodeAggregateWithNode commands are executed:
+      | nodeAggregateId  | parentNodeAggregateId  | nodeTypeName                                                   | tetheredDescendantNodeAggregateIds                                                            |
+      | node-mc-nodeface | lady-eleonode-rootford | Neos.ContentRepository.Testing:DocumentWithoutTetheredChildren | {}                                                                                            |
+      | node-wan-kenody  | lady-eleonode-rootford | Neos.ContentRepository.Testing:Document                        | {"tethered-document": "nodewyn-tetherton", "tethered-document/tethered": "nodimer-tetherton"} |
+
+  Scenario: Coping fails if the leaf of a nested tethered node is attempted to be copied
+    And I expect the node aggregate "nodewyn-tetherton" to exist
+    And I expect this node aggregate to be classified as "tethered"
+
+    And I expect the node aggregate "nodimer-tetherton" to exist
+    And I expect this node aggregate to be classified as "tethered"
+
+    When the command CopyNodesRecursively is executed with payload and exceptions are caught:
+      | Key                         | Value                                                                                          |
+      | sourceDimensionSpacePoint   | {}                                                                                             |
+      | sourceNodeAggregateId       | "nodewyn-tetherton"                                                                            |
+      | targetDimensionSpacePoint   | {}                                                                                             |
+      | targetParentNodeAggregateId | "node-mc-nodeface"                                                                              |
+      | nodeAggregateIdMapping      | {"nodewyn-tetherton": "nodewyn-tetherton-copy", "nodimer-tetherton": "nodimer-tetherton-copy"} |
+
+    Then the last command should have thrown an exception of type "TetheredNodesCannotBePartiallyCopied"

Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/NodeCopying/CopyNode_TetheredNodes.feature

@@ -24,10 +24,10 @@ Feature: Copy nodes with tethered nodes
       | nodeTypeName    | "Neos.ContentRepository:Root" |
 
     When the following CreateNodeAggregateWithNode commands are executed:
-      | nodeAggregateId            | parentNodeAggregateId  | nodeTypeName                                        | tetheredDescendantNodeAggregateIds |
-      | sir-david-nodenborough     | lady-eleonode-rootford | Neos.ContentRepository.Testing:Document             | {}                                 |
-      | nody-mc-nodeface           | sir-david-nodenborough | Neos.ContentRepository.Testing:Document             |                                    |
-      | sir-nodeward-nodington-iii | lady-eleonode-rootford | Neos.ContentRepository.Testing:DocumentWithTethered | {"tethered": "nodewyn-tetherton"}  |
+      | nodeAggregateId          | parentNodeAggregateId  | nodeTypeName                                        | tetheredDescendantNodeAggregateIds |
+      | sir-david-nodenborough   | lady-eleonode-rootford | Neos.ContentRepository.Testing:Document             | {}                                 |
+      | nody-mc-nodeface         | sir-david-nodenborough | Neos.ContentRepository.Testing:Document             |                                    |
+      | sir-nodeward-nodington-i | lady-eleonode-rootford | Neos.ContentRepository.Testing:DocumentWithTethered | {"tethered": "nodewyn-tetherton"}  |
 
   Scenario: Coping a tethered node turns it into a regular node
     And I expect the node aggregate "nodewyn-tetherton" to exist
@@ -50,3 +50,33 @@ Feature: Copy nodes with tethered nodes
     And I expect this node aggregate to disable dimension space points []
     And I expect this node aggregate to have no child node aggregates
     And I expect this node aggregate to have the parent node aggregates ["sir-david-nodenborough"]
+
+  Scenario: Coping a node with tethered node keeps the child node tethered
+    And I expect the node aggregate "nodewyn-tetherton" to exist
+    And I expect this node aggregate to be classified as "tethered"
+
+    When the command CopyNodesRecursively is executed with payload:
+      | Key                         | Value                                                                                                    |
+      | sourceDimensionSpacePoint   | {}                                                                                                       |
+      | sourceNodeAggregateId       | "sir-nodeward-nodington-i"                                                                               |
+      | targetDimensionSpacePoint   | {}                                                                                                       |
+      | targetParentNodeAggregateId | "sir-david-nodenborough"                                                                                 |
+      | nodeAggregateIdMapping      | {"sir-nodeward-nodington-i": "sir-nodeward-nodington-ii", "nodewyn-tetherton": "nodewyn-tetherton-copy"} |
+
+    And I expect the node aggregate "sir-nodeward-nodington-ii" to exist
+    And I expect this node aggregate to be classified as "regular"
+    And I expect this node aggregate to be unnamed
+    And I expect this node aggregate to be of type "Neos.ContentRepository.Testing:DocumentWithTethered"
+    And I expect this node aggregate to occupy dimension space points [[]]
+    And I expect this node aggregate to disable dimension space points []
+    And I expect this node aggregate to have the child node aggregates ["nodewyn-tetherton-copy"]
+    And I expect this node aggregate to have the parent node aggregates ["sir-david-nodenborough"]
+
+    And I expect the node aggregate "nodewyn-tetherton-copy" to exist
+    And I expect this node aggregate to be classified as "tethered"
+    And I expect this node aggregate to be named "tethered"
+    And I expect this node aggregate to be of type "Neos.ContentRepository.Testing:Document"
+    And I expect this node aggregate to occupy dimension space points [[]]
+    And I expect this node aggregate to disable dimension space points []
+    And I expect this node aggregate to have no child node aggregates
+    And I expect this node aggregate to have the parent node aggregates ["sir-nodeward-nodington-ii"]

Neos.ContentRepository.Core/Classes/Feature/NodeDuplication/Dto/NodeSubtreeSnapshot.php

@@ -14,6 +14,7 @@
 use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindReferencesFilter;
 use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
 use Neos\ContentRepository\Core\Projection\ContentGraph\References;
+use Neos\ContentRepository\Core\SharedModel\Exception\TetheredNodesCannotBePartiallyCopied;
 use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateClassification;
 use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
 use Neos\ContentRepository\Core\SharedModel\Node\NodeName;
@@ -49,24 +50,34 @@ private function __construct(
 
     public static function fromSubgraphAndStartNode(ContentSubgraphInterface $subgraph, Node $sourceNode): self
     {
-        $childNodes = [];
+        return self::createSnapshotRecursively($subgraph, $sourceNode, true);
+    }
+
+    private static function createSnapshotRecursively(ContentSubgraphInterface $subgraph, Node $sourceNode, bool $firstLevel): self
+    {
+        $childNodeSnapshots = [];
         foreach (
             $subgraph->findChildNodes($sourceNode->aggregateId, FindChildNodesFilter::create()) as $sourceChildNode
         ) {
-            $childNodes[] = self::fromSubgraphAndStartNode($subgraph, $sourceChildNode);
+            if ($firstLevel && $sourceNode->classification->isTethered() && $sourceChildNode->classification->isTethered()) {
+                // we assume here that the child node is tethered because the grandparent specifies that.
+                // this is not always fully correct and we could loosen the constraint by checking the node type schema
+                throw new TetheredNodesCannotBePartiallyCopied(sprintf('Cannot copy tethered node %s because child node %s is also tethered. Only standalone tethered nodes can be copied.', $sourceNode->aggregateId->value, $sourceChildNode->aggregateId->value), 1731264887);
+            }
+
+            $childNodeSnapshots[] = self::createSnapshotRecursively($subgraph, $sourceChildNode, false);
         }
-        $properties = $sourceNode->properties;
 
         return new self(
             $sourceNode->aggregateId,
             $sourceNode->nodeTypeName,
             $sourceNode->name,
-            $sourceNode->classification,
-            $properties->serialized(),
+            $firstLevel ? NodeAggregateClassification::CLASSIFICATION_REGULAR : $sourceNode->classification,
+            $sourceNode->properties->serialized(),
             self::serializeProjectedReferences(
                 $subgraph->findReferences($sourceNode->aggregateId, FindReferencesFilter::create())
             ),
-            $childNodes
+            $childNodeSnapshots
         );
     }
 

Neos.ContentRepository.Core/Classes/SharedModel/Exception/TetheredNodesCannotBePartiallyCopied.php

@@ -0,0 +1,26 @@
+<?php
+
+/*
+ * This file is part of the Neos.ContentRepository package.
+ *
+ * (c) Contributors of the Neos Project - www.neos.io
+ *
+ * This package is Open Source Software. For the full copyright and license
+ * information, please view the LICENSE file which was distributed with this
+ * source code.
+ */
+
+declare(strict_types=1);
+
+namespace Neos\ContentRepository\Core\SharedModel\Exception;
+
+/**
+ * The exception to be thrown if a tethered node aggregate with further whether nodes, determined by their ancestor is attempted to be copied.
+ *
+ * Only leaf tethered nodes can be copied.
+ *
+ * @api because exception is thrown during invariant checks on command execution
+ */
+final class TetheredNodesCannotBePartiallyCopied extends \DomainException
+{
+}

Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/Features/NodeCopying.php

@@ -80,4 +80,18 @@ public function theCommandCopyNodesRecursivelyIsExecutedWithPayload(TableNode $p
 
         $this->currentContentRepository->handle($command);
     }
+
+    /**
+     * @Given /^the command CopyNodesRecursively is executed with payload and exceptions are caught:$/
+     * @param TableNode $payloadTable
+     * @throws \Exception
+     */
+    public function theCommandCopyNodesRecursivelyIsExecutedWithPayloadAndExceptionsAreCaught(TableNode $payloadTable)
+    {
+        try {
+            $this->theCommandCopyNodesRecursivelyIsExecutedWithPayload($payloadTable);
+        } catch (\Exception $exception) {
+            $this->lastCommandException = $exception;
+        }
+    }
 }

Neos.ContentRepositoryRegistry/Classes/Command/MigrateEventsCommandController.php

@@ -116,4 +116,20 @@ public function migrateSetReferencesToMultiNameFormatCommand(string $contentRepo
         $eventMigrationService = $this->contentRepositoryRegistry->buildService($contentRepositoryId, $this->eventMigrationServiceFactory);
         $eventMigrationService->migrateReferencesToMultiFormat($this->outputLine(...));
     }
+
+    /**
+     * Migrates "nodeAggregateClassification":"tethered" to "regular", in case for copied tethered nodes.
+     *
+     * Needed for #5350: https://github.com/neos/neos-development-collection/issues/5350
+     *
+     * Included in November 2024 - before final Neos 9.0 release
+     *
+     * @param string $contentRepository Identifier of the Content Repository to migrate
+     */
+    public function migrateCopyTetheredNodeCommand(string $contentRepository = 'default'): void
+    {
+        $contentRepositoryId = ContentRepositoryId::fromString($contentRepository);
+        $eventMigrationService = $this->contentRepositoryRegistry->buildService($contentRepositoryId, $this->eventMigrationServiceFactory);
+        $eventMigrationService->migrateCopyTetheredNode($this->outputLine(...));
+    }
 }

Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php

@@ -15,14 +15,14 @@
 use Neos\ContentRepository\Core\Feature\NodeModification\Command\SetSerializedNodeProperties;
 use Neos\ContentRepository\Core\Feature\NodeMove\Command\MoveNodeAggregate;
 use Neos\ContentRepository\Core\Feature\NodeReferencing\Command\SetSerializedNodeReferences;
-use Neos\ContentRepository\Core\Feature\NodeReferencing\Event\NodeReferencesWereSet;
 use Neos\ContentRepository\Core\Feature\NodeRemoval\Command\RemoveNodeAggregate;
 use Neos\ContentRepository\Core\Feature\NodeRenaming\Command\ChangeNodeAggregateName;
 use Neos\ContentRepository\Core\Feature\NodeTypeChange\Command\ChangeNodeAggregateType;
 use Neos\ContentRepository\Core\Feature\NodeVariation\Command\CreateNodeVariant;
 use Neos\ContentRepository\Core\Feature\RootNodeCreation\Command\CreateRootNodeAggregateWithNode;
 use Neos\ContentRepository\Core\Feature\RootNodeCreation\Command\UpdateRootNodeAggregateDimensions;
 use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId;
+use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateClassification;
 use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;
 use Neos\ContentRepositoryRegistry\Command\MigrateEventsCommandController;
 use Neos\ContentRepositoryRegistry\Factory\EventStore\DoctrineEventStoreFactory;
@@ -65,6 +65,58 @@ public function backup(\Closure $outputFn): void
         $outputFn(sprintf('Backup. Copied events table to %s', $backupEventTableName));
     }
 
+    public function migrateCopyTetheredNode(\Closure $outputFn): void
+    {
+        $this->eventsModified = [];
+        $warnings = 0;
+
+        $backupEventTableName = DoctrineEventStoreFactory::databaseTableName($this->contentRepositoryId)
+            . '_bkp_' . date('Y_m_d_H_i_s');
+        $outputFn(sprintf('Backup: copying events table to %s', $backupEventTableName));
+
+        $this->copyEventTable($backupEventTableName);
+
+        $streamName = VirtualStreamName::all();
+        $eventStream = $this->eventStore->load($streamName, EventStreamFilter::create(EventTypes::create(EventType::fromString('NodeAggregateWithNodeWasCreated'))));
+        foreach ($eventStream as $eventEnvelope) {
+            $outputRewriteNotice = fn(string $message) => $outputFn(sprintf('%s@%s %s', $eventEnvelope->sequenceNumber->value, $eventEnvelope->event->type->value, $message));
+            if ($eventEnvelope->event->type->value !== 'NodeAggregateWithNodeWasCreated') {
+                throw new \RuntimeException(sprintf('Unhandled event: %s', $eventEnvelope->event->type->value));
+            }
+
+            $eventMetaData = $eventEnvelope->event->metadata?->value;
+            // a copy is basically a NodeAggregateWithNodeWasCreated with CopyNodesRecursively command, so we skip others:
+            if (!$eventMetaData || ($eventMetaData['commandClass'] ?? null) !== CopyNodesRecursively::class) {
+                continue;
+            }
+
+            $eventData = self::decodeEventPayload($eventEnvelope);
+            if ($eventData['nodeAggregateClassification'] !== NodeAggregateClassification::CLASSIFICATION_TETHERED->value) {
+                // this copy is okay
+                continue;
+            }
+
+            $eventData['nodeAggregateClassification'] = NodeAggregateClassification::CLASSIFICATION_REGULAR->value;
+            $this->updateEventPayload($eventEnvelope->sequenceNumber, $eventData);
+
+            $eventMetaData['commandPayload']['nodeTreeToInsert']['nodeAggregateClassification'] = NodeAggregateClassification::CLASSIFICATION_REGULAR->value;
+
+            $this->updateEventMetaData($eventEnvelope->sequenceNumber, $eventMetaData);
+            $outputRewriteNotice(sprintf('Copied tethered node "%s" of type "%s" (name: %s) was migrated', $eventData['nodeAggregateId'], $eventData['nodeTypeName'], json_encode($eventData['nodeName'])));
+        }
+
+        if (!count($this->eventsModified)) {
+            $outputFn('Migration was not necessary.');
+            return;
+        }
+
+        $outputFn();
+        $outputFn(sprintf('Migration applied to %s events. Please replay the projections `./flow cr:projectionReplayAll`', count($this->eventsModified)));
+        if ($warnings) {
+            $outputFn(sprintf('WARNING: Finished but %d warnings emitted.', $warnings));
+        }
+    }
+
     /**
      * The following things have to be migrated:
      *