From f0fc051e0365200e0f005ffb4987115400eae2a8 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 13 Jan 2025 17:51:26 +0100 Subject: [PATCH] TASK: Ensure that `Events` are never instantiated empty 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 https://github.com/neos/neos-development-collection/pull/5337 - structure adjustment TETHERED_NODE_WRONGLY_ORDERED --- .../Classes/EventStore/Events.php | 21 +++---- .../Common/NodeTypeChangeInternals.php | 15 +++-- .../Feature/Common/NodeVariationInternals.php | 6 +- .../Feature/Common/TetheredNodeInternals.php | 56 +++++++++---------- .../Feature/NodeCreation/NodeCreation.php | 14 +++-- .../Feature/NodeTypeChange/NodeTypeChange.php | 18 +++--- .../Classes/Feature/RebaseableCommand.php | 2 +- .../RootNodeCreation/RootNodeHandling.php | 14 +++-- 8 files changed, 79 insertions(+), 67 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/EventStore/Events.php b/Neos.ContentRepository.Core/Classes/EventStore/Events.php index f9d9abc8191..2401685f992 100644 --- a/Neos.ContentRepository.Core/Classes/EventStore/Events.php +++ b/Neos.ContentRepository.Core/Classes/EventStore/Events.php @@ -10,16 +10,17 @@ * @implements \IteratorAggregate * @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 + * @var non-empty-array */ - private readonly array $events; + public array $items; private function __construct(EventInterface|DecoratedEvent ...$events) { - $this->events = $events; + /** @var non-empty-array $events */ + $this->items = $events; } public static function with(EventInterface|DecoratedEvent $event): self @@ -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 $events + * @param non-empty-array $events * @return static */ public static function fromArray(array $events): self @@ -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 + * @return non-empty-list */ 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); } } diff --git a/Neos.ContentRepository.Core/Classes/Feature/Common/NodeTypeChangeInternals.php b/Neos.ContentRepository.Core/Classes/Feature/Common/NodeTypeChangeInternals.php index 31d944fe121..abcb07b5bff 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/Common/NodeTypeChangeInternals.php +++ b/Neos.ContentRepository.Core/Classes/Feature/Common/NodeTypeChangeInternals.php @@ -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; @@ -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 */ 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. @@ -117,15 +119,18 @@ private function deleteDisallowedNodesWhenChangingNodeType( } } - return Events::fromArray($events); + return $events; } + /** + * @return array + */ private function deleteObsoleteTetheredNodesWhenChangingNodeType( ContentGraphInterface $contentGraph, NodeAggregate $nodeAggregate, NodeType $newNodeType, NodeAggregateIds &$alreadyRemovedNodeAggregateIds, - ): Events { + ): array { $events = []; // find disallowed tethered nodes $tetheredNodeAggregates = $contentGraph->findTetheredChildNodeAggregates($nodeAggregate->nodeAggregateId); @@ -156,7 +161,7 @@ private function deleteObsoleteTetheredNodesWhenChangingNodeType( } } - return Events::fromArray($events); + return $events; } /** diff --git a/Neos.ContentRepository.Core/Classes/Feature/Common/NodeVariationInternals.php b/Neos.ContentRepository.Core/Classes/Feature/Common/NodeVariationInternals.php index 12f3df80da1..349de0d4ced 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/Common/NodeVariationInternals.php +++ b/Neos.ContentRepository.Core/Classes/Feature/Common/NodeVariationInternals.php @@ -89,7 +89,7 @@ protected function handleCreateNodeSpecializationVariant( /** * @param array $events - * @return array + * @return non-empty-array */ protected function collectNodeSpecializationVariantsThatWillHaveBeenCreated( ContentGraphInterface $contentGraph, @@ -152,7 +152,7 @@ protected function handleCreateNodeGeneralizationVariant( /** * @param array $events - * @return array + * @return non-empty-array */ protected function collectNodeGeneralizationVariantsThatWillHaveBeenCreated( ContentGraphInterface $contentGraph, @@ -215,7 +215,7 @@ protected function handleCreateNodePeerVariant( /** * @param array $events - * @return array + * @return non-empty-array */ protected function collectNodePeerVariantsThatWillHaveBeenCreated( ContentGraphInterface $contentGraph, diff --git a/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php b/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php index 57b99797015..c53ca1f4313 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php +++ b/Neos.ContentRepository.Core/Classes/Feature/Common/TetheredNodeInternals.php @@ -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; @@ -164,6 +165,9 @@ protected function createEventsForMissingTetheredNode( ); } + /** + * @return array + */ protected function createEventsForMissingTetheredNodeAggregate( ContentGraphInterface $contentGraph, TetheredNodeTypeDefinition $tetheredNodeTypeDefinition, @@ -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(); @@ -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( @@ -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 )); } @@ -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, @@ -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( diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php index b73ba4750f6..df4415e6dfb 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeCreation/NodeCreation.php @@ -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; @@ -212,7 +213,7 @@ private function handleCreateNodeAggregateWithNodeAndSerializedProperties( ) ]; - array_push($events, ...iterator_to_array($this->handleTetheredChildNodes( + array_push($events, ...$this->handleTetheredChildNodes( $command, $contentGraph, $nodeType, @@ -220,7 +221,7 @@ private function handleCreateNodeAggregateWithNodeAndSerializedProperties( $command->nodeAggregateId, $descendantNodeAggregateIds, null - ))); + )); return new EventsToPublish( ContentStreamEventStreamName::fromContentStreamId($contentGraph->getContentStreamId()) @@ -261,6 +262,7 @@ private function createRegularWithNode( /** * @throws ContentStreamDoesNotExistYet * @throws NodeTypeNotFound + * @return array */ private function handleTetheredChildNodes( CreateNodeAggregateWithNodeAndSerializedProperties $command, @@ -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); @@ -298,7 +300,7 @@ private function handleTetheredChildNodes( SerializedNodeReferences::createEmpty(), ); - array_push($events, ...iterator_to_array($this->handleTetheredChildNodes( + array_push($events, ...$this->handleTetheredChildNodes( $command, $contentGraph, $childNodeType, @@ -306,9 +308,9 @@ private function handleTetheredChildNodes( $childNodeAggregateId, $nodeAggregateIds, $childNodePath - ))); + )); } - return Events::fromArray($events); + return $events; } } diff --git a/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php b/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php index 5436b048236..acb6e5a4c96 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php +++ b/Neos.ContentRepository.Core/Classes/Feature/NodeTypeChange/NodeTypeChange.php @@ -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; @@ -90,6 +91,9 @@ abstract protected function areNodeTypeConstraintsImposedByGrandparentValid( NodeType $nodeType ): bool; + /** + * @return array + */ abstract protected function createEventsForMissingTetheredNodeAggregate( ContentGraphInterface $contentGraph, TetheredNodeTypeDefinition $tetheredNodeTypeDefinition, @@ -99,7 +103,7 @@ abstract protected function createEventsForMissingTetheredNodeAggregate( ?NodeAggregateId $succeedingSiblingNodeAggregateId, NodeAggregateIdsByNodePaths $nodeAggregateIdsByNodePaths, NodePath $currentNodePath, - ): Events; + ): array; abstract protected function createEventsForWronglyTypedNodeAggregate( ContentGraphInterface $contentGraph, @@ -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 @@ -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, @@ -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, diff --git a/Neos.ContentRepository.Core/Classes/Feature/RebaseableCommand.php b/Neos.ContentRepository.Core/Classes/Feature/RebaseableCommand.php index b62d85eddd8..2ea502274f3 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/RebaseableCommand.php +++ b/Neos.ContentRepository.Core/Classes/Feature/RebaseableCommand.php @@ -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) { diff --git a/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php b/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php index 1ed9b90f862..8c131e00a00 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php +++ b/Neos.ContentRepository.Core/Classes/Feature/RootNodeCreation/RootNodeHandling.php @@ -17,6 +17,7 @@ use Neos\ContentRepository\Core\CommandHandler\CommandHandlingDependencies; use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePointSet; use Neos\ContentRepository\Core\DimensionSpace\OriginDimensionSpacePoint; +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; @@ -104,7 +105,7 @@ private function handleCreateRootNodeAggregateWithNode( ]; foreach ($this->getInterDimensionalVariationGraph()->getRootGeneralizations() as $rootGeneralization) { - array_push($events, ...iterator_to_array($this->handleTetheredRootChildNodes( + array_push($events, ...$this->handleTetheredRootChildNodes( $contentGraph->getWorkspaceName(), $contentGraph->getContentStreamId(), $nodeType, @@ -113,7 +114,7 @@ private function handleCreateRootNodeAggregateWithNode( $command->nodeAggregateId, $command->tetheredDescendantNodeAggregateIds, null - ))); + )); } $contentStreamEventStream = ContentStreamEventStreamName::fromContentStreamId($contentGraph->getContentStreamId()); @@ -185,6 +186,7 @@ private function handleUpdateRootNodeAggregateDimensions( /** * @throws ContentStreamDoesNotExistYet * @throws NodeTypeNotFound + * @return array */ private function handleTetheredRootChildNodes( WorkspaceName $workspaceName, @@ -195,7 +197,7 @@ private function handleTetheredRootChildNodes( NodeAggregateId $parentNodeAggregateId, NodeAggregateIdsByNodePaths $nodeAggregateIdsByNodePath, ?NodePath $nodePath - ): Events { + ): array { $events = []; foreach ($nodeType->tetheredNodeTypeDefinitions as $tetheredNodeTypeDefinition) { $childNodeType = $this->requireNodeType($tetheredNodeTypeDefinition->nodeTypeName); @@ -218,7 +220,7 @@ private function handleTetheredRootChildNodes( $initialPropertyValues ); - array_push($events, ...iterator_to_array($this->handleTetheredRootChildNodes( + array_push($events, ...$this->handleTetheredRootChildNodes( $workspaceName, $contentStreamId, $childNodeType, @@ -227,10 +229,10 @@ private function handleTetheredRootChildNodes( $childNodeAggregateId, $nodeAggregateIdsByNodePath, $childNodePath - ))); + )); } - return Events::fromArray($events); + return $events; } private function createTetheredWithNodeForRoot(