TASK: NodeCreation: Check if node is abstract or if node is not allow…

…ed explicitly Backport of #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.
Flowpack · Jun 13, 2023 · 86586a6 · 86586a6
1 parent 0ebcab1
commit 86586a6
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
+    }
 }