Merge pull request #4975 from neos/4742-documentUriPathProjectionAdju…

…stments BUGFIX: 4742 - document uri path projection adjustments
neos · Apr 25, 2024 · 9e4ba1e · 9e4ba1e
2 parents 7593180 + 3ee4a29
commit 9e4ba1e
Show file tree

Hide file tree

Showing 5 changed files with 446 additions and 20 deletions.
diff --git a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentNodeInfo.php b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentNodeInfo.php
@@ -71,6 +71,15 @@ public function withOriginDimensionSpacePoint(OriginDimensionSpacePoint $originD
         return new self($source);
     }
 
+    public function withoutSiblings(): self
+    {
+        $source = $this->source;
+        $source['precedingnodeaggregateid'] = null;
+        $source['succeedingnodeaggregateid'] = null;
+
+        return new self($source);
+    }
+
     public function getNodeAggregateId(): NodeAggregateId
     {
         return NodeAggregateId::fromString($this->source['nodeaggregateid']);

diff --git a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathFinder.php b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathFinder.php
@@ -195,6 +195,28 @@ public function getLastChildNode(
         );
     }
 
+    /**
+     * @throws NodeNotFoundException
+     * @internal
+     */
+    public function getLastChildNodeNotBeing(
+        NodeAggregateId $parentNodeAggregateId,
+        string $dimensionSpacePointHash,
+        NodeAggregateId $excludedNodeAggregateId
+    ): DocumentNodeInfo {
+        return $this->fetchSingle(
+            'dimensionSpacePointHash = :dimensionSpacePointHash
+                AND parentNodeAggregateId = :parentNodeAggregateId
+                AND nodeAggregateId != :excludedNodeAggregateId
+                AND succeedingNodeAggregateId IS NULL',
+            [
+                'dimensionSpacePointHash' => $dimensionSpacePointHash,
+                'parentNodeAggregateId' => $parentNodeAggregateId->value,
+                'excludedNodeAggregateId' => $excludedNodeAggregateId->value
+            ]
+        );
+    }
+
     /**
      * @api
      */

diff --git a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathProjection.php b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathProjection.php
@@ -8,9 +8,9 @@
 use Doctrine\DBAL\DBALException;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Types\Types;
-use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePointSet;
 use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
 use Neos\ContentRepository\Core\EventStore\EventInterface;
+use Neos\ContentRepository\Core\Feature\Common\InterdimensionalSiblings;
 use Neos\ContentRepository\Core\Feature\DimensionSpaceAdjustment\Event\DimensionShineThroughWasAdded;
 use Neos\ContentRepository\Core\Feature\DimensionSpaceAdjustment\Event\DimensionSpacePointWasMoved;
 use Neos\ContentRepository\Core\Feature\NodeCreation\Event\NodeAggregateWithNodeWasCreated;
@@ -394,7 +394,7 @@ private function whenNodePeerVariantWasCreated(NodePeerVariantWasCreated $event)
             $event->nodeAggregateId,
             $event->sourceOrigin,
             $event->peerOrigin,
-            $event->peerSucceedingSiblings->toDimensionSpacePointSet()
+            $event->peerSucceedingSiblings
         );
     }
 
@@ -407,7 +407,7 @@ private function whenNodeGeneralizationVariantWasCreated(NodeGeneralizationVaria
             $event->nodeAggregateId,
             $event->sourceOrigin,
             $event->generalizationOrigin,
-            $event->variantSucceedingSiblings->toDimensionSpacePointSet()
+            $event->variantSucceedingSiblings
         );
     }
 
