From fb89fb65cb133c1c48c4d82d553fa23e5951468c Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 6 Jun 2023 21:40:21 +0200 Subject: [PATCH 1/3] TASK: Make tethered node ids deterministic --- .../Command/CreateNodeAggregateWithNode.php | 3 ++ .../Feature/NodeCreation/NodeCreation.php | 32 ++----------------- .../Feature/NodeTypeChange/NodeTypeChange.php | 13 ++------ .../Classes/NodeType/NodeType.php | 23 +++++++++++++ .../SharedModel/Node/NodeAggregateId.php | 17 ++++++++++ 5 files changed, 49 insertions(+), 39 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php index 081b877b0d8..c6cec737cb3 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php @@ -58,6 +58,9 @@ final class CreateNodeAggregateWithNode implements CommandInterface * using this assignment registry. * Since tethered child nodes may have tethered child nodes themselves, * this registry is indexed using relative node paths to the node to create in the first place. + * + * Specifying them is optional, and otherwise they are calculated deterministic based on the $nodeAggregateId and the tethered nodeName. + * So you can determine the child node identifier via {@see NodeAggregateId::fromParentNodeAggregateIdAndNodeName()} */ public readonly NodeAggregateIdsByNodePaths $tetheredDescendantNodeAggregateIds; diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php index 10cc11bc488..d430dc2dfc5 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php @@ -199,10 +199,9 @@ private function handleCreateNodeAggregateWithNodeAndSerializedProperties( $contentRepository ); } - $descendantNodeAggregateIds = self::populateNodeAggregateIds( - $nodeType, - $command->tetheredDescendantNodeAggregateIds - ); + + $descendantNodeAggregateIds = $nodeType->tetheredNodeAggregateIds($command->nodeAggregateId) + ->merge($command->tetheredDescendantNodeAggregateIds); // Write the auto-created descendant node aggregate ids back to the command; // so that when rebasing the command, it stays fully deterministic. $command = $command->withTetheredDescendantNodeAggregateIds($descendantNodeAggregateIds); @@ -338,29 +337,4 @@ private function createTetheredWithNode( $precedingNodeAggregateId ); } - - protected static function populateNodeAggregateIds( - NodeType $nodeType, - ?NodeAggregateIdsByNodePaths $nodeAggregateIds, - NodePath $childPath = null - ): NodeAggregateIdsByNodePaths { - if ($nodeAggregateIds === null) { - $nodeAggregateIds = NodeAggregateIdsByNodePaths::createEmpty(); - } - // TODO: handle Multiple levels of autocreated child nodes - foreach ($nodeType->getAutoCreatedChildNodes() as $rawChildName => $childNodeType) { - $childName = NodeName::fromString($rawChildName); - $childPath = $childPath - ? $childPath->appendPathSegment($childName) - : NodePath::fromString($childName->value); - if (!$nodeAggregateIds->getNodeAggregateId($childPath)) { - $nodeAggregateIds = $nodeAggregateIds->add( - $childPath, - NodeAggregateId::create() - ); - } - } - - return $nodeAggregateIds; - } } diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php b/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php index 6db1e898724..ad4816a209a 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php @@ -87,12 +87,6 @@ abstract protected function areNodeTypeConstraintsImposedByGrandparentValid( NodeType $nodeType ): bool; - abstract protected static function populateNodeAggregateIds( - NodeType $nodeType, - NodeAggregateIdsByNodePaths $nodeAggregateIds, - NodePath $childPath = null - ): NodeAggregateIdsByNodePaths; - abstract protected function createEventsForMissingTetheredNode( NodeAggregate $parentNodeAggregate, Node $parentNode, @@ -156,10 +150,9 @@ private function handleChangeNodeAggregateType( /************** * Preparation - make the command fully deterministic in case of rebase **************/ - $descendantNodeAggregateIds = static::populateNodeAggregateIds( - $newNodeType, - $command->tetheredDescendantNodeAggregateIds - ); + $descendantNodeAggregateIds = $newNodeType->tetheredNodeAggregateIds($command->nodeAggregateId) + ->merge($command->tetheredDescendantNodeAggregateIds ?? NodeAggregateIdsByNodePaths::createEmpty()); + // Write the auto-created descendant node aggregate ids back to the command; // so that when rebasing the command, it stays fully deterministic. $command = $command->withTetheredDescendantNodeAggregateIds($descendantNodeAggregateIds); diff --git a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php index 91f4d254b78..191edfd330b 100644 --- a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php +++ b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php @@ -15,7 +15,10 @@ */ +use Neos\ContentRepository\Core\Feature\NodeCreation\Dto\NodeAggregateIdsByNodePaths; +use Neos\ContentRepository\Core\Projection\ContentGraph\NodePath; use Neos\ContentRepository\Core\SharedModel\Exception\NodeTypeNotFoundException; +use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; use Neos\ContentRepository\Core\SharedModel\Node\NodeName; use Neos\Utility\ObjectAccess; use Neos\Utility\Arrays; @@ -490,11 +493,31 @@ public function hasAutoCreatedChildNode(NodeName $nodeName): bool */ public function getTypeOfAutoCreatedChildNode(NodeName $nodeName): ?NodeType { + $this->initialize(); return isset($this->fullConfiguration['childNodes'][$nodeName->value]['type']) ? $this->nodeTypeManager->getNodeType($this->fullConfiguration['childNodes'][$nodeName->value]['type']) : null; } + /** + * Calculates the deterministic nodeAggregateIds for the auto-created childNodes. + */ + public function tetheredNodeAggregateIds(NodeAggregateId $parentNodeAggregateId): NodeAggregateIdsByNodePaths + { + $this->initialize(); + $nodeAggregateIdsByNodePaths = NodeAggregateIdsByNodePaths::createEmpty(); + if (!isset($this->fullConfiguration['childNodes'])) { + return $nodeAggregateIdsByNodePaths; + } + // TODO: handle Multiple levels of autocreated child nodes + foreach (array_keys($this->fullConfiguration['childNodes']) as $tetheredNodeName) { + $nodeAggregateIdsByNodePaths = $nodeAggregateIdsByNodePaths->add( + NodePath::fromString($tetheredNodeName), + NodeAggregateId::fromParentNodeAggregateIdAndNodeName($parentNodeAggregateId, NodeName::fromString($tetheredNodeName)) + ); + } + return $nodeAggregateIdsByNodePaths; + } /** * Checks if the given NodeType is acceptable as sub-node with the configured constraints, diff --git a/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeAggregateId.php b/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeAggregateId.php index f81c98fbce6..193bfb43f18 100644 --- a/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeAggregateId.php +++ b/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeAggregateId.php @@ -50,6 +50,23 @@ public static function fromString(string $value): self return new self($value); } + /** + * Calculates a deterministic nodeAggregateId. + * + * Used for determining the nodeAggregateIds of tethered nodes. + * + * e.g. in case you want to get the main content collection node aggregate id: + * NodeAggregateId::fromParentNodeAggregateIdAndNodeName($parentNodeAggregateId, NodeName::fromString('main')) + * + */ + public static function fromParentNodeAggregateIdAndNodeName(NodeAggregateId $parentNodeAggregateId, NodeName $tetheredNodeName): self + { + $hex = md5($parentNodeAggregateId->value . '-' . $tetheredNodeName->value); + return new self( + substr($hex, 0, 8) . '-' . substr($hex, 8, 4) . '-' . substr($hex, 12, 4) . '-' . substr($hex, 16, 4) . '-' . substr($hex, 20, 12) + ); + } + public function equals(self $other): bool { return $this->value === $other->value; From 7f1103c844e780473918fb8485680cf180ba0dab Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 6 Jun 2023 22:35:34 +0200 Subject: [PATCH 2/3] TASK: `tetheredNodeAggregateIds` handle multiple levels of autocreated child nodes --- .../Classes/NodeType/NodeType.php | 19 +++--- .../Tests/Unit/NodeType/NodeTypeTest.php | 63 +++++++++++++++++++ .../SharedModel/Node/NodeAggregateIdTest.php | 37 +++++++++++ 3 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php diff --git a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php index 191edfd330b..e07c1aae503 100644 --- a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php +++ b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php @@ -504,17 +504,20 @@ public function getTypeOfAutoCreatedChildNode(NodeName $nodeName): ?NodeType */ public function tetheredNodeAggregateIds(NodeAggregateId $parentNodeAggregateId): NodeAggregateIdsByNodePaths { - $this->initialize(); $nodeAggregateIdsByNodePaths = NodeAggregateIdsByNodePaths::createEmpty(); - if (!isset($this->fullConfiguration['childNodes'])) { - return $nodeAggregateIdsByNodePaths; - } - // TODO: handle Multiple levels of autocreated child nodes - foreach (array_keys($this->fullConfiguration['childNodes']) as $tetheredNodeName) { + foreach ($this->getAutoCreatedChildNodes() as $childNodeName => $childNodeType) { $nodeAggregateIdsByNodePaths = $nodeAggregateIdsByNodePaths->add( - NodePath::fromString($tetheredNodeName), - NodeAggregateId::fromParentNodeAggregateIdAndNodeName($parentNodeAggregateId, NodeName::fromString($tetheredNodeName)) + $childNodePath = NodePath::fromString($childNodeName), + $childNodeAggregateId = NodeAggregateId::fromParentNodeAggregateIdAndNodeName($parentNodeAggregateId, NodeName::fromString($childNodeName)) ); + + // handle multiple levels of autocreated child nodes + foreach ($childNodeType->tetheredNodeAggregateIds($childNodeAggregateId)->getNodeAggregateIds() as $nestedChildNodeName => $nodeAggregateId) { + $nodeAggregateIdsByNodePaths = $nodeAggregateIdsByNodePaths->add( + $childNodePath->appendPathSegment(NodeName::fromString($nestedChildNodeName)), + $nodeAggregateId + ); + }; } return $nodeAggregateIdsByNodePaths; } diff --git a/Neos.ContentRepository.Core/Tests/Unit/NodeType/NodeTypeTest.php b/Neos.ContentRepository.Core/Tests/Unit/NodeType/NodeTypeTest.php index bd38fc10066..c9a456876df 100644 --- a/Neos.ContentRepository.Core/Tests/Unit/NodeType/NodeTypeTest.php +++ b/Neos.ContentRepository.Core/Tests/Unit/NodeType/NodeTypeTest.php @@ -16,6 +16,7 @@ use Neos\ContentRepository\Core\NodeType\NodeTypeName; use Neos\ContentRepository\Core\NodeType\NodeType; use Neos\ContentRepository\Core\NodeType\NodeTypeManager; +use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; use PHPUnit\Framework\TestCase; /** @@ -422,4 +423,66 @@ public function getAutoCreatedChildNodesReturnsLowercasePaths() self::assertArrayHasKey('nodename', $autoCreatedChildNodes); } + + /** + * @test + */ + public function tetheredNodeAggregateIdsAreDetermined(): void + { + $nodeTypeNameA = NodeTypeName::fromString('Neos.ContentRepository:A'); + $nodeTypeConfigurationA = [ + 'childNodes' => [ + 'to-b' => [ + 'type' => 'Neos.ContentRepository:B' + ], + 'to-c' => [ + 'type' => 'Neos.ContentRepository:C' + ] + ] + ]; + + $nodeTypeNameB = NodeTypeName::fromString('Neos.ContentRepository:B'); + $nodeTypeConfigurationB = [ + 'childNodes' => [ + 'to-c-nested' => [ + 'type' => 'Neos.ContentRepository:C' + ] + ] + ]; + + $nodeTypeNameC = NodeTypeName::fromString('Neos.ContentRepository:C'); + $nodeTypeConfigurationC = []; + + $mockNodeTypeManager = $this->getMockBuilder(NodeTypeManager::class)->disableOriginalConstructor()->getMock(); + + /** @var list $nodeTypes */ + $nodeTypes = [ + $nodeTypeA = new NodeType($nodeTypeNameA, [], $nodeTypeConfigurationA, $mockNodeTypeManager, new DefaultNodeLabelGeneratorFactory()), + new NodeType($nodeTypeNameB, [], $nodeTypeConfigurationB, $mockNodeTypeManager, new DefaultNodeLabelGeneratorFactory()), + new NodeType($nodeTypeNameC, [], $nodeTypeConfigurationC, $mockNodeTypeManager, new DefaultNodeLabelGeneratorFactory()) + ]; + + $mockNodeTypeManager->expects(self::any())->method('getNodeType')->willReturnCallback(function (string|NodeTypeName $nodeTypeName) use ($nodeTypes) { + $nodeTypeName = is_string($nodeTypeName) ? NodeTypeName::fromString($nodeTypeName) : $nodeTypeName; + foreach ($nodeTypes as $nodeType) { + if ($nodeTypeName->equals($nodeType->name)) { + return $nodeType; + } + } + self::fail(sprintf('NodeTypeManagerMock doesnt know NodeTypeName: "%s"', $nodeTypeName->value)); + }); + + $nodeAggregateId = NodeAggregateId::fromString('b5b00957-2ed3-480e-845b-03fe3620bd5e'); + + $tetheredNodeAggregateIds = $nodeTypeA->tetheredNodeAggregateIds($nodeAggregateId); + + self::assertEquals( + [ + 'to-b' => NodeAggregateId::fromString('b9b6d9ac-dba4-2838-bec7-43baaf8a10d0'), + 'to-b/to-c-nested' => NodeAggregateId::fromString('bf248c95-cdd0-53c2-e570-75378aef47f2'), + 'to-c' => NodeAggregateId::fromString('59fbd163-06c5-4526-ffa6-0b20c81ae73a'), + ], + $tetheredNodeAggregateIds->getNodeAggregateIds() + ); + } } diff --git a/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php b/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php new file mode 100644 index 00000000000..173ed147212 --- /dev/null +++ b/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php @@ -0,0 +1,37 @@ + Date: Wed, 2 Aug 2023 13:37:18 +0200 Subject: [PATCH 3/3] TASK: Refactor to `NodeName::fromString('main')->tetheredNodeAggregateIdByParent($parentId)` instead of `NodeAggregateId::fromParentNodeAggregateIdAndNodeName` --- .../Command/CreateNodeAggregateWithNode.php | 2 +- .../Classes/NodeType/NodeType.php | 2 +- .../Classes/SharedModel/Node/NodeName.php | 17 +++++++++ .../SharedModel/Node/NodeAggregateIdTest.php | 37 ------------------- .../Unit/SharedModel/Node/NodeNameTest.php | 16 ++++++++ 5 files changed, 35 insertions(+), 39 deletions(-) delete mode 100644 Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php index c6cec737cb3..3a6592dd7a4 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/Command/CreateNodeAggregateWithNode.php @@ -60,7 +60,7 @@ final class CreateNodeAggregateWithNode implements CommandInterface * this registry is indexed using relative node paths to the node to create in the first place. * * Specifying them is optional, and otherwise they are calculated deterministic based on the $nodeAggregateId and the tethered nodeName. - * So you can determine the child node identifier via {@see NodeAggregateId::fromParentNodeAggregateIdAndNodeName()} + * So you can determine the child node identifier via {@see NodeName::tetheredNodeAggregateIdByParent()} */ public readonly NodeAggregateIdsByNodePaths $tetheredDescendantNodeAggregateIds; diff --git a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php index e07c1aae503..ea1a6f09a58 100644 --- a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php +++ b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php @@ -508,7 +508,7 @@ public function tetheredNodeAggregateIds(NodeAggregateId $parentNodeAggregateId) foreach ($this->getAutoCreatedChildNodes() as $childNodeName => $childNodeType) { $nodeAggregateIdsByNodePaths = $nodeAggregateIdsByNodePaths->add( $childNodePath = NodePath::fromString($childNodeName), - $childNodeAggregateId = NodeAggregateId::fromParentNodeAggregateIdAndNodeName($parentNodeAggregateId, NodeName::fromString($childNodeName)) + $childNodeAggregateId = NodeName::fromString($childNodeName)->tetheredNodeAggregateIdByParent($parentNodeAggregateId) ); // handle multiple levels of autocreated child nodes diff --git a/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeName.php b/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeName.php index 017ab717eba..d8351423396 100644 --- a/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeName.php +++ b/Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeName.php @@ -79,6 +79,23 @@ public static function transliterateFromString(string $name): self return new self($name); } + /** + * Calculates a deterministic nodeAggregateId. + * + * Used for determining the nodeAggregateIds of tethered nodes. + * + * e.g. in case you want to get the main content collection node aggregate id: + * NodeName::fromString('main')->tetheredNodeAggregateIdByParent($parentNodeAggregateId) + * + */ + public function tetheredNodeAggregateIdByParent(NodeAggregateId $parentNodeAggregateId): NodeAggregateId + { + $hex = md5($parentNodeAggregateId->value . '-' . $this->value); + return NodeAggregateId::fromString( + substr($hex, 0, 8) . '-' . substr($hex, 8, 4) . '-' . substr($hex, 12, 4) . '-' . substr($hex, 16, 4) . '-' . substr($hex, 20, 12) + ); + } + public function jsonSerialize(): string { return $this->value; diff --git a/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php b/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php deleted file mode 100644 index 173ed147212..00000000000 --- a/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Node/NodeAggregateIdTest.php +++ /dev/null @@ -1,37 +0,0 @@ -value); } + + /** + * @test + */ + public function nodeAggregateIdForTetheredNodesCanBeCalculatedDeterministic(): void + { + $nodeAggregateId = NodeAggregateId::fromString('b2e41ac5-671f-82ed-639d-3c8226631a74'); + + $childNodeAggregateId = NodeName::fromString('main')->tetheredNodeAggregateIdByParent($nodeAggregateId); + + self::assertEquals( + NodeAggregateId::fromString('ada70507-eb1e-a6aa-ebc6-4f7eddd441ac'), + $childNodeAggregateId + ); + } }