Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: DateTime node property with defaultValue #4902

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -66,6 +71,20 @@ 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 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 |
| array | {"givenName":"Nody", "familyName":"McNodeface"} |
Expand Down Expand Up @@ -94,3 +113,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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,6 +37,8 @@ trait TetheredNodeInternals
{
use NodeVariationInternals;

abstract protected function getPropertyConverter(): PropertyConverter;

abstract protected function createEventsForVariations(
ContentStreamId $contentStreamId,
OriginDimensionSpacePoint $sourceOrigin,
Expand Down Expand Up @@ -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;
Expand All @@ -117,7 +120,7 @@ protected function createEventsForMissingTetheredNode(
$parentNodeAggregate->getCoverageByOccupant($originDimensionSpacePoint),
$parentNodeAggregate->nodeAggregateId,
$tetheredNodeName,
SerializedPropertyValues::defaultFromNodeType($expectedTetheredNodeType),
SerializedPropertyValues::defaultFromNodeType($expectedTetheredNodeType, $this->getPropertyConverter()),
NodeAggregateClassification::CLASSIFICATION_TETHERED,
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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) {
Expand Down Expand Up @@ -224,7 +199,7 @@ private function handleCreateNodeAggregateWithNodeAndSerializedProperties(
);
}

$defaultPropertyValues = SerializedPropertyValues::defaultFromNodeType($nodeType);
$defaultPropertyValues = SerializedPropertyValues::defaultFromNodeType($nodeType, $this->getPropertyConverter());
$initialPropertyValues = $defaultPropertyValues->merge($command->initialPropertyValues);

$events = [
Expand Down Expand Up @@ -294,7 +269,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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

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;
use Neos\ContentRepository\Core\SharedModel\Node\PropertyNames;
Expand Down Expand Up @@ -65,22 +67,33 @@ 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) {
if ($defaultValue instanceof \DateTimeInterface) {
$defaultValue = json_encode($defaultValue);
}

$propertyTypeFromSchema = $nodeType->getPropertyType($propertyName);
self::assertTypeIsNoReference($propertyTypeFromSchema);

if ($defaultValue === null) {
continue;
}

$values[$propertyName] = SerializedPropertyValue::create($defaultValue, $propertyTypeFromSchema);
$propertyType = PropertyType::fromNodeTypeDeclaration(
$nodeType->getPropertyType($propertyName),
PropertyName::fromString($propertyName),
$nodeType->name
);
$deserializedDefaultValue = $propertyConverter->deserializePropertyValue(
SerializedPropertyValue::create($defaultValue, $propertyType->getSerializationType())
);
// 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),
$nodeType->name
),
$deserializedDefaultValue
);
$values[$propertyName] = $properlySerializedDefaultValue;
}

return new self($values);
Expand All @@ -91,15 +104,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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -45,32 +44,26 @@ 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
);
}

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
);
}
Expand All @@ -80,22 +73,22 @@ 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
);
}

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()
);
}

Expand All @@ -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
);
}
Expand All @@ -122,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)),
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved
1708416598,
$e
);
}
}
}
2 changes: 1 addition & 1 deletion Neos.ContentRepository.Core/Classes/NodeType/NodeType.php
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ public function getPropertyType(string $propertyName): string
*
* The default value is configured for each property under the "default" key.
*
* @return array<string,mixed>
* @return array<string,int|float|string|bool|array<int|string,mixed>>
* @api
*/
public function getDefaultValuesForProperties(): array
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,35 @@
{
private function __construct(
public float $price,
public string $priceCurrency
public string $priceCurrency,
public bool $valueAddedTaxIncluded
) {
}

public static function fromArray(array $array): self
{
return new self(
$array['price'],
$array['priceCurrency']
$array['priceCurrency'],
$array['valueAddedTaxIncluded'] ?? true, // default value
);
}

public static function dummy(): self
{
return new self(
13.37,
'EUR'
'EUR',
true
);
}

public static function anotherDummy(): self
{
return new self(
84.72,
'EUR'
'EUR',
false
);
}

Expand Down
Loading
Loading