Skip to content

Commit

Permalink
Merge pull request #4677 from neos/4549-separateReferencesAndProperties
Browse files Browse the repository at this point in the history
!!! FEATURE: Separate properties from references in NodeType declaration
  • Loading branch information
mhsdesign authored Apr 6, 2024
2 parents 3e38b36 + e282c14 commit 7cf6bee
Show file tree
Hide file tree
Showing 34 changed files with 593 additions and 209 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,24 @@ Feature: Constraint checks on SetNodeReferences
'Neos.ContentRepository.Testing:ReferencedNode': []
'Neos.ContentRepository.Testing:NodeWithReferences':
# legacy notation
properties:
referenceProperty:
type: reference
referencesProperty:
type: references
nonReferenceProperty:
type: string
references:
constrainedReferenceCount:
constraints:
maxItems: 1
constrainedReferenceProperty:
type: reference
constraints:
maxItems: 1
nodeTypes:
'Neos.ContentRepository.Testing:ReferencedNode': false
referencePropertyWithProperties:
type: reference
properties:
text:
type: string
Expand Down Expand Up @@ -128,6 +132,22 @@ Feature: Constraint checks on SetNodeReferences
| references | [{"target":"lady-eleonode-rootford"}] |
Then the last command should have thrown an exception of type "NodeAggregateIsRoot"

Scenario: Try to set references exceeding the maxItems count
When the command SetNodeReferences is executed with payload and exceptions are caught:
| Key | Value |
| sourceNodeAggregateId | "source-nodandaise" |
| referenceName | "constrainedReferenceCount" |
| references | [{"target":"anthony-destinode"}, {"target":"berta-destinode"}] |
Then the last command should have thrown an exception of type "ReferenceCannotBeSet" with code 1700150156

Scenario: Try to set references exceeding the maxItems count for legacy property reference declaration
When the command SetNodeReferences is executed with payload and exceptions are caught:
| Key | Value |
| sourceNodeAggregateId | "source-nodandaise" |
| referenceName | "referenceProperty" |
| references | [{"target":"anthony-destinode"}, {"target":"berta-destinode"}] |
Then the last command should have thrown an exception of type "ReferenceCannotBeSet" with code 1700150156

Scenario: Try to reference a node aggregate of a type not matching the constraints
When the command SetNodeReferences is executed with payload and exceptions are caught:
| Key | Value |
Expand Down Expand Up @@ -187,3 +207,11 @@ Feature: Constraint checks on SetNodeReferences
| referenceName | "referencePropertyWithProperties" |
| references | [{"target":"anthony-destinode", "properties":{"postalAddress": "28 31st of February Street"}}] |
Then the last command should have thrown an exception of type "ReferenceCannotBeSet" with code 1658406762

