Merge pull request #5396 from mhsdesign/bugfix/fail-early-if-set-prop…

…erties-is-empty

BUGFIX: Ensure that its not attempted to publish 0 events

Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/04-NodeModification/01-SetNodeProperties_ConstraintChecks.feature

@@ -67,7 +67,7 @@ Feature: Set node properties: Constraint checks
       | Key                       | Value                    |
       | nodeAggregateId           | "lady-eleonode-rootford" |
       | originDimensionSpacePoint | {"language":"de"}        |
-      | propertyValues            | {}                       |
+      | propertyValues            | {"text":"New text"}      |
     Then the last command should have thrown an exception of type "NodeAggregateIsRoot"
 
   Scenario: Try to set properties in an origin dimension space point that does not exist

Neos.ContentRepository.Core/Classes/EventStore/Events.php

@@ -4,22 +4,33 @@
 
 namespace Neos\ContentRepository\Core\EventStore;
 
+use Neos\EventStore\EventStoreInterface;
+
 /**
- * A set of Content Repository "domain events"
+ * A set of Content Repository "domain events", part of {@see EventsToPublish}
+ *
+ * For better type checking we ensure that this collection is never empty.
+ * That is because {@see EventStoreInterface::commit()} will throw an exception if there are 0 events passed:
+ *
+ * > Writable events must contain at least one event
+ *
+ * We do not skip the case for 0 events to ensure each command always maps to a mutation.
+ * Forgiving noop behaviour is not intended for this low level code.
  *
  * @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
@@ -29,11 +40,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
@@ -43,21 +54,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);
     }
 }

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<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.
@@ -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);
@@ -156,7 +161,7 @@ private function deleteObsoleteTetheredNodesWhenChangingNodeType(
             }
         }
 
-        return Events::fromArray($events);
+        return $events;
     }
 
     /**

Neos.ContentRepository.Core/Classes/Feature/Common/NodeVariationInternals.php

@@ -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,
@@ -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,
@@ -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,

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<EventInterface>
+     */
     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(

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,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())
@@ -261,6 +262,7 @@ private function createRegularWithNode(
     /**
      * @throws ContentStreamDoesNotExistYet
      * @throws NodeTypeNotFound
+     * @return array<EventInterface>
      */
     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,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;
     }
 }

Neos.ContentRepository.Core/Classes/Feature/NodeModification/Command/SetNodeProperties.php

@@ -48,6 +48,9 @@ private function __construct(
         public OriginDimensionSpacePoint $originDimensionSpacePoint,
         public PropertyValuesToWrite $propertyValues,
     ) {
+        if ($this->propertyValues->isEmpty()) {
+            throw new \InvalidArgumentException(sprintf('The command "SetNodeProperties" for node %s must contain property values', $this->nodeAggregateId->value), 1733394351);
+        }
     }
 
     /**

Neos.ContentRepository.Core/Classes/Feature/NodeModification/Dto/PropertyValuesToWrite.php

@@ -95,4 +95,9 @@ public function getPropertiesToUnset(): PropertyNames
             )
         );
     }
+
+    public function isEmpty(): bool
+    {
+        return $this->values === [];
+    }
 }

Neos.ContentRepository.Core/Classes/Feature/NodeModification/NodeModification.php

@@ -129,6 +129,11 @@ private function handleSetSerializedNodeProperties(
 
         $events = $this->mergeSplitEvents($events);
 
+        if ($events === []) {
+            // cannot happen here as the command could not be instantiated without any intention see constructor validation
+            throw new \RuntimeException('Cannot handle "SetSerializedNodeProperties" with no properties to modify', 1736798016);
+        }
+
         return new EventsToPublish(
             ContentStreamEventStreamName::fromContentStreamId($contentGraph->getContentStreamId())
                 ->getEventStreamName(),

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/Command/SetNodeReferences.php

@@ -36,6 +36,9 @@ private function __construct(
         public OriginDimensionSpacePoint $sourceOriginDimensionSpacePoint,
         public NodeReferencesToWrite $references,
     ) {
+        if ($this->references->isEmpty()) {
+            throw new \InvalidArgumentException(sprintf('The command "SetNodeReferences" for node %s must contain references to modify', $this->sourceNodeAggregateId->value), 1736797678);
+        }
     }
 
     /**

Neos.ContentRepository.Core/Classes/Feature/NodeReferencing/NodeReferencing.php

@@ -148,6 +148,11 @@ private function handleSetSerializedNodeReferences(
             );
         }
 
+        if ($events === []) {
+            // cannot happen here as the command could not be instantiated without any intention see constructor validation
+            throw new \RuntimeException('Cannot handle "SetSerializedNodeReferences" with no references to modify', 1736797975);
+        }
+
         $events = Events::fromArray($events);
 
         return new EventsToPublish(