Merge pull request #62 from Flowpack/bugfix/61-triggerPropertyMapping

BUGFIX: Use property mapping for `type: SomeClass` or `array<SomeClass>`

Classes/Domain/NodeCreation/NodeCreationService.php

@@ -8,11 +8,15 @@
 use Flowpack\NodeTemplates\Domain\Template\Template;
 use Flowpack\NodeTemplates\Domain\Template\Templates;
 use Neos\ContentRepository\Domain\Model\NodeInterface;
+use Neos\ContentRepository\Domain\Model\NodeType;
 use Neos\ContentRepository\Domain\Service\NodeTypeManager;
 use Neos\ContentRepository\Exception\NodeConstraintException;
 use Neos\Flow\Annotations as Flow;
+use Neos\Flow\Property\PropertyMapper;
+use Neos\Flow\Property\PropertyMappingConfiguration;
 use Neos\Neos\Service\NodeOperations;
 use Neos\Neos\Utility\NodeUriPathSegmentGenerator;
+use Neos\Flow\Property\Exception as PropertyWasNotMappedException;
 
 class NodeCreationService
 {
@@ -34,14 +38,20 @@ class NodeCreationService
      */
     protected $nodeUriPathSegmentGenerator;
 
+    /**
+     * @Flow\Inject
+     * @var PropertyMapper
+     */
+    protected $propertyMapper;
+
     /**
      * Applies the root template and its descending configured child node templates on the given node.
      * @throws \InvalidArgumentException
      */
     public function apply(RootTemplate $template, NodeInterface $node, CaughtExceptions $caughtExceptions): void
     {
         $nodeType = $node->getNodeType();
-        $propertiesAndReferences = PropertiesAndReferences::createFromArrayAndTypeDeclarations($template->getProperties(), $nodeType);
+        $propertiesAndReferences = PropertiesAndReferences::createFromArrayAndTypeDeclarations($this->convertProperties($nodeType, $template->getProperties(), $caughtExceptions), $nodeType);
 
         // set properties
         foreach ($propertiesAndReferences->requireValidProperties($nodeType, $caughtExceptions) as $key => $value) {
@@ -119,7 +129,7 @@ private function applyTemplateRecursively(Templates $templates, NodeInterface $p
                 }
             }
             $nodeType = $node->getNodeType();
-            $propertiesAndReferences = PropertiesAndReferences::createFromArrayAndTypeDeclarations($template->getProperties(), $nodeType);
+            $propertiesAndReferences = PropertiesAndReferences::createFromArrayAndTypeDeclarations($this->convertProperties($nodeType, $template->getProperties(), $caughtExceptions), $nodeType);
 
             // set properties
             foreach ($propertiesAndReferences->requireValidProperties($nodeType, $caughtExceptions) as $key => $value) {
@@ -153,4 +163,40 @@ private function ensureNodeHasUriPathSegment(NodeInterface $node, $template)
         }
         $node->setProperty('uriPathSegment', $this->nodeUriPathSegmentGenerator->generateUriPathSegment($node, $properties['title'] ?? null));
     }
+
+    private function convertProperties(NodeType $nodeType, array $properties, CaughtExceptions $caughtExceptions): array
+    {
+        // TODO combine with PropertiesAndReferences::requireValidProperties
+        foreach ($nodeType->getConfiguration('properties') as $propertyName => $propertyConfiguration) {
+            if (!isset($properties[$propertyName])) {
+                continue;
+            }
+            $propertyType = $nodeType->getPropertyType($propertyName);
+            if ($propertyType === 'references' || $propertyType === 'reference') {
+                continue;
+            }
+            $propertyType = PropertyType::fromPropertyOfNodeType($propertyName, $nodeType);
+            $propertyValue = $properties[$propertyName];
+            if (!$propertyType->isClass() && !$propertyType->isArrayOfClass()) {
+                // property mapping only for class types or array of classes!
+                continue;
+            }
+            try {
+                $propertyMappingConfiguration = new PropertyMappingConfiguration();
+                $propertyMappingConfiguration->allowAllProperties();
+
+                $properties[$propertyName] = $this->propertyMapper->convert($propertyValue, $propertyType->getValue(), $propertyMappingConfiguration);
+                $messages = $this->propertyMapper->getMessages();
+                if ($messages->hasErrors()) {
+                    throw new PropertyWasNotMappedException($this->propertyMapper->getMessages()->getFirstError()->getMessage(), 1686779371122);
+                }
+            } catch (PropertyWasNotMappedException $exception) {
+                $caughtExceptions->add(CaughtException::fromException(
+                    $exception
+                )->withOrigin(sprintf('Property "%s" in NodeType "%s"', $propertyName, $nodeType->getName())));
+                unset($properties[$propertyName]);
+            }
+        }
+        return $properties;
+    }
 }

