diff --git a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentTypeClassification.php b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentTypeClassification.php new file mode 100644 index 00000000000..8098e78668c --- /dev/null +++ b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentTypeClassification.php @@ -0,0 +1,68 @@ +getNodeType($nodeTypeName); + if ($nodeType === null) { + return self::CLASSIFICATION_UNKNOWN; + } + + if ($nodeType->isOfType(NodeTypeNameFactory::forSite())) { + return self::CLASSIFICATION_SITE; + } elseif ($nodeType->isOfType(NodeTypeNameFactory::forShortcut())) { + return self::CLASSIFICATION_SHORTCUT; + } elseif ($nodeType->isOfType(NodeTypeNameFactory::forDocument())) { + return self::CLASSIFICATION_DOCUMENT; + } + + return self::CLASSIFICATION_NONE; + } +} diff --git a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathFinder.php b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathFinder.php index 1f0ddda17ba..d219e450a88 100644 --- a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathFinder.php +++ b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathFinder.php @@ -52,7 +52,8 @@ public function getEnabledBySiteNodeNameUriPathAndDimensionSpacePointHash( 'dimensionSpacePointHash = :dimensionSpacePointHash AND siteNodeName = :siteNodeName AND uriPath = :uriPath - AND disabled = 0', + AND disabled = 0 + AND isPlaceholder = 0', [ 'dimensionSpacePointHash' => $dimensionSpacePointHash, 'siteNodeName' => $siteNodeName->value, diff --git a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathProjection.php b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathProjection.php index 7128d92a049..9d99aad0174 100644 --- a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathProjection.php +++ b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathProjection.php @@ -38,7 +38,6 @@ use Neos\EventStore\Model\Event\SequenceNumber; use Neos\EventStore\Model\EventEnvelope; use Neos\Neos\Domain\Model\SiteNodeName; -use Neos\Neos\Domain\Service\NodeTypeNameFactory; use Neos\Neos\FrontendRouting\Exception\NodeNotFoundException; /** @@ -54,9 +53,9 @@ final class DocumentUriPathProjection implements ProjectionInterface, WithMarkSt private ?DocumentUriPathFinder $stateAccessor = null; /** - * @var array> + * @var array */ - private array $nodeTypeImplementsRuntimeCache = []; + private array $documentTypeClassificationRuntimeCache = []; public function __construct( private readonly NodeTypeManager $nodeTypeManager, @@ -269,7 +268,8 @@ private function whenNodeAggregateWithNodeWasCreated(NodeAggregateWithNodeWasCre if (!$this->getState()->isLiveContentStream($event->contentStreamId)) { return; } - if (!$this->isDocumentNodeType($event->nodeTypeName)) { + $documentTypeClassification = $this->getDocumentTypeClassification($event->nodeTypeName); + if ($documentTypeClassification === DocumentTypeClassification::CLASSIFICATION_NONE) { return; } @@ -277,7 +277,7 @@ private function whenNodeAggregateWithNodeWasCreated(NodeAggregateWithNodeWasCre $uriPathSegment = $propertyValues['uriPathSegment'] ?? ''; $shortcutTarget = null; - if ($this->isShortcutNodeType($event->nodeTypeName)) { + if ($documentTypeClassification === DocumentTypeClassification::CLASSIFICATION_SHORTCUT) { $shortcutTarget = [ 'mode' => $propertyValues['targetMode'] ?? 'firstChildNode', 'target' => $propertyValues['target'] ?? null, @@ -353,6 +353,7 @@ private function whenNodeAggregateWithNodeWasCreated(NodeAggregateWithNodeWasCre 'shortcutTarget' => $shortcutTarget, 'nodeTypeName' => $event->nodeTypeName->value, 'disabled' => $parentNode->getDisableLevel(), + 'isPlaceholder' => (int)($documentTypeClassification === DocumentTypeClassification::CLASSIFICATION_UNKNOWN) ]); } } @@ -362,20 +363,33 @@ private function whenNodeAggregateTypeWasChanged(NodeAggregateTypeWasChanged $ev if (!$this->getState()->isLiveContentStream($event->contentStreamId)) { return; } - if ($this->isShortcutNodeType($event->newNodeTypeName)) { - // The node has been turned into a shortcut node, but since the shortcut mode is not yet set - // we'll set it to "firstChildNode" in order to prevent an invalid mode - $this->updateNodeQuery('SET shortcuttarget = COALESCE(shortcuttarget,\'{"mode":"firstChildNode","target":null}\'), nodeTypeName=:nodeTypeName + switch ($this->getDocumentTypeClassification($event->newNodeTypeName)) { + case DocumentTypeClassification::CLASSIFICATION_SHORTCUT: + // The node has been turned into a shortcut node, but since the shortcut mode is not yet set + // we'll set it to "firstChildNode" in order to prevent an invalid mode + $this->updateNodeQuery('SET shortcuttarget = COALESCE(shortcuttarget,\'{"mode":"firstChildNode","target":null}\'), nodeTypeName=:nodeTypeName, isPlaceholder=:isPlaceholder WHERE nodeAggregateId = :nodeAggregateId', [ - 'nodeAggregateId' => $event->nodeAggregateId->value, - 'nodeTypeName' => $event->newNodeTypeName->value, - ]); - } elseif ($this->isDocumentNodeType($event->newNodeTypeName)) { - $this->updateNodeQuery('SET shortcuttarget = NULL, nodeTypeName=:nodeTypeName + 'nodeAggregateId' => $event->nodeAggregateId->value, + 'nodeTypeName' => $event->newNodeTypeName->value, + 'isPlaceholder' => 0 + ]); + break; + case DocumentTypeClassification::CLASSIFICATION_DOCUMENT: + $this->updateNodeQuery('SET shortcuttarget = NULL, nodeTypeName=:nodeTypeName, isPlaceholder=:isPlaceholder WHERE nodeAggregateId = :nodeAggregateId', [ - 'nodeAggregateId' => $event->nodeAggregateId->value, - 'nodeTypeName' => $event->newNodeTypeName->value, - ]); + 'nodeAggregateId' => $event->nodeAggregateId->value, + 'nodeTypeName' => $event->newNodeTypeName->value, + 'isPlaceholder' => 0 + ]); + break; + case DocumentTypeClassification::CLASSIFICATION_SITE: + // Sites cannot be moved or type-changed to anything else, so it must have been a site befor + // -> nothing to do + break; + case DocumentTypeClassification::CLASSIFICATION_UNKNOWN: + case DocumentTypeClassification::CLASSIFICATION_NONE: + // @todo: probably set to isPlaceholder: true if anything is found + break; } } @@ -561,7 +575,8 @@ private function whenNodePropertiesWereSet(NodePropertiesWereSet $event, EventEn if ( $node === null - || $this->isSiteNodeType($node->getNodeTypeName()) + || $this->getDocumentTypeClassification($node->getNodeTypeName()) + === DocumentTypeClassification::CLASSIFICATION_SITE ) { // probably not a document node continue; @@ -716,35 +731,19 @@ private function isNodeExplicitlyDisabled(DocumentNodeInfo $node): bool return $node->getDisableLevel() - $parentDisabledLevel !== 0; } - private function isSiteNodeType(NodeTypeName $nodeTypeName): bool - { - return $this->isNodeTypeOfType($nodeTypeName, NodeTypeNameFactory::forSite()); - } - - private function isDocumentNodeType(NodeTypeName $nodeTypeName): bool + private function getDocumentTypeClassification(NodeTypeName $nodeTypeName): DocumentTypeClassification { - return $this->isNodeTypeOfType($nodeTypeName, NodeTypeNameFactory::forDocument()); - } - - private function isShortcutNodeType(NodeTypeName $nodeTypeName): bool - { - return $this->isNodeTypeOfType($nodeTypeName, NodeTypeNameFactory::forShortcut()); - } - - private function isNodeTypeOfType(NodeTypeName $nodeTypeName, NodeTypeName $superNodeTypeName): bool - { - if (!array_key_exists($superNodeTypeName->value, $this->nodeTypeImplementsRuntimeCache)) { - $this->nodeTypeImplementsRuntimeCache[$superNodeTypeName->value] = []; - } - if (!array_key_exists($nodeTypeName->value, $this->nodeTypeImplementsRuntimeCache[$superNodeTypeName->value])) { + if (!array_key_exists($nodeTypeName->value, $this->documentTypeClassificationRuntimeCache)) { // HACK: We consider the currently configured node type of the given node. // This is a deliberate side effect of this projector! // Note: We could add some hash over all node type decisions to the projected read model // to tell whether a replay is required (e.g. if a document node type was changed to a content type vice versa) // With https://github.com/neos/neos-development-collection/issues/4468 this can be compared in the `getStatus()` implementation - $this->nodeTypeImplementsRuntimeCache[$superNodeTypeName->value][$nodeTypeName->value] = $this->nodeTypeManager->getNodeType($nodeTypeName)?->isOfType($superNodeTypeName) ?? false; + $this->documentTypeClassificationRuntimeCache[$nodeTypeName->value] + = DocumentTypeClassification::forNodeType($nodeTypeName, $this->nodeTypeManager); } - return $this->nodeTypeImplementsRuntimeCache[$superNodeTypeName->value][$nodeTypeName->value]; + + return $this->documentTypeClassificationRuntimeCache[$nodeTypeName->value]; } private function tryGetNode(\Closure $closure): ?DocumentNodeInfo @@ -980,7 +979,8 @@ private function whenDimensionShineThroughWasAdded(DimensionShineThroughWasAdded precedingnodeaggregateid, succeedingnodeaggregateid, shortcuttarget, - nodetypename + nodetypename, + isplaceholder ) SELECT nodeaggregateid, @@ -994,7 +994,8 @@ private function whenDimensionShineThroughWasAdded(DimensionShineThroughWasAdded precedingnodeaggregateid, succeedingnodeaggregateid, shortcuttarget, - nodetypename + nodetypename, + isplaceholder FROM ' . $this->tableNamePrefix . '_uri WHERE diff --git a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathSchemaBuilder.php b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathSchemaBuilder.php index 0d7a02dcfb9..37dba73f0dc 100644 --- a/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathSchemaBuilder.php +++ b/Neos.Neos/Classes/FrontendRouting/Projection/DocumentUriPathSchemaBuilder.php @@ -52,7 +52,8 @@ private function createUriTable(): Table DbalSchemaFactory::columnForNodeAggregateId('precedingnodeaggregateid')->setNotNull(false), DbalSchemaFactory::columnForNodeAggregateId('succeedingnodeaggregateid')->setNotNull(false), (new Column('shortcuttarget', Type::getType(Types::STRING)))->setLength(1000)->setNotnull(false)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), - DbalSchemaFactory::columnForNodeTypeName('nodetypename') + DbalSchemaFactory::columnForNodeTypeName('nodetypename'), + (new Column('isplaceholder', Type::getType(Types::INTEGER)))->setLength(4)->setUnsigned(true)->setDefault(0)->setNotnull(true), ]); return $table diff --git a/Neos.Neos/Tests/Behavior/Features/FrontendRouting/NodeAggregateTypeChangeEdgeCases.feature b/Neos.Neos/Tests/Behavior/Features/FrontendRouting/NodeAggregateTypeChangeEdgeCases.feature new file mode 100644 index 00000000000..c2f8c663fc7 --- /dev/null +++ b/Neos.Neos/Tests/Behavior/Features/FrontendRouting/NodeAggregateTypeChangeEdgeCases.feature @@ -0,0 +1,95 @@ +@flowEntities @contentrepository +Feature: Test cases for node aggregate type change edge cases + + Scenario: Create a (non-document) node with uri path segment, change the node aggregate's type to be a document and expect it to now have a uri path segment + # Note: this is not some esoteric fantasy case, it happens when you rename a node type, change a node's type to the new one and replay the documentUriPathProjection + Given using the following content dimensions: + | Identifier | Values | Generalizations | + | example | general,source,peer,peerSpec | peerSpec->peer->general,source->general | + 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 I am in workspace "live" and dimension space point {"example":"general"} + And the command CreateRootNodeAggregateWithNode is executed with payload: + | Key | Value | + | nodeAggregateId | "lady-eleonode-rootford" | + | nodeTypeName | "Neos.Neos:Sites" | + And the following CreateNodeAggregateWithNode commands are executed: + | nodeAggregateId | originDimensionSpacePoint | nodeName | parentNodeAggregateId | nodeTypeName | initialPropertyValues | + | shernode-homes | {"example":"general"} | site | lady-eleonode-rootford | Neos.Neos:Site | {} | + | anthony-destinode | {"example":"general"} | anthony | shernode-homes | Neos.Neos:Document | {"uriPathSegment": "anthony"} | + | berta-destinode | {"example":"general"} | berta | shernode-homes | Neos.Neos:Document | {"uriPathSegment": "berta"} | + # Set up our test subject document + # We do this via event since it must be of an unknown node type to simulate the replay + And the event NodeAggregateWithNodeWasCreated was published with payload: + | Key | Value | + | workspaceName | "live" | + | contentStreamId | "cs-identifier" | + | nodeAggregateId | "nody-mc-nodeface" | + | nodeTypeName | "Neos.Neos:SomethingThatOnceWasADocument" | + | originDimensionSpacePoint | {"example":"general"} | + | coveredDimensionSpacePoints | [{"example":"general"},{"example":"source"},{"example":"peer"},{"example":"peerSpec"}] | + | parentNodeAggregateId | "anthony-destinode" | + | nodeName | "document" | + | nodeAggregateClassification | "regular" | + | initialPropertyValues | {"uriPathSegment": {"type": "string", "value": "nody"}} | + And the event NodeAggregateWasMoved was published with payload: + # Let's move the node partially to add a little brainfuck + # We do this via event to bypass constraint checks on the still non-existent node type + | Key | Value | + | workspaceName | "live" | + | contentStreamId | "cs-identifier" | + | nodeAggregateId | "nody-mc-nodeface" | + | newParentNodeAggregateId | "berta-destinode" | + | succeedingSiblingsForCoverage | [{"dimensionSpacePoint":{"example": "peer"},"nodeAggregateId": null},{"dimensionSpacePoint":{"example": "peerSpec"},"nodeAggregateId": null}] | + And the command ChangeNodeAggregateType is executed with payload: + | Key | Value | + | nodeAggregateId | "nody-mc-nodeface" | + | newNodeTypeName | "Neos.Neos:Document" | + | strategy | "happypath" | + + Then I expect the documenturipath table to contain exactly: + # general: 033e5de7b423f45bb4f5a09f73af839e + # source: 65901ded4f068dac14ad0dce4f459b29 + # peer: fbe53ddc3305685fbb4dbf529f283a0e + # peerSpec: 2ca4fae2f65267c94c85602df0cbb728 + | dimensionspacepointhash | uripath | nodeaggregateidpath | nodeaggregateid | parentnodeaggregateid | precedingnodeaggregateid | succeedingnodeaggregateid | nodetypename | + | "033e5de7b423f45bb4f5a09f73af839e" | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" | + | "2ca4fae2f65267c94c85602df0cbb728" | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" | + | "65901ded4f068dac14ad0dce4f459b29" | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" | + | "fbe53ddc3305685fbb4dbf529f283a0e" | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" | + | "033e5de7b423f45bb4f5a09f73af839e" | "" | "lady-eleonode-rootford/shernode-homes" | "shernode-homes" | "lady-eleonode-rootford" | null | null | "Neos.Neos:Site" | + | "2ca4fae2f65267c94c85602df0cbb728" | "" | "lady-eleonode-rootford/shernode-homes" | "shernode-homes" | "lady-eleonode-rootford" | null | null | "Neos.Neos:Site" | + | "65901ded4f068dac14ad0dce4f459b29" | "" | "lady-eleonode-rootford/shernode-homes" | "shernode-homes" | "lady-eleonode-rootford" | null | null | "Neos.Neos:Site" | + | "fbe53ddc3305685fbb4dbf529f283a0e" | "" | "lady-eleonode-rootford/shernode-homes" | "shernode-homes" | "lady-eleonode-rootford" | null | null | "Neos.Neos:Site" | + | "033e5de7b423f45bb4f5a09f73af839e" | "anthony" | "lady-eleonode-rootford/shernode-homes/anthony-destinode" | "anthony-destinode" | "shernode-homes" | null | "berta-destinode" | "Neos.Neos:Document" | + | "2ca4fae2f65267c94c85602df0cbb728" | "anthony" | "lady-eleonode-rootford/shernode-homes/anthony-destinode" | "anthony-destinode" | "shernode-homes" | null | "berta-destinode" | "Neos.Neos:Document" | + | "65901ded4f068dac14ad0dce4f459b29" | "anthony" | "lady-eleonode-rootford/shernode-homes/anthony-destinode" | "anthony-destinode" | "shernode-homes" | null | "berta-destinode" | "Neos.Neos:Document" | + | "fbe53ddc3305685fbb4dbf529f283a0e" | "anthony" | "lady-eleonode-rootford/shernode-homes/anthony-destinode" | "anthony-destinode" | "shernode-homes" | null | "berta-destinode" | "Neos.Neos:Document" | + | "033e5de7b423f45bb4f5a09f73af839e" | "anthony/nody" | "lady-eleonode-rootford/shernode-homes/anthony-destinode/nody-mc-nodeface" | "nody-mc-nodeface" | "anthony-destinode" | null | null | "Neos.Neos:Document" | + | "65901ded4f068dac14ad0dce4f459b29" | "anthony/nody" | "lady-eleonode-rootford/shernode-homes/anthony-destinode/nody-mc-nodeface" | "nody-mc-nodeface" | "anthony-destinode" | null | null | "Neos.Neos:Document" | + | "033e5de7b423f45bb4f5a09f73af839e" | "berta" | "lady-eleonode-rootford/shernode-homes/berta-destinode" | "berta-destinode" | "shernode-homes" | "anthony-destinode" | null | "Neos.Neos:Document" | + | "2ca4fae2f65267c94c85602df0cbb728" | "berta" | "lady-eleonode-rootford/shernode-homes/berta-destinode" | "berta-destinode" | "shernode-homes" | "anthony-destinode" | null | "Neos.Neos:Document" | + | "65901ded4f068dac14ad0dce4f459b29" | "berta" | "lady-eleonode-rootford/shernode-homes/berta-destinode" | "berta-destinode" | "shernode-homes" | "anthony-destinode" | null | "Neos.Neos:Document" | + | "fbe53ddc3305685fbb4dbf529f283a0e" | "berta" | "lady-eleonode-rootford/shernode-homes/berta-destinode" | "berta-destinode" | "shernode-homes" | "anthony-destinode" | null | "Neos.Neos:Document" | + | "2ca4fae2f65267c94c85602df0cbb728" | "berta/nody" | "lady-eleonode-rootford/shernode-homes/berta-destinode/nody-mc-nodeface" | "nody-mc-nodeface" | "berta-destinode" | null | null | "Neos.Neos:Document" | + | "fbe53ddc3305685fbb4dbf529f283a0e" | "berta/nody" | "lady-eleonode-rootford/shernode-homes/berta-destinode/nody-mc-nodeface" | "nody-mc-nodeface" | "berta-destinode" | null | null | "Neos.Neos:Document" | diff --git a/Neos.Neos/Tests/Behavior/Features/FrontendRouting/UnknownNodeTypes.feature b/Neos.Neos/Tests/Behavior/Features/FrontendRouting/UnknownNodeTypes.feature index 2d2d5495265..904990f72db 100644 --- a/Neos.Neos/Tests/Behavior/Features/FrontendRouting/UnknownNodeTypes.feature +++ b/Neos.Neos/Tests/Behavior/Features/FrontendRouting/UnknownNodeTypes.feature @@ -58,7 +58,8 @@ Feature: Basic routing functionality (match & resolve nodes with unknown types) | nodeAggregateClassification | "regular" | | initialPropertyValues | {"uriPathSegment": {"type": "string", "value": "non-existing"}} | Then I expect the documenturipath table to contain exactly: - | uripath | nodeaggregateidpath | nodeaggregateid | parentnodeaggregateid | precedingnodeaggregateid | succeedingnodeaggregateid | nodetypename | - | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" | - | "" | "lady-eleonode-rootford/shernode-homes" | "shernode-homes" | "lady-eleonode-rootford" | null | null | "Neos.Neos:Test.Routing.Page" | - | "david-nodenborough" | "lady-eleonode-rootford/shernode-homes/sir-david-nodenborough" | "sir-david-nodenborough" | "shernode-homes" | null | null | "Neos.Neos:Test.Routing.Page" | + | uripath | nodeaggregateidpath | nodeaggregateid | parentnodeaggregateid | precedingnodeaggregateid | succeedingnodeaggregateid | nodetypename | isPlaceholder | + | "" | "lady-eleonode-rootford" | "lady-eleonode-rootford" | null | null | null | "Neos.Neos:Sites" | false | + | "" | "lady-eleonode-rootford/shernode-homes" | "shernode-homes" | "lady-eleonode-rootford" | null | null | "Neos.Neos:Test.Routing.Page" | false | + | "david-nodenborough" | "lady-eleonode-rootford/shernode-homes/sir-david-nodenborough" | "sir-david-nodenborough" | "shernode-homes" | null | "unknown-nodetype" | "Neos.Neos:Test.Routing.Page" | false | + | "non-existing" | "lady-eleonode-rootford/shernode-homes/unknown-nodetype" | "unknown-nodetype" | "shernode-homes" | "sir-david-nodenborough" | null | "Neos.Neos:Test.Routing.NonExisting" | true |