diff --git a/Classes/Domain/ExceptionHandling/ExceptionHandler.php b/Classes/Domain/ExceptionHandling/ExceptionHandler.php index 873d5ed..5fff117 100644 --- 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 index 9768349..98043d9 100644 --- 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 index a0a8ba1..6aba0c3 100644 --- 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 index b2bc39b..fdfcef1 100644 --- 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 index d57d41d..5d083fe 100644 --- 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 index 0143772..f2858dd 100644 --- 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 index adaae79..93e405c 100644 --- a/Classes/Domain/TemplateConfiguration/TemplatePart.php +++ b/Classes/Domain/TemplateConfiguration/TemplatePart.php @@ -43,6 +43,7 @@ class TemplatePart * @psalm-param array $configuration * @psalm-param array $evaluationContext * @psalm-param \Closure(mixed $value, array $evaluationContext): mixed $configurationValueProcessor + * @throws StopBuildingTemplatePartException */ private function __construct( array $configuration, @@ -63,6 +64,7 @@ private function __construct( * @psalm-param array $configuration * @psalm-param array $evaluationContext * @psalm-param \Closure(mixed $value, array $evaluationContext): mixed $configurationValueProcessor + * @throws StopBuildingTemplatePartException */ public static function createRoot( array $configuration, @@ -91,6 +93,7 @@ public function getFullPathToConfiguration(): array /** * @psalm-param string|list $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 index d1268fa..1d5cd1e 100644 --- a/Configuration/Testing/NodeTypes.Malformed.yaml +++ b/Configuration/Testing/NodeTypes.Malformed.yaml @@ -33,6 +33,9 @@ template: properties: foo: "${cannotCallThis()}" + # legacy properties fail + _hidden: true + _hiddenAfterDateTime: 123 boolValue: 123 stringValue: false reference: "non-existing-node-id" @@ -40,6 +43,9 @@ 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 index 58cfa11..0e0b812 100644 --- 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 index 74a5192..d802cb0 100644 --- 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",