Classes/Domain/NodeCreation/PropertyType.php

@@ -178,6 +178,19 @@ public function isArrayOf(): bool
         return (bool)preg_match(self::PATTERN_ARRAY_OF, $this->value);
     }
 
+    public function isArrayOfClass(): bool
+    {
+        return $this->isArrayOf() && $this->arrayOfType->isClass();
+    }
+
+    public function isClass(): bool
+    {
+        $className = $this->value[0] != '\\'
+            ? '\\' . $this->value
+            : $this->value;
+        return (class_exists($className) || interface_exists($className));
+    }
+
     public function isDate(): bool
     {
         return $this->value === self::TYPE_DATE;

Configuration/Testing/NodeTypes.ResolvablePropertyValues.yaml

@@ -0,0 +1,23 @@
+# Test, that asset ids are correctly resolved to asset objects (via the property mapper)
+# Also reference node id's should be correctly resolved
+---
+
+'Flowpack.NodeTemplates:Content.ResolvablePropertyValues':
+  superTypes:
+    'Neos.Neos:Content': true
+  properties:
+    asset:
+      type: Neos\Media\Domain\Model\Asset
+    images:
+      type: array<Neos\Media\Domain\Model\ImageInterface>
+    reference:
+      type: reference
+    references:
+      type: references
+  options:
+    template:
+      properties:
+        asset: 'c228200e-7472-4290-9936-4454a5b5692a'
+        reference: 'some-node-id'
+        references: "${['some-node-id', 'other-node-id', data.realNode]}"
+        images: "${['c8ae9f9f-dd11-4373-bf42-4bf31ec5bd19']}"

Configuration/Testing/NodeTypes.UnresolvablePropertyValues.yaml

@@ -0,0 +1,27 @@
+# We make sure that we dont trigger unwanted property mapping, so we wont allow an array in a string field.
+---
+
+'Flowpack.NodeTemplates:Content.UnresolvablePropertyValues':
+  superTypes:
+    'Neos.Neos:Content': true
+  ui:
+    label: UnresolvablePropertyValues
+  properties:
+    someString:
+      type: string
+    asset:
+      type: Neos\Media\Domain\Model\Asset
+    images:
+      type: array<Neos\Media\Domain\Model\ImageInterface>
+    reference:
+      type: reference
+    references:
+      type: references
+  options:
+    template:
+      properties:
+        someString: "${['foo']}"
+        reference: true
+        references: "${['some-non-existing-node-id']}"
+        asset: "non-existing"
+        images: "${['non-existing']}"

Tests/Functional/Fixtures/ResolvablePropertyValues.nodes.json

@@ -0,0 +1,26 @@
+{
+    "properties": {
+        "asset": "object(Neos\\Media\\Domain\\Model\\Asset, c228200e-7472-4290-9936-4454a5b5692a)",
+        "images": [
+            "object(Neos\\Media\\Domain\\Model\\Image, c8ae9f9f-dd11-4373-bf42-4bf31ec5bd19)"
+        ]
+    },
+    "references": {
+        "reference": [
+            {
+                "node": "Node(some-node-id, unstructured)"
+            }
+        ],
+        "references": [
+            {
+                "node": "Node(some-node-id, unstructured)"
+            },
+            {
+                "node": "Node(other-node-id, unstructured)"
+            },
+            {
+                "node": "Node(real-node-id, unstructured)"
+            }
+        ]
+    }
+}

Tests/Functional/Fixtures/ResolvablePropertyValues.template.json

@@ -0,0 +1,15 @@
+{
+    "properties": {
+        "asset": "c228200e-7472-4290-9936-4454a5b5692a",
+        "reference": "some-node-id",
+        "references": [
+            "some-node-id",
+            "other-node-id",
+            "Node(real-node-id, unstructured)"
+        ],
+        "images": [
+            "c8ae9f9f-dd11-4373-bf42-4bf31ec5bd19"
+        ]
+    },
+    "childNodes": []
+}

Tests/Functional/Fixtures/ResolvablePropertyValues.yaml

@@ -0,0 +1,8 @@
+'{nodeTypeName}':
+  options:
+    template:
+      properties:
+        # asset -> object(Neos\Media\Domain\Model\Asset)
+        # images -> array(Neos\Media\Domain\Model\Image)
+        # reference -> Reference of NodeTypes (Neos.Neos:Document) with value Node(some-node-id)
+        # references -> Nodes(some-node-id, other-node-id, real-node-id)

Tests/Functional/Fixtures/UnresolvablePropertyValues.messages.json

@@ -0,0 +1,26 @@
+[
+    {
+        "message": "Template for \"UnresolvablePropertyValues\" only partially applied. Please check the newly created nodes beneath Node \/sites\/test-site\/homepage\/main\/new-node@live[Flowpack.NodeTemplates:Content.UnresolvablePropertyValues].",
+        "severity": "ERROR"
+    },
+    {
+        "message": "Property \"asset\" in NodeType \"Flowpack.NodeTemplates:Content.UnresolvablePropertyValues\" | FlowException(Object of type \"Neos\\Media\\Domain\\Model\\Asset\" with identity \"non-existing\" not found., 1686779371122)",
+        "severity": "ERROR"
+    },
+    {
+        "message": "Property \"images\" in NodeType \"Flowpack.NodeTemplates:Content.UnresolvablePropertyValues\" | FlowException(Could not convert target type \"array<Neos\\Media\\Domain\\Model\\ImageInterface>\", at property path \"0\": No converter found which can be used to convert from \"string\" to \"Neos\\Media\\Domain\\Model\\ImageInterface\"., 1297759968) | TypeConverterException(No converter found which can be used to convert from \"string\" to \"Neos\\Media\\Domain\\Model\\ImageInterface\"., 0)",
+        "severity": "ERROR"
+    },
+    {
+        "message": "Property \"someString\" in NodeType \"Flowpack.NodeTemplates:Content.UnresolvablePropertyValues\" | PropertyIgnoredException(Because value `[\"foo\"]` is not assignable to property type \"string\"., 1685958105644)",
+        "severity": "ERROR"
+    },
+    {
+        "message": "Reference \"reference\" in NodeType \"Flowpack.NodeTemplates:Content.UnresolvablePropertyValues\" | RuntimeException(Reference could not be set, because node reference(s) true cannot be resolved., 1685958176560)",
+        "severity": "ERROR"
+    },
+    {
+        "message": "Reference \"references\" in NodeType \"Flowpack.NodeTemplates:Content.UnresolvablePropertyValues\" | RuntimeException(Reference could not be set, because node reference(s) [\"some-non-existing-node-id\"] cannot be resolved., 1685958176560)",
+        "severity": "ERROR"
+    }
+]

Tests/Functional/Fixtures/UnresolvablePropertyValues.nodes.json

@@ -0,0 +1 @@
+[]

Tests/Functional/Fixtures/UnresolvablePropertyValues.template.json

@@ -0,0 +1,16 @@
+{
+    "properties": {
+        "someString": [
+            "foo"
+        ],
+        "reference": true,
+        "references": [
+            "some-non-existing-node-id"
+        ],
+        "asset": "non-existing",
+        "images": [
+            "non-existing"
+        ]
+    },
+    "childNodes": []
+}

Tests/Functional/Fixtures/UnresolvablePropertyValues.yaml

@@ -0,0 +1,3 @@
+'{nodeTypeName}':
+  options:
+    template: []

Tests/Functional/JsonSerializeNodeTreeTrait.php

@@ -3,6 +3,7 @@
 namespace Flowpack\NodeTemplates\Tests\Functional;
 
 use Neos\ContentRepository\Domain\Model\NodeInterface;
+use Neos\Utility\ObjectAccess;
 
 trait JsonSerializeNodeTreeTrait
 {
@@ -51,7 +52,8 @@ private function serializeValuesInArray(array $array): array
                     $value = $this->serializeValuesInArray($value);
                 }
             } elseif (is_object($value)) {
-                $value = sprintf('object(%s)', get_class($value));
+                $id = ObjectAccess::getProperty($value, 'Persistence_Object_Identifier', true);
+                $value = sprintf('object(%s%s)', get_class($value), $id ? (sprintf(', %s', $id)) : '');
             } else {
                 continue;
             }