@@ -420,15 +420,15 @@ private function whenNodeSpecializationVariantWasCreated(NodeSpecializationVaria
             $event->nodeAggregateId,
             $event->sourceOrigin,
             $event->specializationOrigin,
-            $event->specializationSiblings->toDimensionSpacePointSet()
+            $event->specializationSiblings
         );
     }
 
     private function copyVariants(
         NodeAggregateId $nodeAggregateId,
         OriginDimensionSpacePoint $sourceOrigin,
         OriginDimensionSpacePoint $targetOrigin,
-        DimensionSpacePointSet $coveredSpacePoints
+        InterdimensionalSiblings $interdimensionalSiblings,
     ): void {
         $sourceNode = $this->tryGetNode(fn () => $this->getState()->getByIdAndDimensionSpacePointHash(
             $nodeAggregateId,
@@ -438,17 +438,18 @@ private function copyVariants(
             // Probably not a document node
             return;
         }
-        foreach ($coveredSpacePoints as $coveredSpacePoint) {
+        foreach ($interdimensionalSiblings as $interdimensionalSibling) {
             // Especially when importing a site it can happen that variants are created in a "non-deterministic" order,
             // so we need to first make sure a target variant doesn't exist:
-            $this->deleteNodeByIdAndDimensionSpacePointHash($nodeAggregateId, $coveredSpacePoint->hash);
+            $this->deleteNodeByIdAndDimensionSpacePointHash($nodeAggregateId, $interdimensionalSibling->dimensionSpacePoint->hash);
 
-            $this->insertNode(
-                $sourceNode
-                ->withDimensionSpacePoint($coveredSpacePoint)
+            $targetNode = $sourceNode
+                ->withDimensionSpacePoint($interdimensionalSibling->dimensionSpacePoint)
                 ->withOriginDimensionSpacePoint($targetOrigin)
-                ->toArray()
-            );
+                ->withoutSiblings();
+
+            $this->insertNode($targetNode->toArray());
+            $this->connectNodeWithSiblings($targetNode, $targetNode->getParentNodeAggregateId(), $interdimensionalSibling->nodeAggregateId);
         }
     }
 
@@ -911,7 +912,7 @@ private function disconnectNodeFromSiblings(DocumentNodeInfo $node): void
     private function connectNodeWithSiblings(
         DocumentNodeInfo $node,
         NodeAggregateId $parentNodeAggregateId,
-        ?NodeAggregateId $newSucceedingNodeAggregateId
+        ?NodeAggregateId $newSucceedingNodeAggregateId,
     ): void {
         if ($newSucceedingNodeAggregateId !== null) {
             $newPrecedingNode = $this->tryGetNode(fn () => $this->getState()->getPrecedingNode(
@@ -927,24 +928,34 @@ private function connectNodeWithSiblings(
                 ['precedingNodeAggregateId' => $node->getNodeAggregateId()->value]
             );
         } else {
-            $newPrecedingNode = $this->tryGetNode(fn () => $this->getState()->getLastChildNode(
+            $newPrecedingNode = $this->tryGetNode(fn () => $this->getState()->getLastChildNodeNotBeing(
                 $parentNodeAggregateId,
-                $node->getDimensionSpacePointHash()
+                $node->getDimensionSpacePointHash(),
+                $node->getNodeAggregateId()
             ));
         }
-        if ($newPrecedingNode !== null) {
+        if (
+            $newPrecedingNode !== null
+            && !$newPrecedingNode->getNodeAggregateId()->equals($node->getNodeAggregateId())
+        ) {
             $this->updateNode(
                 $newPrecedingNode,
                 ['succeedingNodeAggregateId' => $node->getNodeAggregateId()->value]
             );
         }
 
-        // update node itself
-        $this->updateNode($node, [
+        $updatedNodeData = [
             'parentNodeAggregateId' => $parentNodeAggregateId->value,
-            'precedingNodeAggregateId' => $newPrecedingNode?->getNodeAggregateId()->value,
             'succeedingNodeAggregateId' => $newSucceedingNodeAggregateId?->value,
-        ]);
+        ];
+        if (
+            !$newPrecedingNode?->getNodeAggregateId()->equals($node->getNodeAggregateId())
+        ) {
+            $updatedNodeData['precedingNodeAggregateId'] = $newPrecedingNode?->getNodeAggregateId()->value;
+        }
+
+        // update node itself
+        $this->updateNode($node, $updatedNodeData);
     }
 
 

diff --git a/Neos.Neos/Tests/Behavior/Features/FrontendRouting/NodeCreationEdgeCases.feature b/Neos.Neos/Tests/Behavior/Features/FrontendRouting/NodeCreationEdgeCases.feature
@@ -0,0 +1,84 @@
+@flowEntities @contentrepository
+Feature: Test cases for node creation edge cases
+
+  Scenario: Delete the succeeding sibling node in a virtual specialization and then create the node
+    Given using the following content dimensions:
+      | Identifier | Values                 | Generalizations            |
+      | example    | source, spec, leafSpec | leafSpec -> spec -> source |
+    And using the following node types:
+    """yaml
+    'Neos.Neos:Sites':
+      superTypes:
+        'Neos.ContentRepository:Root': true
+    'Neos.Neos:Document':
+      properties:
+        uriPathSegment:
+          type: string
+    'Neos.Neos:Site':
+      superTypes:
+        'Neos.Neos:Document': true
+    """
+    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"               |
+      | workspaceTitle       | "Live"               |
+      | workspaceDescription | "The live workspace" |
+      | newContentStreamId   | "cs-identifier"      |
+    And the graph projection is fully up to date
+    And I am in the active content stream of workspace "live" and dimension space point {"example":"source"}
+    And the command CreateRootNodeAggregateWithNode is executed with payload:
+      | Key             | Value                    |
+      | nodeAggregateId | "lady-eleonode-rootford" |
+      | nodeTypeName    | "Neos.Neos:Sites"        |
+    And the graph projection is fully up to date
+    And the command CreateNodeAggregateWithNode is executed with payload:
+      | Key                       | Value                    |
+      | nodeAggregateId           | "shernode-homes"         |
+      | nodeTypeName              | "Neos.Neos:Site"         |
+      | parentNodeAggregateId     | "lady-eleonode-rootford" |
+      | originDimensionSpacePoint | {"example":"source"}     |
+    And the graph projection is fully up to date
+    And the following CreateNodeAggregateWithNode commands are executed:
+      | nodeAggregateId      | nodeName          | parentNodeAggregateId | succeedingSiblingNodeAggregateId | nodeTypeName       | initialPropertyValues          |
+    # Let's prepare some siblings to check orderings. Also, everything gets better with siblings.
+      | elder-mc-nodeface    | elder-document    | shernode-homes        |                                  | Neos.Neos:Document | {"uriPathSegment": "elder"}    |
+      | eldest-mc-nodeface   | eldest-document   | shernode-homes        | elder-mc-nodeface                | Neos.Neos:Document | {"uriPathSegment": "eldest"}   |
+      | younger-mc-nodeface  | younger-document  | shernode-homes        |                                  | Neos.Neos:Document | {"uriPathSegment": "younger"}  |
+      | youngest-mc-nodeface | youngest-document | shernode-homes        |                                  | Neos.Neos:Document | {"uriPathSegment": "youngest"} |
+    Given the command RemoveNodeAggregate is executed with payload:
+      | Key                          | Value                 |
+      | nodeAggregateId              | "younger-mc-nodeface" |
+      | coveredDimensionSpacePoint   | {"example":"spec"}    |
+      | nodeVariantSelectionStrategy | "allSpecializations"  |
+    And the graph projection is fully up to date
+    When the following CreateNodeAggregateWithNode commands are executed:
+      | nodeAggregateId  | nodeName | parentNodeAggregateId | succeedingSiblingNodeAggregateId | nodeTypeName       | initialPropertyValues      |
+      | nody-mc-nodeface | document | shernode-homes        | younger-mc-nodeface              | Neos.Neos:Document | {"uriPathSegment": "nody"} |
+
+    Then I expect the documenturipath table to contain exactly:
+      # source: 65901ded4f068dac14ad0dce4f459b29
+      # spec: 9a723c057afa02982dae9d0b541739be
+      # leafSpec: c60c44685475d0e2e4f2b964e6158ce2
+      | dimensionspacepointhash            | uripath    | nodeaggregateidpath                                          | nodeaggregateid          | parentnodeaggregateid    | precedingnodeaggregateid | succeedingnodeaggregateid | nodetypename         |
+      | "65901ded4f068dac14ad0dce4f459b29" | ""         | "lady-eleonode-rootford"                                     | "lady-eleonode-rootford" | null                     | null                     | null                      | "Neos.Neos:Sites"    |
+      | "9a723c057afa02982dae9d0b541739be" | ""         | "lady-eleonode-rootford"                                     | "lady-eleonode-rootford" | null                     | null                     | null                      | "Neos.Neos:Sites"    |
+      | "c60c44685475d0e2e4f2b964e6158ce2" | ""         | "lady-eleonode-rootford"                                     | "lady-eleonode-rootford" | null                     | null                     | null                      | "Neos.Neos:Sites"    |
+      | "65901ded4f068dac14ad0dce4f459b29" | ""         | "lady-eleonode-rootford/shernode-homes"                      | "shernode-homes"         | "lady-eleonode-rootford" | null                     | null                      | "Neos.Neos:Site"     |
+      | "9a723c057afa02982dae9d0b541739be" | ""         | "lady-eleonode-rootford/shernode-homes"                      | "shernode-homes"         | "lady-eleonode-rootford" | null                     | null                      | "Neos.Neos:Site"     |
+      | "c60c44685475d0e2e4f2b964e6158ce2" | ""         | "lady-eleonode-rootford/shernode-homes"                      | "shernode-homes"         | "lady-eleonode-rootford" | null                     | null                      | "Neos.Neos:Site"     |
+      | "65901ded4f068dac14ad0dce4f459b29" | "elder"    | "lady-eleonode-rootford/shernode-homes/elder-mc-nodeface"    | "elder-mc-nodeface"      | "shernode-homes"         | "eldest-mc-nodeface"     | "nody-mc-nodeface"        | "Neos.Neos:Document" |
+      | "9a723c057afa02982dae9d0b541739be" | "elder"    | "lady-eleonode-rootford/shernode-homes/elder-mc-nodeface"    | "elder-mc-nodeface"      | "shernode-homes"         | "eldest-mc-nodeface"     | "nody-mc-nodeface"        | "Neos.Neos:Document" |
+      | "c60c44685475d0e2e4f2b964e6158ce2" | "elder"    | "lady-eleonode-rootford/shernode-homes/elder-mc-nodeface"    | "elder-mc-nodeface"      | "shernode-homes"         | "eldest-mc-nodeface"     | "nody-mc-nodeface"        | "Neos.Neos:Document" |
+      | "65901ded4f068dac14ad0dce4f459b29" | "eldest"   | "lady-eleonode-rootford/shernode-homes/eldest-mc-nodeface"   | "eldest-mc-nodeface"     | "shernode-homes"         | null                     | "elder-mc-nodeface"       | "Neos.Neos:Document" |
+      | "9a723c057afa02982dae9d0b541739be" | "eldest"   | "lady-eleonode-rootford/shernode-homes/eldest-mc-nodeface"   | "eldest-mc-nodeface"     | "shernode-homes"         | null                     | "elder-mc-nodeface"       | "Neos.Neos:Document" |
+      | "c60c44685475d0e2e4f2b964e6158ce2" | "eldest"   | "lady-eleonode-rootford/shernode-homes/eldest-mc-nodeface"   | "eldest-mc-nodeface"     | "shernode-homes"         | null                     | "elder-mc-nodeface"       | "Neos.Neos:Document" |
+      | "65901ded4f068dac14ad0dce4f459b29" | "nody"     | "lady-eleonode-rootford/shernode-homes/nody-mc-nodeface"     | "nody-mc-nodeface"       | "shernode-homes"         | "elder-mc-nodeface"      | "younger-mc-nodeface"     | "Neos.Neos:Document" |
+      | "9a723c057afa02982dae9d0b541739be" | "nody"     | "lady-eleonode-rootford/shernode-homes/nody-mc-nodeface"     | "nody-mc-nodeface"       | "shernode-homes"         | "elder-mc-nodeface"      | "youngest-mc-nodeface"    | "Neos.Neos:Document" |
+      | "c60c44685475d0e2e4f2b964e6158ce2" | "nody"     | "lady-eleonode-rootford/shernode-homes/nody-mc-nodeface"     | "nody-mc-nodeface"       | "shernode-homes"         | "elder-mc-nodeface"      | "youngest-mc-nodeface"    | "Neos.Neos:Document" |
+      | "65901ded4f068dac14ad0dce4f459b29" | "younger"  | "lady-eleonode-rootford/shernode-homes/younger-mc-nodeface"  | "younger-mc-nodeface"    | "shernode-homes"         | "nody-mc-nodeface"       | "youngest-mc-nodeface"    | "Neos.Neos:Document" |
+      | "65901ded4f068dac14ad0dce4f459b29" | "youngest" | "lady-eleonode-rootford/shernode-homes/youngest-mc-nodeface" | "youngest-mc-nodeface"   | "shernode-homes"         | "younger-mc-nodeface"    | null                      | "Neos.Neos:Document" |
+      | "9a723c057afa02982dae9d0b541739be" | "youngest" | "lady-eleonode-rootford/shernode-homes/youngest-mc-nodeface" | "youngest-mc-nodeface"   | "shernode-homes"         | "nody-mc-nodeface"       | null                      | "Neos.Neos:Document" |
+      | "c60c44685475d0e2e4f2b964e6158ce2" | "youngest" | "lady-eleonode-rootford/shernode-homes/youngest-mc-nodeface" | "youngest-mc-nodeface"   | "shernode-homes"         | "nody-mc-nodeface"       | null                      | "Neos.Neos:Document" |