Merge pull request #57 from Flowpack/task/nodecreationCheckIfNodeIsAb…

…stractOrNotAllowedExplicitly TASK: NodeCreation: Check if node is abstract or if node is not allowed explicitly
Flowpack · Jun 14, 2023 · f9a3626 · f9a3626
2 parents 0ebcab1 + 86586a6
commit f9a3626
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);
+    }
 }