From 281165423da2caabc96b6e099be2ece8be79a47c Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 20 Feb 2024 07:03:49 +0100 Subject: [PATCH 1/5] BUGFIX: `defaultValue` of `DateTime` to treated immutable the property should be a `DateTimeImmutable` and thus NOT mutable - Add test for that `type: DateTime` is now a `DateTimeImmutable` --- ...ComplexDefaultAndInitialProperties.feature | 14 ++++++++++- .../Dto/SerializedPropertyValues.php | 23 ++++++------------- .../Features/Bootstrap/ProjectedNodeTrait.php | 15 ++++++++++++ 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature index 99ece1dbca9..26120dc84ba 100644 --- a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature +++ b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature @@ -18,7 +18,7 @@ Feature: Create a node aggregate with complex default values type: Neos\ContentRepository\Core\Tests\Behavior\Fixtures\DayOfWeek defaultValue: 'https://schema.org/Wednesday' now: - type: DateTimeImmutable + type: DateTime defaultValue: 'now' date: type: DateTimeImmutable @@ -66,6 +66,18 @@ Feature: Create a node aggregate with complex default values | parentNodeAggregateId | "lady-eleonode-rootford" | And the graph projection is fully up to date Then I expect a node identified by cs-identifier;nody-mc-nodeface;{} to exist in the content graph + + And I expect this node to have the following serialized property types: + | Key | Type | + | array | array | + | dayOfWeek | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\DayOfWeek | + | postalAddress | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PostalAddress | + # DateTime must always be treated as immutable see DateTimeImmutable + | now | DateTimeImmutable | + | date | DateTimeImmutable | + | uri | GuzzleHttp\Psr7\Uri | + | price | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PriceSpecification | + And I expect this node to have the following properties: | Key | Value | | array | {"givenName":"Nody", "familyName":"McNodeface"} | diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php index 31f825e171b..9fa790ce32d 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php @@ -14,6 +14,7 @@ namespace Neos\ContentRepository\Core\Feature\NodeModification\Dto; +use Neos\ContentRepository\Core\Infrastructure\Property\PropertyType; use Neos\ContentRepository\Core\NodeType\NodeType; use Neos\ContentRepository\Core\SharedModel\Node\PropertyName; use Neos\ContentRepository\Core\SharedModel\Node\PropertyNames; @@ -73,14 +74,13 @@ public static function defaultFromNodeType(NodeType $nodeType): self $defaultValue = json_encode($defaultValue); } - $propertyTypeFromSchema = $nodeType->getPropertyType($propertyName); - self::assertTypeIsNoReference($propertyTypeFromSchema); - - if ($defaultValue === null) { - continue; - } + $propertyType = PropertyType::fromNodeTypeDeclaration( + $nodeType->getPropertyType($propertyName), + PropertyName::fromString($propertyName), + $nodeType->name + ); - $values[$propertyName] = SerializedPropertyValue::create($defaultValue, $propertyTypeFromSchema); + $values[$propertyName] = SerializedPropertyValue::create($defaultValue, $propertyType->getSerializationType()); } return new self($values); @@ -91,15 +91,6 @@ public static function fromJsonString(string $jsonString): self return self::fromArray(\json_decode($jsonString, true)); } - private static function assertTypeIsNoReference(string $propertyTypeFromSchema): void - { - if ($propertyTypeFromSchema === 'reference' || $propertyTypeFromSchema === 'references') { - throw new \RuntimeException( - 'TODO: references cannot be serialized; you need to use the SetNodeReferences command instead.' - ); - } - } - public function merge(self $other): self { return new self(array_merge($this->values, $other->values)); diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php index 5ba244abcd9..fd4118cbc85 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php @@ -255,6 +255,21 @@ public function iExpectThisNodeToBeUnnamed(): void }); } + /** + * @Then /^I expect this node to have the following serialized property types:$/ + * @param TableNode $expectedPropertyTypes + */ + public function iExpectThisNodeToHaveTheFollowingSerializedPropertyTypes(TableNode $expectedPropertyTypes): void + { + $this->assertOnCurrentNode(function (Node $currentNode) use ($expectedPropertyTypes) { + $serialized = $currentNode->properties->serialized(); + foreach ($expectedPropertyTypes->getHash() as $row) { + Assert::assertTrue($serialized->propertyExists($row['Key']), 'Property "' . $row['Key'] . '" not found'); + Assert::assertEquals($row['Type'], $serialized->getProperty($row['Key'])->type, 'Serialized node property ' . $row['Key'] . ' does not match expected type.'); + } + }); + } + /** * @Then /^I expect this node to have the following properties:$/ * @param TableNode $expectedProperties From ce956e0444a63b27187784e69febc6e2f4425ebb Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 20 Feb 2024 08:44:15 +0100 Subject: [PATCH 2/5] BUGFIX: `defaultValue` must be deserialized and serialize before written That will lead to relative `DateTime` values like `now` reflecting the actual time when the command was handled, rather glitching around and reflecting the date when being fetched. --- .../Feature/Common/TetheredNodeInternals.php | 7 +++- .../Feature/NodeCreation/NodeCreation.php | 4 +- .../Dto/SerializedPropertyValues.php | 17 +++++++-- .../RootNodeCreation/RootNodeHandling.php | 2 +- .../Property/PropertyConverter.php | 37 ++++++++----------- .../Adjustment/TetheredNodeAdjustments.php | 7 ++++ .../src/StructureAdjustmentService.php | 3 ++ .../src/StructureAdjustmentServiceFactory.php | 1 + .../Features/Bootstrap/ProjectedNodeTrait.php | 4 ++ .../Classes/Unit/NodeSubjectProvider.php | 10 +---- 10 files changed, 54 insertions(+), 38 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php b/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php index ba8ff204f27..099e0428efb 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php +++ b/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php @@ -19,6 +19,7 @@ use Neos\ContentRepository\Core\Feature\NodeCreation\Event\NodeAggregateWithNodeWasCreated; use Neos\ContentRepository\Core\Feature\NodeModification\Dto\SerializedPropertyValues; use Neos\ContentRepository\Core\Feature\NodeVariation\Event\NodePeerVariantWasCreated; +use Neos\ContentRepository\Core\Infrastructure\Property\PropertyConverter; use Neos\ContentRepository\Core\Projection\ContentGraph\Node; use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateClassification; @@ -36,6 +37,8 @@ trait TetheredNodeInternals { use NodeVariationInternals; + abstract protected function getPropertyConverter(): PropertyConverter; + abstract protected function createEventsForVariations( ContentStreamId $contentStreamId, OriginDimensionSpacePoint $sourceOrigin, @@ -100,7 +103,7 @@ protected function createEventsForMissingTetheredNode( $this->getInterDimensionalVariationGraph()->getSpecializationSet($rootGeneralization), $parentNodeAggregate->nodeAggregateId, $tetheredNodeName, - SerializedPropertyValues::defaultFromNodeType($expectedTetheredNodeType), + SerializedPropertyValues::defaultFromNodeType($expectedTetheredNodeType, $this->getPropertyConverter()), NodeAggregateClassification::CLASSIFICATION_TETHERED, ); $creationOriginDimensionSpacePoint = $rootGeneralizationOrigin; @@ -117,7 +120,7 @@ protected function createEventsForMissingTetheredNode( $parentNodeAggregate->getCoverageByOccupant($originDimensionSpacePoint), $parentNodeAggregate->nodeAggregateId, $tetheredNodeName, - SerializedPropertyValues::defaultFromNodeType($expectedTetheredNodeType), + SerializedPropertyValues::defaultFromNodeType($expectedTetheredNodeType, $this->getPropertyConverter()), NodeAggregateClassification::CLASSIFICATION_TETHERED, ) ); diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php index 23e08875d69..593b7ca1809 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php @@ -224,7 +224,7 @@ private function handleCreateNodeAggregateWithNodeAndSerializedProperties( ); } - $defaultPropertyValues = SerializedPropertyValues::defaultFromNodeType($nodeType); + $defaultPropertyValues = SerializedPropertyValues::defaultFromNodeType($nodeType, $this->getPropertyConverter()); $initialPropertyValues = $defaultPropertyValues->merge($command->initialPropertyValues); $events = [ @@ -294,7 +294,7 @@ private function handleTetheredChildNodes( : NodePath::fromString($nodeName->value); $childNodeAggregateId = $nodeAggregateIds->getNodeAggregateId($childNodePath) ?? NodeAggregateId::create(); - $initialPropertyValues = SerializedPropertyValues::defaultFromNodeType($childNodeType); + $initialPropertyValues = SerializedPropertyValues::defaultFromNodeType($childNodeType, $this->getPropertyConverter()); $this->requireContentStreamToExist($command->contentStreamId, $contentRepository); $events[] = $this->createTetheredWithNode( diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php index 9fa790ce32d..8d88a40420e 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php @@ -14,6 +14,7 @@ namespace Neos\ContentRepository\Core\Feature\NodeModification\Dto; +use Neos\ContentRepository\Core\Infrastructure\Property\PropertyConverter; use Neos\ContentRepository\Core\Infrastructure\Property\PropertyType; use Neos\ContentRepository\Core\NodeType\NodeType; use Neos\ContentRepository\Core\SharedModel\Node\PropertyName; @@ -66,7 +67,8 @@ public static function fromArray(array $propertyValues): self return new self($values); } - public static function defaultFromNodeType(NodeType $nodeType): self + /** @internal */ + public static function defaultFromNodeType(NodeType $nodeType, PropertyConverter $propertyConverter): self { $values = []; foreach ($nodeType->getDefaultValuesForProperties() as $propertyName => $defaultValue) { @@ -79,8 +81,17 @@ public static function defaultFromNodeType(NodeType $nodeType): self PropertyName::fromString($propertyName), $nodeType->name ); - - $values[$propertyName] = SerializedPropertyValue::create($defaultValue, $propertyType->getSerializationType()); + $deserializedDefaultValue = $propertyConverter->deserializePropertyValue( + SerializedPropertyValue::create($defaultValue, $propertyType->getSerializationType()) + ); + $values[$propertyName] = $propertyConverter->serializePropertyValue( + PropertyType::fromNodeTypeDeclaration( + $nodeType->getPropertyType($propertyName), + PropertyName::fromString($propertyName), + $nodeType->name + ), + $deserializedDefaultValue + ); } return new self($values); diff --git a/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php b/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php index 20de19bc5c6..1c5a2dbc6ae 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php +++ b/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php @@ -200,7 +200,7 @@ private function handleTetheredRootChildNodes( : NodePath::fromString($nodeName->value); $childNodeAggregateId = $nodeAggregateIdsByNodePath->getNodeAggregateId($childNodePath) ?? NodeAggregateId::create(); - $initialPropertyValues = SerializedPropertyValues::defaultFromNodeType($childNodeType); + $initialPropertyValues = SerializedPropertyValues::defaultFromNodeType($childNodeType, $this->getPropertyConverter()); $this->requireContentStreamToExist($command->contentStreamId, $contentRepository); $events[] = $this->createTetheredWithNodeForRoot( diff --git a/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php b/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php index 95616081f6c..8cde51c9680 100644 --- a/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php +++ b/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php @@ -16,7 +16,6 @@ use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName; use Neos\ContentRepository\Core\NodeType\NodeType; -use Neos\ContentRepository\Core\NodeType\NodeTypeName; use Neos\ContentRepository\Core\Feature\NodeModification\Dto\PropertyValuesToWrite; use Neos\ContentRepository\Core\SharedModel\Node\PropertyName; use Neos\ContentRepository\Core\Feature\NodeModification\Dto\SerializedPropertyValue; @@ -45,9 +44,11 @@ public function serializePropertyValues( foreach ($propertyValuesToWrite->values as $propertyName => $propertyValue) { $serializedPropertyValues[$propertyName] = $this->serializePropertyValue( - $nodeType->getPropertyType($propertyName), - PropertyName::fromString($propertyName), - $nodeType->name, + PropertyType::fromNodeTypeDeclaration( + $nodeType->getPropertyType($propertyName), + PropertyName::fromString($propertyName), + $nodeType->name + ), $propertyValue ); } @@ -55,22 +56,14 @@ public function serializePropertyValues( return SerializedPropertyValues::fromArray($serializedPropertyValues); } - private function serializePropertyValue( - string $declaredType, - PropertyName $propertyName, - NodeTypeName $nodeTypeName, + public function serializePropertyValue( + PropertyType $propertyType, mixed $propertyValue ): SerializedPropertyValue { - $propertyType = PropertyType::fromNodeTypeDeclaration( - $declaredType, - $propertyName, - $nodeTypeName - ); - if ($propertyValue === null) { // should not happen, as we must separate regular properties and unsets beforehand! throw new \RuntimeException( - sprintf('Property %s with value "null" cannot be serialized as unsets are treated differently.', $propertyName->value), + sprintf('Property type %s with value "null" cannot be serialized as unsets are treated differently.', $propertyType->value), 1707578784 ); } @@ -80,7 +73,7 @@ private function serializePropertyValue( } catch (NotEncodableValueException | NotNormalizableValueException $e) { // todo add custom exception class throw new \RuntimeException( - sprintf('There was a problem serializing property %s with value "%s".', $propertyName->value, get_debug_type($propertyValue)), + sprintf('There was a problem serializing property type %s with value "%s".', $propertyType->value, get_debug_type($propertyValue)), 1594842314, $e ); @@ -88,14 +81,14 @@ private function serializePropertyValue( if ($serializedPropertyValue === null) { throw new \RuntimeException( - sprintf('While serializing property %s with value "%s" the serializer returned not allowed value "null".', $propertyName->value, get_debug_type($propertyValue)), + sprintf('While serializing property type %s with value "%s" the serializer returned not allowed value "null".', $propertyType->value, get_debug_type($propertyValue)), 1707578784 ); } return SerializedPropertyValue::create( $serializedPropertyValue, - $propertyType->value + $propertyType->getSerializationType() ); } @@ -110,9 +103,11 @@ public function serializeReferencePropertyValues( $declaredType = $nodeType->getProperties()[$referenceName->value]['properties'][$propertyName]['type']; $serializedPropertyValues[$propertyName] = $this->serializePropertyValue( - $declaredType, - PropertyName::fromString($propertyName), - $nodeType->name, + PropertyType::fromNodeTypeDeclaration( + $declaredType, + PropertyName::fromString($propertyName), + $nodeType->name, + ), $propertyValue ); } diff --git a/Neos.ContentRepository.StructureAdjustment/src/Adjustment/TetheredNodeAdjustments.php b/Neos.ContentRepository.StructureAdjustment/src/Adjustment/TetheredNodeAdjustments.php index c4dc2eebf33..003680ae0ef 100644 --- a/Neos.ContentRepository.StructureAdjustment/src/Adjustment/TetheredNodeAdjustments.php +++ b/Neos.ContentRepository.StructureAdjustment/src/Adjustment/TetheredNodeAdjustments.php @@ -9,6 +9,7 @@ use Neos\ContentRepository\Core\EventStore\EventsToPublish; use Neos\ContentRepository\Core\Feature\NodeMove\Dto\CoverageNodeMoveMapping; use Neos\ContentRepository\Core\Feature\NodeMove\Dto\CoverageNodeMoveMappings; +use Neos\ContentRepository\Core\Infrastructure\Property\PropertyConverter; use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter; use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate; use Neos\ContentRepository\Core\Projection\ContentGraph\NodePath; @@ -41,6 +42,7 @@ public function __construct( private readonly ProjectedNodeIterator $projectedNodeIterator, private readonly NodeTypeManager $nodeTypeManager, private readonly DimensionSpace\InterDimensionalVariationGraph $interDimensionalVariationGraph, + private readonly PropertyConverter $propertyConverter ) { } @@ -211,6 +213,11 @@ protected function getInterDimensionalVariationGraph(): DimensionSpace\InterDime return $this->interDimensionalVariationGraph; } + protected function getPropertyConverter(): PropertyConverter + { + return $this->propertyConverter; + } + /** * array key: name of tethered child node. Value: the Node itself. * @param array $actualTetheredChildNodes diff --git a/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentService.php b/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentService.php index f816d330232..3f50d52afe7 100644 --- a/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentService.php +++ b/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentService.php @@ -9,6 +9,7 @@ use Neos\ContentRepository\Core\EventStore\EventPersister; use Neos\ContentRepository\Core\EventStore\EventsToPublish; use Neos\ContentRepository\Core\Factory\ContentRepositoryServiceInterface; +use Neos\ContentRepository\Core\Infrastructure\Property\PropertyConverter; use Neos\ContentRepository\Core\NodeType\NodeTypeManager; use Neos\ContentRepository\Core\NodeType\NodeTypeName; use Neos\ContentRepository\StructureAdjustment\Adjustment\DimensionAdjustment; @@ -32,6 +33,7 @@ public function __construct( private readonly EventPersister $eventPersister, NodeTypeManager $nodeTypeManager, InterDimensionalVariationGraph $interDimensionalVariationGraph, + PropertyConverter $propertyConverter ) { $projectedNodeIterator = new ProjectedNodeIterator( $contentRepository->getWorkspaceFinder(), @@ -43,6 +45,7 @@ public function __construct( $projectedNodeIterator, $nodeTypeManager, $interDimensionalVariationGraph, + $propertyConverter ); $this->unknownNodeTypeAdjustment = new UnknownNodeTypeAdjustment( diff --git a/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentServiceFactory.php b/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentServiceFactory.php index 8d835c29b51..6c1c9597da5 100644 --- a/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentServiceFactory.php +++ b/Neos.ContentRepository.StructureAdjustment/src/StructureAdjustmentServiceFactory.php @@ -20,6 +20,7 @@ public function build(ContentRepositoryServiceFactoryDependencies $serviceFactor $serviceFactoryDependencies->eventPersister, $serviceFactoryDependencies->nodeTypeManager, $serviceFactoryDependencies->interDimensionalVariationGraph, + $serviceFactoryDependencies->propertyConverter ); } } diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php index fd4118cbc85..69ca326691f 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php @@ -283,6 +283,10 @@ public function iExpectThisNodeToHaveTheFollowingProperties(TableNode $expectedP $expectedPropertyValue = $this->resolvePropertyValue($row['Value']); $actualPropertyValue = $properties->offsetGet($row['Key']); if ($row['Value'] === 'Date:now') { + // special case as It's hard to work with relative times. We only handle `now` right now (pun intended) but this or similar handling would also be required for `yesterday` or `after rep tv` + // as It's hard to check otherwise, we have to verify that `now` was not actually saved as string `now` but as timestamp when it was created + $serializedValue = $currentNode->properties->serialized()->getProperty($row['Key'])?->value; + Assert::assertNotEquals('now', $serializedValue, 'Relative DateTime must be serialized as absolute time in the events/serialized-properties'); // we accept 10s offset for the projector to be fine Assert::assertLessThan($actualPropertyValue, $expectedPropertyValue->sub(new \DateInterval('PT10S')), 'Node property ' . $row['Key'] . ' does not match. Expected: ' . json_encode($expectedPropertyValue) . '; Actual: ' . json_encode($actualPropertyValue)); } else { diff --git a/Neos.ContentRepository.TestSuite/Classes/Unit/NodeSubjectProvider.php b/Neos.ContentRepository.TestSuite/Classes/Unit/NodeSubjectProvider.php index eb631b1d3c1..80cd87080db 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Unit/NodeSubjectProvider.php +++ b/Neos.ContentRepository.TestSuite/Classes/Unit/NodeSubjectProvider.php @@ -17,7 +17,6 @@ use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint; use Neos\ContentRepository\Core\Factory\ContentRepositoryId; -use Neos\ContentRepository\Core\Feature\NodeModification\Dto\SerializedPropertyValue; use Neos\ContentRepository\Core\Feature\NodeModification\Dto\SerializedPropertyValues; use Neos\ContentRepository\Core\Infrastructure\Property\Normalizer\ArrayNormalizer; use Neos\ContentRepository\Core\Infrastructure\Property\Normalizer\CollectionTypeDenormalizer; @@ -85,14 +84,7 @@ public function createMinimalNodeOfType( SerializedPropertyValues $propertyValues = null, ?NodeName $nodeName = null ): Node { - $defaultPropertyValues = []; - foreach ($nodeType->getDefaultValuesForProperties() as $propertyName => $propertyValue) { - $defaultPropertyValues[$propertyName] = SerializedPropertyValue::create( - $propertyValue, - $nodeType->getPropertyType($propertyName) - ); - } - $serializedDefaultPropertyValues = SerializedPropertyValues::fromArray($defaultPropertyValues); + $serializedDefaultPropertyValues = SerializedPropertyValues::defaultFromNodeType($nodeType, $this->propertyConverter); return Node::create( ContentSubgraphIdentity::create( ContentRepositoryId::fromString('default'), From 26ce195d714c60265525d309773e4ba82db281d6 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 20 Feb 2024 09:12:17 +0100 Subject: [PATCH 3/5] TASK: Remove duplicate obsolete defaultValues validation from `handleCreateNodeAggregateWithNode` This is now part of the inner `handleCreateNodeAggregateWithNodeAndSerializedProperties` via `SerializedPropertyValues::defaultFromNodeType` --- ...ComplexDefaultAndInitialProperties.feature | 13 ++++++++++ .../Feature/NodeCreation/NodeCreation.php | 25 ------------------- .../Property/PropertyConverter.php | 16 +++++++++--- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature index 26120dc84ba..049d3cae29c 100644 --- a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature +++ b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature @@ -39,6 +39,11 @@ Feature: Create a node aggregate with complex default values price: 13.37 priceCurrency: 'EUR' + 'Neos.ContentRepository.Testing:FaultyDateNode': + properties: + date: + type: DateTimeImmutable + defaultValue: 'not a date' """ And using identifier "default", I define a content repository And I am in content repository "default" @@ -106,3 +111,11 @@ Feature: Create a node aggregate with complex default values | date | Date:2021-03-13T17:33:17+00:00 | | uri | URI:https://www.neos.io | | price | PriceSpecification:anotherDummy | + + Scenario: Create a node aggregate with faulty date time defaultValue fails + When the command CreateNodeAggregateWithNode is executed with payload and exceptions are caught: + | Key | Value | + | nodeAggregateId | "nody-mc-nodeface" | + | nodeTypeName | "Neos.ContentRepository.Testing:FaultyDateNode" | + | parentNodeAggregateId | "lady-eleonode-rootford" | + And the last command should have thrown an exception of type "RuntimeException" with code 1708416598 diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php index 593b7ca1809..2609c3b41ae 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php @@ -69,10 +69,6 @@ private function handleCreateNodeAggregateWithNode( ContentRepository $contentRepository ): EventsToPublish { $this->requireNodeType($command->nodeTypeName); - $this->validateProperties( - $this->deserializeDefaultProperties($command->nodeTypeName), - $command->nodeTypeName - ); $this->validateProperties($command->initialPropertyValues, $command->nodeTypeName); $lowLevelCommand = CreateNodeAggregateWithNodeAndSerializedProperties::create( @@ -95,27 +91,6 @@ private function handleCreateNodeAggregateWithNode( return $this->handleCreateNodeAggregateWithNodeAndSerializedProperties($lowLevelCommand, $contentRepository); } - private function deserializeDefaultProperties(NodeTypeName $nodeTypeName): PropertyValuesToWrite - { - $nodeType = $this->nodeTypeManager->getNodeType($nodeTypeName); - $defaultValues = []; - foreach ($nodeType->getDefaultValuesForProperties() as $propertyName => $defaultValue) { - $propertyType = PropertyType::fromNodeTypeDeclaration( - $nodeType->getPropertyType($propertyName), - PropertyName::fromString($propertyName), - $nodeTypeName - ); - - $serializedPropertyValue = SerializedPropertyValue::create($defaultValue, $propertyType->getSerializationType()); - - $defaultValues[$propertyName] = $this->getPropertyConverter()->deserializePropertyValue( - $serializedPropertyValue - ); - } - - return PropertyValuesToWrite::fromArray($defaultValues); - } - private function validateProperties(?PropertyValuesToWrite $propertyValues, NodeTypeName $nodeTypeName): void { if (!$propertyValues) { diff --git a/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php b/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php index 8cde51c9680..a7ea583b5b5 100644 --- a/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php +++ b/Neos.ContentRepository.Core/Classes/Infrastructure/Property/PropertyConverter.php @@ -117,9 +117,17 @@ public function serializeReferencePropertyValues( public function deserializePropertyValue(SerializedPropertyValue $serializedPropertyValue): mixed { - return $this->serializer->denormalize( - $serializedPropertyValue->value, - $serializedPropertyValue->type - ); + try { + return $this->serializer->denormalize( + $serializedPropertyValue->value, + $serializedPropertyValue->type + ); + } catch (NotNormalizableValueException $e) { + throw new \RuntimeException( + sprintf('TODO: There was a problem deserializing %s', json_encode($serializedPropertyValue)), + 1708416598, + $e + ); + } } } From 41bc8866f36ad0ce92635d7b4ce8d87efa2ff5fc Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 20 Feb 2024 09:15:28 +0100 Subject: [PATCH 4/5] TASK: Remove obsolete special handling for `DateTimeInterface` With #4251 `NodeType::getDefaultValuesForProperties` doesnt return a `DateTime` anymore for dateTime default values --- .../NodeModification/Dto/SerializedPropertyValues.php | 4 ---- Neos.ContentRepository.Core/Classes/NodeType/NodeType.php | 2 +- .../src/Adjustment/PropertyAdjustment.php | 7 ------- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php index 8d88a40420e..e8b3d764151 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php @@ -72,10 +72,6 @@ public static function defaultFromNodeType(NodeType $nodeType, PropertyConverter { $values = []; foreach ($nodeType->getDefaultValuesForProperties() as $propertyName => $defaultValue) { - if ($defaultValue instanceof \DateTimeInterface) { - $defaultValue = json_encode($defaultValue); - } - $propertyType = PropertyType::fromNodeTypeDeclaration( $nodeType->getPropertyType($propertyName), PropertyName::fromString($propertyName), diff --git a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php index 6d6369eb3e8..ef73a8a8ed9 100644 --- a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php +++ b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php @@ -433,7 +433,7 @@ public function getPropertyType(string $propertyName): string * * The default value is configured for each property under the "default" key. * - * @return array + * @return array> * @api */ public function getDefaultValuesForProperties(): array diff --git a/Neos.ContentRepository.StructureAdjustment/src/Adjustment/PropertyAdjustment.php b/Neos.ContentRepository.StructureAdjustment/src/Adjustment/PropertyAdjustment.php index 994a562a607..72a7d3e5479 100644 --- a/Neos.ContentRepository.StructureAdjustment/src/Adjustment/PropertyAdjustment.php +++ b/Neos.ContentRepository.StructureAdjustment/src/Adjustment/PropertyAdjustment.php @@ -78,13 +78,6 @@ public function findAdjustmentsForNodeType(NodeTypeName $nodeTypeName): \Generat // detect missing default values foreach ($nodeType->getDefaultValuesForProperties() as $propertyKey => $defaultValue) { - if ($defaultValue instanceof \DateTimeInterface) { - $defaultValue = json_encode($defaultValue); - } - if ($defaultValue === null) { - // we don't need to set null as default value if it doesn't exist - continue; - } if (!array_key_exists($propertyKey, $propertyKeysInNode)) { yield StructureAdjustment::createForNode( $node, From 3117ab00715d822148c2459a507620e675eb2251 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 3 Mar 2024 20:39:29 +0100 Subject: [PATCH 5/5] TASK: Improve tests and documentation about double defaultValue property conversion --- ...ComplexDefaultAndInitialProperties.feature | 22 ++++++++++--------- .../Dto/SerializedPropertyValues.php | 8 ++++++- .../Behavior/Fixtures/PriceSpecification.php | 12 ++++++---- .../Features/Bootstrap/ProjectedNodeTrait.php | 10 ++++++++- 4 files changed, 36 insertions(+), 16 deletions(-) diff --git a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature index 049d3cae29c..99bd1294925 100644 --- a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature +++ b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/02-NodeCreation/05-CreateNodeAggregateWithNode_ComplexDefaultAndInitialProperties.feature @@ -72,16 +72,18 @@ Feature: Create a node aggregate with complex default values And the graph projection is fully up to date Then I expect a node identified by cs-identifier;nody-mc-nodeface;{} to exist in the content graph - And I expect this node to have the following serialized property types: - | Key | Type | - | array | array | - | dayOfWeek | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\DayOfWeek | - | postalAddress | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PostalAddress | - # DateTime must always be treated as immutable see DateTimeImmutable - | now | DateTimeImmutable | - | date | DateTimeImmutable | - | uri | GuzzleHttp\Psr7\Uri | - | price | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PriceSpecification | + And I expect this node to have the following serialized properties: + | Key | Type | Value | + | array | array | {"givenName":"Nody","familyName":"McNodeface"} | + | dayOfWeek | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\DayOfWeek | "https://schema.org/Wednesday" | + | postalAddress | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PostalAddress | {"streetAddress":"28 31st of February Street","postalCode":12345,"addressLocality":"City","addressCountry":"Country"} | + # DateTime must always be treated as immutable see DateTimeImmutable. + # And the default value "now" must not be serialized as string "now" but as its actual value of the time of the command: + | now | DateTimeImmutable | NOT:"now" | + | date | DateTimeImmutable | "2020-08-20T18:56:15+00:00" | + | uri | GuzzleHttp\Psr7\Uri | "https://neos.io" | + # Defaults while deserializing value objects will be manifested at the time of the command: (valueAddedTaxIncluded was not explicitly declared above) + | price | Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PriceSpecification | {"price":13.37,"priceCurrency":"EUR","valueAddedTaxIncluded":true} | And I expect this node to have the following properties: | Key | Value | diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php index e8b3d764151..7aa73103f95 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/SerializedPropertyValues.php @@ -80,7 +80,12 @@ public static function defaultFromNodeType(NodeType $nodeType, PropertyConverter $deserializedDefaultValue = $propertyConverter->deserializePropertyValue( SerializedPropertyValue::create($defaultValue, $propertyType->getSerializationType()) ); - $values[$propertyName] = $propertyConverter->serializePropertyValue( + // The $defaultValue and $properlySerializedDefaultValue will likely equal, but in some cases diverge. + // For example relative date time default values like "now" will herby be serialized to the current date. + // Also, custom value objects might serialize slightly different, but more "correct" + // (by for example adding default values for undeclared properties) + // Additionally due the double conversion, we guarantee that a valid property converted exists at this time. + $properlySerializedDefaultValue = $propertyConverter->serializePropertyValue( PropertyType::fromNodeTypeDeclaration( $nodeType->getPropertyType($propertyName), PropertyName::fromString($propertyName), @@ -88,6 +93,7 @@ public static function defaultFromNodeType(NodeType $nodeType, PropertyConverter ), $deserializedDefaultValue ); + $values[$propertyName] = $properlySerializedDefaultValue; } return new self($values); diff --git a/Neos.ContentRepository.Core/Tests/Behavior/Fixtures/PriceSpecification.php b/Neos.ContentRepository.Core/Tests/Behavior/Fixtures/PriceSpecification.php index f8831a22926..71f53412115 100644 --- a/Neos.ContentRepository.Core/Tests/Behavior/Fixtures/PriceSpecification.php +++ b/Neos.ContentRepository.Core/Tests/Behavior/Fixtures/PriceSpecification.php @@ -22,7 +22,8 @@ { private function __construct( public float $price, - public string $priceCurrency + public string $priceCurrency, + public bool $valueAddedTaxIncluded ) { } @@ -30,7 +31,8 @@ public static function fromArray(array $array): self { return new self( $array['price'], - $array['priceCurrency'] + $array['priceCurrency'], + $array['valueAddedTaxIncluded'] ?? true, // default value ); } @@ -38,7 +40,8 @@ public static function dummy(): self { return new self( 13.37, - 'EUR' + 'EUR', + true ); } @@ -46,7 +49,8 @@ public static function anotherDummy(): self { return new self( 84.72, - 'EUR' + 'EUR', + false ); } diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php index 69ca326691f..c438a60fa74 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/ProjectedNodeTrait.php @@ -256,7 +256,7 @@ public function iExpectThisNodeToBeUnnamed(): void } /** - * @Then /^I expect this node to have the following serialized property types:$/ + * @Then /^I expect this node to have the following serialized properties:$/ * @param TableNode $expectedPropertyTypes */ public function iExpectThisNodeToHaveTheFollowingSerializedPropertyTypes(TableNode $expectedPropertyTypes): void @@ -266,6 +266,14 @@ public function iExpectThisNodeToHaveTheFollowingSerializedPropertyTypes(TableNo foreach ($expectedPropertyTypes->getHash() as $row) { Assert::assertTrue($serialized->propertyExists($row['Key']), 'Property "' . $row['Key'] . '" not found'); Assert::assertEquals($row['Type'], $serialized->getProperty($row['Key'])->type, 'Serialized node property ' . $row['Key'] . ' does not match expected type.'); + if (str_starts_with($row['Value'], 'NOT:')) { + // special case. assert NOT equals: + $value = json_decode(mb_substr($row['Value'], 4), true, 512, JSON_THROW_ON_ERROR); + Assert::assertNotEquals($value, $serialized->getProperty($row['Key'])->value, 'Serialized node property ' . $row['Key'] . ' does match value it should not.'); + } else { + $value = json_decode($row['Value'], true, 512, JSON_THROW_ON_ERROR); + Assert::assertEquals($value, $serialized->getProperty($row['Key'])->value, 'Serialized node property ' . $row['Key'] . ' does not match expected value.'); + } } }); }