Merge pull request #4284 from neos/90-do-not-crash-when-node-already-…

…disabled-enabled

BUGFIX: do not crash when node already disabled/enabled

Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/06-NodeDisabling/01-DisableNodeAggregate_ConstraintChecks.feature

@@ -55,12 +55,19 @@ Feature: Constraint checks on node aggregate disabling
       | nodeVariantSelectionStrategy | "allVariants"                          |
     And the graph projection is fully up to date
 
-    When the command DisableNodeAggregate is executed with payload and exceptions are caught:
+      # Note: The behavior has been changed with https://github.com/neos/neos-development-collection/pull/4284 and the test was adjusted accordingly
+    When the command DisableNodeAggregate is executed with payload:
       | Key                          | Value                                  |
       | nodeAggregateId      | "sir-david-nodenborough"               |
       | coveredDimensionSpacePoint   | {"language": "de"}                                     |
       | nodeVariantSelectionStrategy | "allVariants"                          |
-    Then the last command should have thrown an exception of type "NodeAggregateCurrentlyDisablesDimensionSpacePoint"
+    Then I expect exactly 4 events to be published on stream with prefix "ContentStream:cs-identifier"
+    And event at index 3 is of type "NodeAggregateWasDisabled" with payload:
+      | Key                          | Expected                               |
+      | contentStreamId              | "cs-identifier"                        |
+      | nodeAggregateId              | "sir-david-nodenborough"               |
+      | affectedDimensionSpacePoints | [{"language":"de"},{"language":"gsw"}] |
+
 
   Scenario: Try to disable a node aggregate in a non-existing dimension space point
     When the command DisableNodeAggregate is executed with payload and exceptions are caught:

Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/06-NodeDisabling/04-EnableNodeAggregate_ConstraintChecks.feature

@@ -47,12 +47,13 @@ Feature: Enable a node aggregate
       | nodeVariantSelectionStrategy | "allVariants"    |
     Then the last command should have thrown an exception of type "NodeAggregateCurrentlyDoesNotExist"
 
+    # Note: The behavior has been changed with https://github.com/neos/neos-development-collection/pull/4284 and the test was adjusted accordingly
   Scenario: Try to enable an already enabled node aggregate
-    When the command EnableNodeAggregate is executed with payload and exceptions are caught:
+    When the command EnableNodeAggregate is executed with payload:
       | Key                          | Value                    |
       | nodeAggregateId      | "sir-david-nodenborough" |
       | nodeVariantSelectionStrategy | "allVariants"            |
-    Then the last command should have thrown an exception of type "NodeAggregateCurrentlyDoesNotDisableDimensionSpacePoint"
+    Then I expect exactly 3 events to be published on stream with prefix "ContentStream:cs-identifier"
 
   Scenario: Try to enable a node aggregate in a non-existing dimension space point
     When the command EnableNodeAggregate is executed with payload and exceptions are caught:

Neos.ContentRepository.Core/Classes/CommandHandler/CommandResult.php

@@ -7,6 +7,7 @@
 use Neos\ContentRepository\Core\ContentRepository;
 use Neos\ContentRepository\Core\Projection\ProjectionInterface;
 use Neos\ContentRepository\Core\Projection\ProjectionStateInterface;
+use Neos\EventStore\Model\Event\Version;
 use Neos\EventStore\Model\EventStore\CommitResult;
 use Neos\EventStore\Model\Event\SequenceNumber;
 
@@ -25,6 +26,21 @@ public function __construct(
     ) {
     }
 
