From 86586a6c0acab3f6d0d2ec5aba254019aea7267c Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 13 Jun 2023 08:34:51 +0200 Subject: [PATCH] TASK: NodeCreation: Check if node is abstract or if node is not allowed explicitly Backport of https://github.com/Flowpack/Flowpack.NodeTemplates/pull/51 The idea is we move more and more to separating the "command" creation and the actual apply should go through flawless. This will enable us validating the template without applying it. --- .../NodeCreation/NodeCreationService.php | 18 ++++++++++++++++++ Configuration/Testing/NodeTypes.Malformed.yaml | 4 +++- .../WithEvaluationExceptions.messages.json | 6 +++++- .../WithEvaluationExceptions.template.json | 8 +++++++- Tests/Functional/NodeTemplateTest.php | 6 +++--- Tests/Functional/SnapshotTrait.php | 12 ++++++++++++ 6 files changed, 48 insertions(+), 6 deletions(-) diff --git a/Classes/Domain/NodeCreation/NodeCreationService.php b/Classes/Domain/NodeCreation/NodeCreationService.php index 80ce510..12eec36 100644 --- a/Classes/Domain/NodeCreation/NodeCreationService.php +++ b/Classes/Domain/NodeCreation/NodeCreationService.php @@ -84,6 +84,24 @@ private function applyTemplateRecursively(Templates $templates, NodeInterface $p ); continue; } + + $nodeType = $this->nodeTypeManager->getNodeType($template->getType()->getValue()); + + if ($nodeType->isAbstract()) { + $caughtExceptions->add( + CaughtException::fromException(new \RuntimeException(sprintf('Template requires type to be a non abstract NodeType. Got: "%s".', $template->getType()->getValue()), 1686417628976)) + ); + continue; + } + + if (!$parentNode->getNodeType()->allowsChildNodeType($nodeType)) { + $caughtExceptions->add( + CaughtException::fromException(new \RuntimeException(sprintf('Node type "%s" is not allowed for child nodes of type %s', $template->getType()->getValue(), $parentNode->getNodeType()->getName()), 1686417627173)) + ); + continue; + } + + // todo maybe check also explicitly for allowsGrandchildNodeType (we do this currently like below) try { $node = $this->nodeOperations->create( $parentNode, diff --git a/Configuration/Testing/NodeTypes.Malformed.yaml b/Configuration/Testing/NodeTypes.Malformed.yaml index 1d5cd1e..08c96bf 100644 --- a/Configuration/Testing/NodeTypes.Malformed.yaml +++ b/Configuration/Testing/NodeTypes.Malformed.yaml @@ -54,8 +54,10 @@ type: 'Flowpack.NodeTemplates:Content.Text' properties: text: bar + abstractNodeAbort: + type: 'Neos.Neos:Node' illegalNodeAbort: - type: 'Neos.Neos:Document' + type: 'Flowpack.NodeTemplates:Document.Page.Static' name: 'illegal' properties: text: huhu diff --git a/Tests/Functional/Fixtures/WithEvaluationExceptions.messages.json b/Tests/Functional/Fixtures/WithEvaluationExceptions.messages.json index 0e0b812..fdf9b33 100644 --- a/Tests/Functional/Fixtures/WithEvaluationExceptions.messages.json +++ b/Tests/Functional/Fixtures/WithEvaluationExceptions.messages.json @@ -80,7 +80,11 @@ "severity": "ERROR" }, { - "message": "NodeConstraintException(Cannot create new node \"illegal\" of Type \"Neos.Neos:Document\" in Node \/sites\/test-site\/homepage\/main\/new-node@live[Flowpack.NodeTemplates:Content.WithEvaluationExceptions], 1400782413)", + "message": "RuntimeException(Template requires type to be a non abstract NodeType. Got: \"Neos.Neos:Node\"., 1686417628976)", + "severity": "ERROR" + }, + { + "message": "RuntimeException(Node type \"Flowpack.NodeTemplates:Document.Page.Static\" is not allowed for child nodes of type Flowpack.NodeTemplates:Content.WithEvaluationExceptions, 1686417627173)", "severity": "ERROR" }, { diff --git a/Tests/Functional/Fixtures/WithEvaluationExceptions.template.json b/Tests/Functional/Fixtures/WithEvaluationExceptions.template.json index dc99bca..9ff7fd5 100644 --- a/Tests/Functional/Fixtures/WithEvaluationExceptions.template.json +++ b/Tests/Functional/Fixtures/WithEvaluationExceptions.template.json @@ -21,7 +21,13 @@ "childNodes": [] }, { - "type": "Neos.Neos:Document", + "type": "Neos.Neos:Node", + "name": null, + "properties": [], + "childNodes": [] + }, + { + "type": "Flowpack.NodeTemplates:Document.Page.Static", "name": "illegal", "properties": { "text": "huhu" diff --git a/Tests/Functional/NodeTemplateTest.php b/Tests/Functional/NodeTemplateTest.php index dcea19f..996be2d 100644 --- a/Tests/Functional/NodeTemplateTest.php +++ b/Tests/Functional/NodeTemplateTest.php @@ -229,7 +229,7 @@ public function exceptionsAreCaughtAndPartialTemplateIsBuild(): void $createdNode = $targetNode->getChildNodes($toBeCreatedNodeTypeName->getValue())[0]; - $this->assertStringEqualsFileOrCreateSnapshot(__DIR__ . '/Fixtures/WithEvaluationExceptions.messages.json', json_encode($this->getMessagesOfFeedbackCollection(), JSON_PRETTY_PRINT)); + $this->assertJsonStringEqualsJsonFileOrCreateSnapshot(__DIR__ . '/Fixtures/WithEvaluationExceptions.messages.json', json_encode($this->getMessagesOfFeedbackCollection(), JSON_PRETTY_PRINT)); $this->assertNodeDumpAndTemplateDumpMatchSnapshot('WithEvaluationExceptions', $createdNode); } @@ -312,14 +312,14 @@ private function assertLastCreatedTemplateMatchesSnapshot(string $snapShotName): $lastCreatedTemplate = $this->serializeValuesInArray( $this->lastCreatedRootTemplate->jsonSerialize() ); - $this->assertStringEqualsFileOrCreateSnapshot(__DIR__ . '/Fixtures/' . $snapShotName . '.template.json', json_encode($lastCreatedTemplate, JSON_PRETTY_PRINT)); + $this->assertJsonStringEqualsJsonFileOrCreateSnapshot(__DIR__ . '/Fixtures/' . $snapShotName . '.template.json', json_encode($lastCreatedTemplate, JSON_PRETTY_PRINT)); } private function assertNodeDumpAndTemplateDumpMatchSnapshot(string $snapShotName, NodeInterface $node): void { $serializedNodes = $this->jsonSerializeNodeAndDescendents($node); unset($serializedNodes['nodeTypeName']); - $this->assertStringEqualsFileOrCreateSnapshot(__DIR__ . '/Fixtures/' . $snapShotName . '.nodes.json', json_encode($serializedNodes, JSON_PRETTY_PRINT)); + $this->assertJsonStringEqualsJsonFileOrCreateSnapshot(__DIR__ . '/Fixtures/' . $snapShotName . '.nodes.json', json_encode($serializedNodes, JSON_PRETTY_PRINT)); $dumpedYamlTemplate = $this->nodeTemplateDumper->createNodeTemplateYamlDumpFromSubtree($node); diff --git a/Tests/Functional/SnapshotTrait.php b/Tests/Functional/SnapshotTrait.php index a1a994f..032b3cb 100644 --- a/Tests/Functional/SnapshotTrait.php +++ b/Tests/Functional/SnapshotTrait.php @@ -12,7 +12,19 @@ private function assertStringEqualsFileOrCreateSnapshot(string $snapshotFileName if (getenv('CREATE_SNAPSHOT') === '1') { file_put_contents($snapshotFileName, $expectedString); $this->addWarning('Created snapshot.'); + return; } Assert::assertStringEqualsFile($snapshotFileName, $expectedString); } + + private function assertJsonStringEqualsJsonFileOrCreateSnapshot(string $snapshotFileName, string $expectedJsonString): void + { + $expectedJsonString = rtrim($expectedJsonString, "\n") . "\n"; + if (getenv('CREATE_SNAPSHOT') === '1') { + file_put_contents($snapshotFileName, $expectedJsonString); + $this->addWarning('Created snapshot.'); + return; + } + Assert::assertJsonStringEqualsJsonFile($snapshotFileName, $expectedJsonString); + } }