Skip to content

Commit

Permalink
TASK: Ensure that Events are never instantiated empty
Browse files Browse the repository at this point in the history
by enforcing this via "non-empty-array" we can better catch runtime type errors beforehand via phpstan. As otherwise the event store will fail with

> Writable events must contain at least one event

Some internal utility methods previously also returned possibly empty "Events" instances which were then transformed via iterator_to_array back to arrays and merged. This was not simplified to just work with arrays directly to denote that they might be empty. In cases were we work with fully sure filled intermediate event arrays we use non-empty-array.

With this now fine-grained type checking aside from the original found set properties error following cases will also break with 0 events:

- handleSetSerializedNodeProperties (known)
- handleSetSerializedNodeReferences
- handlePublishWorkspace and handlePublishIndividualNodesFromWorkspace if there were no applied events that implement PublishableToWorkspaceInterface -> goes into direction of neos#5337
- structure adjustment TETHERED_NODE_WRONGLY_ORDERED
  • Loading branch information
mhsdesign committed Jan 13, 2025
1 parent c6d093b commit f0fc051
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 67 deletions.
21 changes: 11 additions & 10 deletions Neos.ContentRepository.Core/Classes/EventStore/Events.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@
* @implements \IteratorAggregate<EventInterface|DecoratedEvent>
* @internal only used during event publishing (from within command handlers) - and their implementation is not API
*/
final class Events implements \IteratorAggregate, \Countable
final readonly class Events implements \IteratorAggregate, \Countable
{
/**
* @var array<EventInterface|DecoratedEvent>
* @var non-empty-array<EventInterface|DecoratedEvent>
*/
private readonly array $events;
public array $items;

private function __construct(EventInterface|DecoratedEvent ...$events)
{
$this->events = $events;
/** @var non-empty-array<EventInterface|DecoratedEvent> $events */
$this->items = $events;
}

public static function with(EventInterface|DecoratedEvent $event): self
Expand All @@ -29,11 +30,11 @@ public static function with(EventInterface|DecoratedEvent $event): self

public function withAppendedEvents(Events $events): self
{
return new self(...$this->events, ...$events->events);
return new self(...$this->items, ...$events->items);
}

/**
* @param array<EventInterface|DecoratedEvent> $events
* @param non-empty-array<EventInterface|DecoratedEvent> $events
* @return static
*/
public static function fromArray(array $events): self
Expand All @@ -43,21 +44,21 @@ public static function fromArray(array $events): self

public function getIterator(): \Traversable
{
yield from $this->events;
yield from $this->items;
}

/**
* @template T
* @param \Closure(EventInterface|DecoratedEvent $event): T $callback
* @return list<T>
* @return non-empty-list<T>
*/
public function map(\Closure $callback): array
{
return array_map($callback, $this->events);
return array_map($callback, $this->items);
}

public function count(): int
{
return count($this->events);
return count($this->items);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePointSet;
use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePointSet;
use Neos\ContentRepository\Core\EventStore\Events;
use Neos\ContentRepository\Core\EventStore\EventInterface;
use Neos\ContentRepository\Core\Feature\NodeRemoval\Event\NodeAggregateWasRemoved;
use Neos\ContentRepository\Core\NodeType\NodeType;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentGraphInterface;
Expand All @@ -34,13 +34,15 @@ trait NodeTypeChangeInternals
/**
* NOTE: when changing this method, {@see NodeTypeChange::requireConstraintsImposedByHappyPathStrategyAreMet}
* needs to be modified as well (as they are structurally the same)
*
* @return array<EventInterface>
*/
private function deleteDisallowedNodesWhenChangingNodeType(
ContentGraphInterface $contentGraph,
NodeAggregate $nodeAggregate,
NodeType $newNodeType,
NodeAggregateIds &$alreadyRemovedNodeAggregateIds,
): Events {
): array {
$events = [];
// if we have children, we need to check whether they are still allowed
// after we changed the node type of the $nodeAggregate to $newNodeType.
Expand Down Expand Up @@ -117,15 +119,18 @@ private function deleteDisallowedNodesWhenChangingNodeType(
}
}

return Events::fromArray($events);
return $events;
}

/**
* @return array<EventInterface>
*/
private function deleteObsoleteTetheredNodesWhenChangingNodeType(
ContentGraphInterface $contentGraph,
NodeAggregate $nodeAggregate,
NodeType $newNodeType,
NodeAggregateIds &$alreadyRemovedNodeAggregateIds,
): Events {
): array {
$events = [];
// find disallowed tethered nodes
$tetheredNodeAggregates = $contentGraph->findTetheredChildNodeAggregates($nodeAggregate->nodeAggregateId);
Expand Down Expand Up @@ -156,7 +161,7 @@ private function deleteObsoleteTetheredNodesWhenChangingNodeType(
}
}

return Events::fromArray($events);
return $events;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ protected function handleCreateNodeSpecializationVariant(

/**
* @param array<int,EventInterface> $events
* @return array<int,EventInterface>
* @return non-empty-array<int,EventInterface>
*/
protected function collectNodeSpecializationVariantsThatWillHaveBeenCreated(
ContentGraphInterface $contentGraph,
Expand Down Expand Up @@ -152,7 +152,7 @@ protected function handleCreateNodeGeneralizationVariant(

/**
* @param array<int,EventInterface> $events
* @return array<int,EventInterface>
* @return non-empty-array<int,EventInterface>
*/
protected function collectNodeGeneralizationVariantsThatWillHaveBeenCreated(
ContentGraphInterface $contentGraph,
Expand Down Expand Up @@ -215,7 +215,7 @@ protected function handleCreateNodePeerVariant(

/**
* @param array<int,EventInterface> $events
* @return array<int,EventInterface>
* @return non-empty-array<int,EventInterface>
*/
protected function collectNodePeerVariantsThatWillHaveBeenCreated(
ContentGraphInterface $contentGraph,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePointSet;
use Neos\ContentRepository\Core\DimensionSpace\VariantType;
use Neos\ContentRepository\Core\EventStore\EventInterface;
use Neos\ContentRepository\Core\EventStore\Events;
use Neos\ContentRepository\Core\Feature\NodeCreation\Dto\NodeAggregateIdsByNodePaths;
use Neos\ContentRepository\Core\Feature\NodeCreation\Event\NodeAggregateWithNodeWasCreated;
Expand Down Expand Up @@ -164,6 +165,9 @@ protected function createEventsForMissingTetheredNode(
);
}

/**
* @return array<EventInterface>
*/
protected function createEventsForMissingTetheredNodeAggregate(
ContentGraphInterface $contentGraph,
TetheredNodeTypeDefinition $tetheredNodeTypeDefinition,
Expand All @@ -173,7 +177,7 @@ protected function createEventsForMissingTetheredNodeAggregate(
?NodeAggregateId $succeedingSiblingNodeAggregateId,
NodeAggregateIdsByNodePaths $nodeAggregateIdsByNodePaths,
NodePath $currentNodePath,
): Events {
): array {
$events = [];
$tetheredNodeType = $this->requireNodeType($tetheredNodeTypeDefinition->nodeTypeName);
$nodeAggregateId = $nodeAggregateIdsByNodePaths->getNodeAggregateId($currentNodePath) ?? NodeAggregateId::create();
Expand Down Expand Up @@ -243,22 +247,20 @@ protected function createEventsForMissingTetheredNodeAggregate(
foreach ($tetheredNodeType->tetheredNodeTypeDefinitions as $childTetheredNodeTypeDefinition) {
$events = array_merge(
$events,
iterator_to_array(
$this->createEventsForMissingTetheredNodeAggregate(
$contentGraph,
$childTetheredNodeTypeDefinition,
$affectedOriginDimensionSpacePoints,
$coverageByOrigin,
$nodeAggregateId,
null,
$nodeAggregateIdsByNodePaths,
$currentNodePath->appendPathSegment($childTetheredNodeTypeDefinition->name),
)
$this->createEventsForMissingTetheredNodeAggregate(
$contentGraph,
$childTetheredNodeTypeDefinition,
$affectedOriginDimensionSpacePoints,
$coverageByOrigin,
$nodeAggregateId,
null,
$nodeAggregateIdsByNodePaths,
$currentNodePath->appendPathSegment($childTetheredNodeTypeDefinition->name),
)
);
}

return Events::fromArray($events);
return $events;
}

protected function createEventsForWronglyTypedNodeAggregate(
Expand Down Expand Up @@ -311,21 +313,17 @@ protected function createEventsForWronglyTypedNodeAggregate(

// remove disallowed nodes
if ($conflictResolutionStrategy === NodeAggregateTypeChangeChildConstraintConflictResolutionStrategy::STRATEGY_DELETE) {
array_push($events, ...iterator_to_array(
$this->deleteDisallowedNodesWhenChangingNodeType(
$contentGraph,
$nodeAggregate,
$tetheredNodeType,
$alreadyRemovedNodeAggregateIds
)
array_push($events, ...$this->deleteDisallowedNodesWhenChangingNodeType(
$contentGraph,
$nodeAggregate,
$tetheredNodeType,
$alreadyRemovedNodeAggregateIds
));
array_push($events, ...iterator_to_array(
$this->deleteObsoleteTetheredNodesWhenChangingNodeType(
$contentGraph,
$nodeAggregate,
$tetheredNodeType,
$alreadyRemovedNodeAggregateIds
)
array_push($events, ...$this->deleteObsoleteTetheredNodesWhenChangingNodeType(
$contentGraph,
$nodeAggregate,
$tetheredNodeType,
$alreadyRemovedNodeAggregateIds
));
}

Expand All @@ -338,7 +336,7 @@ protected function createEventsForWronglyTypedNodeAggregate(
if ($tetheredChildNodeAggregate === null) {
$events = array_merge(
$events,
iterator_to_array($this->createEventsForMissingTetheredNodeAggregate(
$this->createEventsForMissingTetheredNodeAggregate(
$contentGraph,
$childTetheredNodeTypeDefinition,
$nodeAggregate->occupiedDimensionSpacePoints,
Expand All @@ -347,7 +345,7 @@ protected function createEventsForWronglyTypedNodeAggregate(
null,
$nodeAggregateIdsByNodePaths,
$currentNodePath->appendPathSegment($childTetheredNodeTypeDefinition->name),
))
)
);
} elseif (!$tetheredChildNodeAggregate->nodeTypeName->equals($childTetheredNodeTypeDefinition->nodeTypeName)) {
$events = array_merge($events, iterator_to_array(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Neos\ContentRepository\Core\CommandHandler\CommandHandlingDependencies;
use Neos\ContentRepository\Core\DimensionSpace;
use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePointSet;
use Neos\ContentRepository\Core\EventStore\EventInterface;
use Neos\ContentRepository\Core\EventStore\Events;
use Neos\ContentRepository\Core\EventStore\EventsToPublish;
use Neos\ContentRepository\Core\Feature\Common\InterdimensionalSiblings;
Expand Down Expand Up @@ -212,15 +213,15 @@ private function handleCreateNodeAggregateWithNodeAndSerializedProperties(
)
];

array_push($events, ...iterator_to_array($this->handleTetheredChildNodes(
array_push($events, ...$this->handleTetheredChildNodes(
$command,
$contentGraph,
$nodeType,
$coveredDimensionSpacePoints,
$command->nodeAggregateId,
$descendantNodeAggregateIds,
null
)));
));

return new EventsToPublish(
ContentStreamEventStreamName::fromContentStreamId($contentGraph->getContentStreamId())
Expand Down Expand Up @@ -261,6 +262,7 @@ private function createRegularWithNode(
/**
* @throws ContentStreamDoesNotExistYet
* @throws NodeTypeNotFound
* @return array<EventInterface>
*/
private function handleTetheredChildNodes(
CreateNodeAggregateWithNodeAndSerializedProperties $command,
Expand All @@ -270,7 +272,7 @@ private function handleTetheredChildNodes(
NodeAggregateId $parentNodeAggregateId,
NodeAggregateIdsByNodePaths $nodeAggregateIds,
?NodePath $nodePath
): Events {
): array {
$events = [];
foreach ($nodeType->tetheredNodeTypeDefinitions as $tetheredNodeTypeDefinition) {
$childNodeType = $this->requireNodeType($tetheredNodeTypeDefinition->nodeTypeName);
Expand Down Expand Up @@ -298,17 +300,17 @@ private function handleTetheredChildNodes(
SerializedNodeReferences::createEmpty(),
);

array_push($events, ...iterator_to_array($this->handleTetheredChildNodes(
array_push($events, ...$this->handleTetheredChildNodes(
$command,
$contentGraph,
$childNodeType,
$coveredDimensionSpacePoints,
$childNodeAggregateId,
$nodeAggregateIds,
$childNodePath
)));
));
}

return Events::fromArray($events);
return $events;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Neos\ContentRepository\Core\CommandHandler\CommandHandlingDependencies;
use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint;
use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePointSet;
use Neos\ContentRepository\Core\EventStore\EventInterface;
use Neos\ContentRepository\Core\EventStore\Events;
use Neos\ContentRepository\Core\EventStore\EventsToPublish;
use Neos\ContentRepository\Core\Feature\RebaseableCommand;
Expand Down Expand Up @@ -90,6 +91,9 @@ abstract protected function areNodeTypeConstraintsImposedByGrandparentValid(
NodeType $nodeType
): bool;

/**
* @return array<EventInterface>
*/
abstract protected function createEventsForMissingTetheredNodeAggregate(
ContentGraphInterface $contentGraph,
TetheredNodeTypeDefinition $tetheredNodeTypeDefinition,
Expand All @@ -99,7 +103,7 @@ abstract protected function createEventsForMissingTetheredNodeAggregate(
?NodeAggregateId $succeedingSiblingNodeAggregateId,
NodeAggregateIdsByNodePaths $nodeAggregateIdsByNodePaths,
NodePath $currentNodePath,
): Events;
): array;

abstract protected function createEventsForWronglyTypedNodeAggregate(
ContentGraphInterface $contentGraph,
Expand Down Expand Up @@ -230,18 +234,18 @@ private function handleChangeNodeAggregateType(
// remove disallowed nodes
$alreadyRemovedNodeAggregateIds = NodeAggregateIds::createEmpty();
if ($command->strategy === NodeAggregateTypeChangeChildConstraintConflictResolutionStrategy::STRATEGY_DELETE) {
array_push($events, ...iterator_to_array($this->deleteDisallowedNodesWhenChangingNodeType(
array_push($events, ...$this->deleteDisallowedNodesWhenChangingNodeType(
$contentGraph,
$nodeAggregate,
$newNodeType,
$alreadyRemovedNodeAggregateIds,
)));
array_push($events, ...iterator_to_array($this->deleteObsoleteTetheredNodesWhenChangingNodeType(
));
array_push($events, ...$this->deleteObsoleteTetheredNodesWhenChangingNodeType(
$contentGraph,
$nodeAggregate,
$newNodeType,
$alreadyRemovedNodeAggregateIds
)));
));
}

// handle (missing) tethered node aggregates
Expand All @@ -254,7 +258,7 @@ private function handleChangeNodeAggregateType(
foreach ($newNodeType->tetheredNodeTypeDefinitions as $tetheredNodeTypeDefinition) {
$tetheredNodeAggregate = $contentGraph->findChildNodeAggregateByName($nodeAggregate->nodeAggregateId, $tetheredNodeTypeDefinition->name);
if ($tetheredNodeAggregate === null) {
$events = array_merge($events, iterator_to_array($this->createEventsForMissingTetheredNodeAggregate(
$events = array_merge($events, $this->createEventsForMissingTetheredNodeAggregate(
$contentGraph,
$tetheredNodeTypeDefinition,
$nodeAggregate->occupiedDimensionSpacePoints,
Expand All @@ -263,7 +267,7 @@ private function handleChangeNodeAggregateType(
$succeedingSiblingIds[$tetheredNodeTypeDefinition->nodeTypeName->value] ?? null,
$command->tetheredDescendantNodeAggregateIds,
NodePath::fromNodeNames($tetheredNodeTypeDefinition->name)
)));
));
} elseif (!$tetheredNodeAggregate->nodeTypeName->equals($tetheredNodeTypeDefinition->nodeTypeName)) {
$events = array_merge($events, iterator_to_array($this->createEventsForWronglyTypedNodeAggregate(
$contentGraph,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static function enrichWithCommand(
$processedEvents = [];
$causationId = null;
$i = 0;
foreach ($events as $event) {
foreach ($events->items as $event) {
if ($event instanceof DecoratedEvent) {
$undecoratedEvent = $event->innerEvent;
if (!$undecoratedEvent instanceof PublishableToWorkspaceInterface) {
Expand Down
Loading

0 comments on commit f0fc051

Please sign in to comment.