+    /**
+     * an empty command result which should not result in projection updates
+     * @return self
+     */
+    public static function empty(): self
+    {
+        return new self(
+            PendingProjections::empty(),
+            new CommitResult(
+                Version::first(),
+                SequenceNumber::none()
+            )
+        );
+    }
+
     /**
      * Wait until all projections are up to date; i.e. have processed the events.
      *

Neos.ContentRepository.Core/Classes/CommandHandler/PendingProjections.php

@@ -85,6 +85,11 @@ public function __construct(
     ) {
     }
 
+    public static function empty(): self
+    {
+        return new self(Projections::create(), []);
+    }
+
     public static function fromProjectionsAndEventsAndSequenceNumber(
         Projections $allProjections,
         Events $events,

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

@@ -15,6 +15,7 @@
 use Neos\EventStore\Model\Event\EventId;
 use Neos\EventStore\Model\Event\EventMetadata;
 use Neos\EventStore\Model\Events;
+use Neos\EventStore\Model\EventStore\CommitResult;
 
 /**
  * Internal service to persist {@see EventInterface} with the proper normalization, and triggering the
@@ -39,6 +40,9 @@ public function __construct(
      */
     public function publishEvents(EventsToPublish $eventsToPublish): CommandResult
     {
+        if ($eventsToPublish->events->isEmpty()) {
+            return CommandResult::empty();
+        }
         // the following logic could also be done in an AppEventStore::commit method (being called
         // directly from the individual Command Handlers).
         $normalizedEvents = Events::fromArray(

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

@@ -49,4 +49,9 @@ public function map(\Closure $callback): array
     {
         return array_map($callback, $this->events);
     }
+
+    public function isEmpty(): bool
+    {
+        return empty($this->events);
+    }
 }

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

@@ -24,4 +24,13 @@ public function __construct(
         public readonly ExpectedVersion $expectedVersion,
     ) {
     }
+
+    public static function empty(): self
+    {
+        return new EventsToPublish(
+            StreamName::fromString("empty"),
+            Events::fromArray([]),
+            ExpectedVersion::ANY()
+        );
+    }
 }

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

@@ -609,39 +609,6 @@ protected function requireNodeAggregateToNotOccupyDimensionSpacePoint(
         }
     }
 
-    /**
-     * @throws NodeAggregateCurrentlyDoesNotDisableDimensionSpacePoint
-     */
-    protected function requireNodeAggregateToDisableDimensionSpacePoint(
-        NodeAggregate $nodeAggregate,
-        DimensionSpacePoint $dimensionSpacePoint
-    ): void {
-        if (!$nodeAggregate->disablesDimensionSpacePoint($dimensionSpacePoint)) {
-            throw new NodeAggregateCurrentlyDoesNotDisableDimensionSpacePoint(
-                'Node aggregate "' . $nodeAggregate->nodeAggregateId->value
-                    . '" currently does not disable dimension space point '
-                    . json_encode($dimensionSpacePoint) . '.',
-                1557735431
-            );
-        }
-    }
-
-    /**
-     * @throws NodeAggregateCurrentlyDisablesDimensionSpacePoint
-     */
-    protected function requireNodeAggregateToNotDisableDimensionSpacePoint(
-        NodeAggregate $nodeAggregate,
-        DimensionSpacePoint $dimensionSpacePoint
-    ): void {
-        if ($nodeAggregate->disablesDimensionSpacePoint($dimensionSpacePoint)) {
-            throw new NodeAggregateCurrentlyDisablesDimensionSpacePoint(
-                'Node aggregate "' . $nodeAggregate->nodeAggregateId->value
-                    . '" currently disables dimension space point ' . json_encode($dimensionSpacePoint) . '.',
-                1555179563
-            );
-        }
-    }
-
     protected function validateReferenceProperties(
         ReferenceName $referenceName,
         PropertyValuesToWrite $referenceProperties,

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

@@ -60,10 +60,11 @@ private function handleDisableNodeAggregate(
             $nodeAggregate,
             $command->coveredDimensionSpacePoint
         );
-        $this->requireNodeAggregateToNotDisableDimensionSpacePoint(
-            $nodeAggregate,
-            $command->coveredDimensionSpacePoint
-        );
+
+        if ($nodeAggregate->disablesDimensionSpacePoint($command->coveredDimensionSpacePoint)) {
+            // already disabled, so we can return a no-operation.
+            return EventsToPublish::empty();
+        }
 
         $affectedDimensionSpacePoints = $command->nodeVariantSelectionStrategy
             ->resolveAffectedDimensionSpacePoints(
@@ -113,10 +114,11 @@ public function handleEnableNodeAggregate(
             $nodeAggregate,
             $command->coveredDimensionSpacePoint
         );
-        $this->requireNodeAggregateToDisableDimensionSpacePoint(
-            $nodeAggregate,
-            $command->coveredDimensionSpacePoint
-        );
+
+        if (!$nodeAggregate->disablesDimensionSpacePoint($command->coveredDimensionSpacePoint)) {
+            // already enabled, so we can return a no-operation.
+            return EventsToPublish::empty();
+        }
 
         $affectedDimensionSpacePoints = $command->nodeVariantSelectionStrategy
             ->resolveAffectedDimensionSpacePoints(