Merge remote-tracking branch 'origin/master' into feature/reintroduce…

…HiddenOrDisabledNodes
Flowpack · Jun 7, 2023 · 0f95072 · 0f95072
2 parents b99832e + a3684ba
commit 0f95072
Show file tree

Hide file tree

Showing 10 changed files with 72 additions and 24 deletions.
diff --git a/Classes/Domain/ExceptionHandling/ExceptionHandler.php b/Classes/Domain/ExceptionHandling/ExceptionHandler.php
@@ -31,9 +31,10 @@ class ExceptionHandler
     protected $throwableStorage;
 
     /**
+     * @var ExceptionHandlingConfiguration
      * @Flow\Inject
      */
-    protected ExceptionHandlingConfiguration $configuration;
+    protected $configuration;
 
     public function handleAfterTemplateConfigurationProcessing(CaughtExceptions $caughtExceptions, NodeInterface $node): void
     {

diff --git a/Classes/Domain/NodeCreation/PropertiesAndReferences.php b/Classes/Domain/NodeCreation/PropertiesAndReferences.php
@@ -53,10 +53,9 @@ public static function createFromArrayAndTypeDeclarations(array $propertiesAndRe
     public function requireValidProperties(NodeType $nodeType, CaughtExceptions $caughtExceptions): array
     {
         $validProperties = [];
-        $defaultValues = $nodeType->getDefaultValuesForProperties();
         foreach ($this->properties as $propertyName => $propertyValue) {
-            $this->assertValidPropertyName($propertyName);
             try {
+                $this->assertValidPropertyName($propertyName);
                 if (!isset($nodeType->getProperties()[$propertyName])) {
                     throw new PropertyIgnoredException(
                         sprintf(
@@ -109,14 +108,14 @@ public function requireValidReferences(NodeType $nodeType, Context $subgraph, Ca
 
     /**
      * In the old CR, it was common practice to set internal or meta properties via this syntax: `_hidden` but we don't allow this anymore.
-     * @throws \InvalidArgumentException
+     * @throws PropertyIgnoredException
      */
     private function assertValidPropertyName($propertyName): void
     {
         $legacyInternalProperties = ['_accessRoles', '_contentObject', '_hidden', '_hiddenAfterDateTime', '_hiddenBeforeDateTime', '_hiddenInIndex',
             '_index', '_name', '_nodeType', '_removed', '_workspace'];
         if (!is_string($propertyName) || $propertyName === '') {
-            throw new \InvalidArgumentException(sprintf('Property name must be a non empty string. Got "%s".', $propertyName));
+            throw new PropertyIgnoredException(sprintf('Because property name must be a non empty string. Got "%s".', $propertyName), 1686149518395);
         }
         if ($propertyName[0] === '_') {
             $lowerPropertyName = strtolower($propertyName);
@@ -125,7 +124,7 @@ private function assertValidPropertyName($propertyName): void
             }
             foreach ($legacyInternalProperties as $legacyInternalProperty) {
                 if ($lowerPropertyName === strtolower($legacyInternalProperty)) {
-                    throw new \InvalidArgumentException(sprintf('Internal legacy property "%s" not implement.', $propertyName));
+                    throw new PropertyIgnoredException(sprintf('Because internal legacy property "%s" not implement.', $propertyName), 1686149513158);
                 }
             }
         }

diff --git a/Classes/Domain/Template/RootTemplate.php b/Classes/Domain/Template/RootTemplate.php
@@ -3,8 +3,6 @@
 
 namespace Flowpack\NodeTemplates\Domain\Template;
 
-use Flowpack\NodeTemplates\Domain\Template\Template;
-use Flowpack\NodeTemplates\Domain\Template\Templates;
 use Neos\Flow\Annotations as Flow;
 
 /**
@@ -34,6 +32,11 @@ public function __construct(?bool $disabled, array $properties, Templates $child
         $this->childNodes = $childNodes;
     }
 
+    public static function empty(): self
+    {
+        return new RootTemplate(null, [], Templates::empty());
+    }
+
     public function getDisabled(): ?bool
     {
         return $this->disabled;

diff --git a/Classes/Domain/Template/Template.php b/Classes/Domain/Template/Template.php
@@ -3,9 +3,8 @@
 
 namespace Flowpack\NodeTemplates\Domain\Template;
 
-use Flowpack\NodeTemplates\Domain\Template\Templates;
-use Neos\ContentRepository\Domain\NodeType\NodeTypeName;
 use Neos\ContentRepository\Domain\NodeAggregate\NodeName;
+use Neos\ContentRepository\Domain\NodeType\NodeTypeName;
 use Neos\Flow\Annotations as Flow;
 
 /** @Flow\Proxy(false) */

diff --git a/Classes/Domain/Template/Templates.php b/Classes/Domain/Template/Templates.php
@@ -2,8 +2,6 @@
 
 namespace Flowpack\NodeTemplates\Domain\Template;
 
-use Flowpack\NodeTemplates\Domain\Template\RootTemplate;
-use Flowpack\NodeTemplates\Domain\Template\Template;
 use Neos\Flow\Annotations as Flow;
 
 /** @Flow\Proxy(false) */
@@ -53,7 +51,7 @@ public function toRootTemplate(): RootTemplate
                 $first->getChildNodes()
             );
         }
-        return new RootTemplate(null, [], Templates::empty());
+        return RootTemplate::empty();
     }
 
     public function jsonSerialize()

diff --git a/Classes/Domain/TemplateConfiguration/TemplateConfigurationProcessor.php b/Classes/Domain/TemplateConfiguration/TemplateConfigurationProcessor.php
@@ -7,7 +7,6 @@
 use Flowpack\NodeTemplates\Domain\Template\RootTemplate;
 use Flowpack\NodeTemplates\Domain\Template\Template;
 use Flowpack\NodeTemplates\Domain\Template\Templates;
-use Flowpack\NodeTemplates\Domain\TemplateConfiguration\EelEvaluationService;
 use Neos\ContentRepository\Domain\NodeAggregate\NodeName;
 use Neos\ContentRepository\Domain\NodeType\NodeTypeName;
 use Neos\Flow\Annotations as Flow;
@@ -31,12 +30,16 @@ class TemplateConfigurationProcessor
      */
     public function processTemplateConfiguration(array $configuration, array $evaluationContext, CaughtExceptions $caughtEvaluationExceptions): RootTemplate
     {
-        $templatePart = TemplatePart::createRoot(
-            $configuration,
-            $evaluationContext,
-            fn ($value, $evaluationContext) => $this->preprocessConfigurationValue($value, $evaluationContext),
-            $caughtEvaluationExceptions
-        );
+        try {
+            $templatePart = TemplatePart::createRoot(
+                $configuration,
+                $evaluationContext,
+                fn ($value, $evaluationContext) => $this->preprocessConfigurationValue($value, $evaluationContext),
+                $caughtEvaluationExceptions
+            );
+        } catch (StopBuildingTemplatePartException $e) {
+            return RootTemplate::empty();
+        }
         return $this->createTemplatesFromTemplatePart($templatePart)->toRootTemplate();
     }
 
@@ -91,7 +94,10 @@ private function createTemplateFromTemplatePart(TemplatePart $templatePart): Tem
         $processedProperties = [];
         foreach ($templatePart->getRawConfiguration('properties') ?? [] as $propertyName => $value) {
             if (!is_scalar($value) && !is_null($value)) {
-                throw new \InvalidArgumentException(sprintf('Template configuration properties can only hold int|float|string|bool|null. Property "%s" has type "%s"', $propertyName, gettype($value)), 1685725310730);
+                $templatePart->getCaughtExceptions()->add(CaughtException::fromException(
+                    new \RuntimeException(sprintf('Template configuration properties can only hold int|float|string|bool|null. Property "%s" has type "%s"', $propertyName, gettype($value)), 1685725310730)
+                ));
+                continue;
             }
             try {
                 $processedProperties[$propertyName] = $templatePart->processConfiguration(['properties', $propertyName]);
@@ -102,7 +108,11 @@ private function createTemplateFromTemplatePart(TemplatePart $templatePart): Tem
         // process the childNodes
         $childNodeTemplates = Templates::empty();
         foreach ($templatePart->getRawConfiguration('childNodes') ?? [] as $childNodeConfigurationPath => $_) {
-            $childNodeTemplatePart = $templatePart->withConfigurationByConfigurationPath(['childNodes', $childNodeConfigurationPath]);
+            try {
+                $childNodeTemplatePart = $templatePart->withConfigurationByConfigurationPath(['childNodes', $childNodeConfigurationPath]);
+            } catch (StopBuildingTemplatePartException $e) {
+                continue;
+            }
             $childNodeTemplates = $childNodeTemplates->merge($this->createTemplatesFromTemplatePart($childNodeTemplatePart));
         }
 

diff --git a/Classes/Domain/TemplateConfiguration/TemplatePart.php b/Classes/Domain/TemplateConfiguration/TemplatePart.php
@@ -43,6 +43,7 @@ class TemplatePart
      * @psalm-param array<string, mixed> $configuration
      * @psalm-param array<string, mixed> $evaluationContext
      * @psalm-param \Closure(mixed $value, array<string, mixed> $evaluationContext): mixed $configurationValueProcessor
+     * @throws StopBuildingTemplatePartException
      */
     private function __construct(
         array $configuration,
@@ -63,6 +64,7 @@ private function __construct(
      * @psalm-param array<string, mixed> $configuration
      * @psalm-param array<string, mixed> $evaluationContext
      * @psalm-param \Closure(mixed $value, array<string, mixed> $evaluationContext): mixed $configurationValueProcessor
+     * @throws StopBuildingTemplatePartException
      */
     public static function createRoot(
         array $configuration,
@@ -91,6 +93,7 @@ public function getFullPathToConfiguration(): array
 
     /**
      * @psalm-param string|list<string> $configurationPath
+     * @throws StopBuildingTemplatePartException
      */
     public function withConfigurationByConfigurationPath($configurationPath): self
     {
@@ -188,16 +191,25 @@ public function hasConfiguration($configurationPath): bool
         return true;
     }
 
+    /**
+     * @throws StopBuildingTemplatePartException
+     */
     private function validateTemplateConfigurationKeys(): void
     {
         $isRootTemplate = $this->fullPathToConfiguration === [];
         foreach (array_keys($this->configuration) as $key) {
             if (!in_array($key, ['type', 'name', 'disabled', 'properties', 'childNodes', 'when', 'withItems', 'withContext'], true)) {
-                throw new \InvalidArgumentException(sprintf('Template configuration has illegal key "%s"', $key));
+                $this->caughtExceptions->add(
+                    CaughtException::fromException(new \InvalidArgumentException(sprintf('Template configuration has illegal key "%s"', $key), 1686150349274))
+                );
+                throw new StopBuildingTemplatePartException();
             }
             if ($isRootTemplate) {
                 if (!in_array($key, ['disabled', 'properties', 'childNodes', 'when', 'withContext'], true)) {
-                    throw new \InvalidArgumentException(sprintf('Root template configuration doesnt allow option "%s', $key));
+                    $this->caughtExceptions->add(
+                        CaughtException::fromException(new \InvalidArgumentException(sprintf('Root template configuration doesnt allow option "%s', $key), 1686150340657))
+                    );
+                    throw new StopBuildingTemplatePartException();
                 }
             }
         }

diff --git a/Configuration/Testing/NodeTypes.Malformed.yaml b/Configuration/Testing/NodeTypes.Malformed.yaml
@@ -33,13 +33,19 @@
     template:
       properties:
         foo: "${cannotCallThis()}"
+        # legacy properties fail
+        _hidden: true
+        _hiddenAfterDateTime: 123
         boolValue: 123
         stringValue: false
         reference: "non-existing-node-id"
         references: "${['non-existing-node-id']}"
         working: "working"
         nonDeclaredProperty: "hi"
         bar: "${'left open"
+        # only simple scalar types
+        nonEelArrayNotAllowed:
+          not: allowed
       childNodes:
         whenAbort:
           type: 'Flowpack.NodeTemplates:Content.Text'
@@ -85,3 +91,5 @@
         typeCantMutate:
           name: "type-cant-mutate"
           type: "Flowpack.NodeTemplates:Content.WithEvaluationExceptions"
+        invalidOption:
+          crazy: me
diff --git a/Tests/Functional/Fixtures/WithEvaluationExceptions.messages.json b/Tests/Functional/Fixtures/WithEvaluationExceptions.messages.json
@@ -11,6 +11,10 @@
         "message": "Expression \"${'left open\" in \"properties.bar\" | EelException(The EEL expression \"${'left open\" was not a valid EEL expression. Perhaps you forgot to wrap it in ${...}?, 1410441849)",
         "severity": "ERROR"
     },
+    {
+        "message": "RuntimeException(Template configuration properties can only hold int|float|string|bool|null. Property \"nonEelArrayNotAllowed\" has type \"array\", 1685725310730)",
+        "severity": "ERROR"
+    },
     {
         "message": "Expression \"${parse \u00e4\u00fc\u00e4\u00f6 error}\" in \"childNodes.whenAbort.when\" | ParserException(Expression \"parse \u00e4\u00fc\u00e4\u00f6 error\" could not be parsed. Error starting at character 5: \" \u00e4\u00fc\u00e4\u00f6 error\"., 1327682383)",
         "severity": "ERROR"
@@ -43,6 +47,18 @@
         "message": "Configuration \"null\" in \"childNodes.withItemsAbortBecauseNotIterable.withItems\" | RuntimeException(Type NULL is not iterable., 1685802354186)",
         "severity": "ERROR"
     },
+    {
+        "message": "InvalidArgumentException(Template configuration has illegal key \"crazy\", 1686150349274)",
+        "severity": "ERROR"
+    },
+    {
+        "message": "Property \"_hidden\" in NodeType \"Flowpack.NodeTemplates:Content.WithEvaluationExceptions\" | PropertyIgnoredException(Because internal legacy property \"_hidden\" not implement., 1686149513158)",
+        "severity": "ERROR"
+    },
+    {
+        "message": "Property \"_hiddenAfterDateTime\" in NodeType \"Flowpack.NodeTemplates:Content.WithEvaluationExceptions\" | PropertyIgnoredException(Because internal legacy property \"_hiddenAfterDateTime\" not implement., 1686149513158)",
+        "severity": "ERROR"
+    },
     {
         "message": "Property \"boolValue\" in NodeType \"Flowpack.NodeTemplates:Content.WithEvaluationExceptions\" | PropertyIgnoredException(Because value `123` is not assignable to property type \"boolean\"., 1685958105644)",
         "severity": "ERROR"

diff --git a/Tests/Functional/Fixtures/WithEvaluationExceptions.template.json b/Tests/Functional/Fixtures/WithEvaluationExceptions.template.json
@@ -1,6 +1,8 @@
 {
     "disabled": null,
     "properties": {
+        "_hidden": true,
+        "_hiddenAfterDateTime": 123,
         "boolValue": 123,
         "stringValue": false,
         "reference": "non-existing-node-id",