Scenario: Node reference cannot hold multiple targets to the same node
When the command SetNodeReferences is executed with payload and exceptions are caught:
| Key | Value |
| sourceNodeAggregateId | "source-nodandaise" |
| referenceName | "referencesProperty" |
| references | [{"target":"anthony-destinode"}, {"target":"anthony-destinode"}] |
Then the last command should have thrown an exception of type "InvalidArgumentException" with code 1700150910
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,21 @@ Feature: Node References without Dimensions
'Neos.ContentRepository.Testing:ReferencedNode': []
'Neos.ContentRepository.Testing:NodeWithReferences':
# legacy notation
properties:
referenceProperty:
type: reference
referencesProperty:
type: references
references:
restrictedReferenceProperty:
type: reference
constraints:
nodeTypes:
'*': false
'Neos.ContentRepository.Testing:ReferencedNode': true
referencePropertyWithProperty:
type: reference
constraints:
maxItems: 1
properties:
text:
type: string
Expand All @@ -31,7 +33,6 @@ Feature: Node References without Dimensions
postalAddress:
type: 'Neos\ContentRepository\Core\Tests\Behavior\Fixtures\PostalAddress'
referencesPropertyWithProperty:
type: references
properties:
text:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ Feature: Find nodes using the findNodeById query
properties:
text:
type: string
references:
refs:
type: references
properties:
foo:
type: string
ref:
type: reference
constraints:
maxItems: 1
properties:
foo:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ Feature: Find nodes using the findNodeByPath query
properties:
text:
type: string
references:
refs:
type: references
properties:
foo:
type: string
ref:
type: reference
constraints:
maxItems: 1
properties:
foo:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ Feature: Find nodes using the findNodeByPath query with node name as path argume
properties:
text:
type: string
references:
refs:
type: references
properties:
foo:
type: string
ref:
type: reference
constraints:
maxItems: 1
properties:
foo:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ Feature: Find nodes using the findParentNodes query
properties:
text:
type: string
references:
refs:
type: references
properties:
foo:
type: string
ref:
type: reference
constraints:
maxItems: 1
properties:
foo:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ Feature: Find and count references and their target nodes using the findReferenc
type: integer
dateProperty:
type: DateTime
references:
refs:
type: references
properties:
foo:
type: string
ref:
type: reference
constraints:
maxItems: 1
properties:
foo:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ Feature: Find sibling nodes using the findPrecedingSiblingNodes and findSucceedi
properties:
text:
type: string
references:
refs:
type: references
properties:
foo:
type: string
ref:
type: reference
constraints:
maxItems: 1
properties:
foo:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ Feature: Behavior of Node timestamp properties "created", "originalCreated", "la
properties:
text:
type: string
references:
refs:
type: references
properties:
foo:
type: string
ref:
type: reference
constraints:
maxItems: 1
properties:
foo:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Neos\ContentRepository\Core\DimensionSpace\Exception\DimensionSpacePointNotFound;
use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
use Neos\ContentRepository\Core\Feature\NodeModification\Dto\PropertyValuesToWrite;
use Neos\ContentRepository\Core\Feature\NodeReferencing\Dto\SerializedNodeReferences;
use Neos\ContentRepository\Core\Feature\NodeVariation\Exception\DimensionSpacePointIsAlreadyOccupied;
use Neos\ContentRepository\Core\Infrastructure\Property\PropertyType;
use Neos\ContentRepository\Core\NodeType\ConstraintCheck;
Expand All @@ -46,6 +47,7 @@
use Neos\ContentRepository\Core\SharedModel\Exception\NodeTypeIsOfTypeRoot;
use Neos\ContentRepository\Core\SharedModel\Exception\NodeTypeNotFound;
use Neos\ContentRepository\Core\SharedModel\Exception\NodeTypeNotFoundException;
use Neos\ContentRepository\Core\SharedModel\Exception\PropertyCannotBeSet;
use Neos\ContentRepository\Core\SharedModel\Exception\ReferenceCannotBeSet;
use Neos\ContentRepository\Core\SharedModel\Exception\RootNodeAggregateDoesNotExist;
use Neos\ContentRepository\Core\SharedModel\Exception\RootNodeAggregateTypeIsAlreadyOccupied;
Expand Down Expand Up @@ -196,20 +198,21 @@ protected function requireTetheredDescendantNodeTypesToNotBeOfTypeRoot(NodeType
protected function requireNodeTypeToDeclareProperty(NodeTypeName $nodeTypeName, PropertyName $propertyName): void
{
$nodeType = $this->requireNodeType($nodeTypeName);
if (!isset($nodeType->getProperties()[$propertyName->value])) {
if (!$nodeType->hasProperty($propertyName->value)) {
throw PropertyCannotBeSet::becauseTheNodeTypeDoesNotDeclareIt(
$propertyName,
$nodeTypeName
);
}
}

protected function requireNodeTypeToDeclareReference(NodeTypeName $nodeTypeName, ReferenceName $propertyName): void
protected function requireNodeTypeToDeclareReference(NodeTypeName $nodeTypeName, ReferenceName $referenceName): void
{
$nodeType = $this->requireNodeType($nodeTypeName);
if (isset($nodeType->getProperties()[$propertyName->value])) {
$propertyType = $nodeType->getPropertyType($propertyName->value);
if ($propertyType === 'reference' || $propertyType === 'references') {
return;
}
if ($nodeType->hasReference($referenceName->value)) {
return;
}
throw ReferenceCannotBeSet::becauseTheNodeTypeDoesNotDeclareIt($propertyName, $nodeTypeName);
throw ReferenceCannotBeSet::becauseTheNodeTypeDoesNotDeclareIt($referenceName, $nodeTypeName);
}

protected function requireNodeTypeToAllowNodesOfTypeInReference(
Expand All @@ -218,22 +221,35 @@ protected function requireNodeTypeToAllowNodesOfTypeInReference(
NodeTypeName $nodeTypeNameInQuestion
): void {
$nodeType = $this->requireNodeType($nodeTypeName);
$propertyDeclaration = $nodeType->getProperties()[$referenceName->value] ?? null;
if (is_null($propertyDeclaration)) {
throw ReferenceCannotBeSet::becauseTheNodeTypeDoesNotDeclareIt($referenceName, $nodeTypeName);
}

$constraints = $propertyDeclaration['constraints']['nodeTypes'] ?? [];
$constraints = $nodeType->getReferences()[$referenceName->value]['constraints']['nodeTypes'] ?? [];

if (!ConstraintCheck::create($constraints)->isNodeTypeAllowed($this->requireNodeType($nodeTypeNameInQuestion))) {
throw ReferenceCannotBeSet::becauseTheConstraintsAreNotMatched(
throw ReferenceCannotBeSet::becauseTheNodeTypeConstraintsAreNotMatched(
$referenceName,
$nodeTypeName,
$nodeTypeNameInQuestion
);
}
}

protected function requireNodeTypeToAllowNumberOfReferencesInReference(SerializedNodeReferences $nodeReferences, ReferenceName $referenceName, NodeTypeName $nodeTypeName): void
{
$nodeType = $this->requireNodeType($nodeTypeName);

$maxItems = $nodeType->getReferences()[$referenceName->value]['constraints']['maxItems'] ?? null;
if ($maxItems === null) {
return;
}

if ($maxItems < count($nodeReferences)) {
throw ReferenceCannotBeSet::becauseTheItemsCountConstraintsAreNotMatched(
$referenceName,
$nodeTypeName,
count($nodeReferences)
);
}
}

/**
* NodeType and NodeName must belong together to the same node, which is the to-be-checked one.
*
Expand Down Expand Up @@ -644,11 +660,11 @@ protected function validateReferenceProperties(
$nodeType = $this->requireNodeType($nodeTypeName);

foreach ($referenceProperties->values as $propertyName => $propertyValue) {
$referencePropertyConfig = $nodeType->getProperties()[$referenceName->value]['properties'][$propertyName]
$referencePropertyConfig = $nodeType->getReferences()[$referenceName->value]['properties'][$propertyName]
?? null;

if (is_null($referencePropertyConfig)) {
throw ReferenceCannotBeSet::becauseTheItDoesNotDeclareAProperty(
throw ReferenceCannotBeSet::becauseTheReferenceDoesNotDeclareTheProperty(
$referenceName,
$nodeTypeName,
PropertyName::fromString($propertyName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,7 @@ private function validateProperties(?PropertyValuesToWrite $propertyValues, Node

$nodeType = $this->requireNodeType($nodeTypeName);
foreach ($propertyValues->values as $propertyName => $propertyValue) {
if (!isset($nodeType->getProperties()[$propertyName])) {
throw PropertyCannotBeSet::becauseTheNodeTypeDoesNotDeclareIt(
PropertyName::fromString($propertyName),
$nodeTypeName
);
}
$this->requireNodeTypeToDeclareProperty($nodeTypeName, PropertyName::fromString($propertyName));
$propertyType = PropertyType::fromNodeTypeDeclaration(
$nodeType->getPropertyType($propertyName),
PropertyName::fromString($propertyName),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
* @implements \IteratorAggregate<NodeReferenceToWrite>
* @api used as part of commands
*/
final readonly class NodeReferencesToWrite implements \IteratorAggregate, \JsonSerializable
final readonly class NodeReferencesToWrite implements \IteratorAggregate, \Countable, \JsonSerializable
{
/**
* @var array<NodeReferenceToWrite>
Expand Down Expand Up @@ -58,6 +58,14 @@ public static function fromArray(array $values): self
));
}

/**
* Unset all references for this reference name.
*/
public static function createEmpty(): self
{
return new self();
}

public static function fromNodeAggregateIds(NodeAggregateIds $nodeAggregateIds): self
{
return new self(...array_map(
Expand Down Expand Up @@ -90,4 +98,9 @@ public function jsonSerialize(): array
{
return $this->references;
}

public function count(): int
{
return count($this->references);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@

private function __construct(SerializedNodeReference ...$references)
{
$existingTargets = [];
foreach ($references as $reference) {
if (isset($existingTargets[$reference->targetNodeAggregateId->value])) {
throw new \InvalidArgumentException(sprintf('Duplicate entry in references to write. Target "%s" already exists in collection.', $reference->targetNodeAggregateId->value), 1700150910);
}
$existingTargets[$reference->targetNodeAggregateId->value] = true;
}
$this->references = $references;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ private function handleSetSerializedNodeReferences(
);
$this->requireNodeTypeToDeclareReference($sourceNodeAggregate->nodeTypeName, $command->referenceName);

$this->requireNodeTypeToAllowNumberOfReferencesInReference(
$command->references,
$command->referenceName,
$sourceNodeAggregate->nodeTypeName
);

foreach ($command->references as $reference) {
assert($reference instanceof SerializedNodeReference);
$destinationNodeAggregate = $this->requireProjectedNodeAggregate(
Expand All @@ -135,7 +141,7 @@ private function handleSetSerializedNodeReferences(
}

$sourceNodeType = $this->requireNodeType($sourceNodeAggregate->nodeTypeName);
$scopeDeclaration = $sourceNodeType->getProperties()[$command->referenceName->value]['scope'] ?? '';
$scopeDeclaration = $sourceNodeType->getReferences()[$command->referenceName->value]['scope'] ?? '';
$scope = PropertyScope::tryFrom($scopeDeclaration) ?: PropertyScope::SCOPE_NODE;

$affectedOrigins = $scope->resolveAffectedOrigins(
Expand Down
Loading

0 comments on commit 7cf6bee

Please sign in to comment.