TASK: Do not send $commitResult to generator but calculate expected…

… version instead Also readd lost documentation and simplifies the `handle` The ->throw logic was initially introduced via neos#5315 but then removed again as we thought it was no longer needed.
mhsdesign · Nov 9, 2024 · 77778f9 · 77778f9
1 parent 0273e32
commit 77778f9
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 23 deletions.
diff --git a/Neos.ContentRepository.Core/Classes/CommandHandler/CommandHandlerInterface.php b/Neos.ContentRepository.Core/Classes/CommandHandler/CommandHandlerInterface.php
@@ -5,14 +5,13 @@
 namespace Neos\ContentRepository\Core\CommandHandler;
 
 use Neos\ContentRepository\Core\EventStore\EventsToPublish;
-use Neos\EventStore\Model\EventStore\CommitResult;
 
 /**
  * Common interface for all Content Repository command handlers
  *
  * The {@see CommandHandlingDependencies} are available during handling to do soft-constraint checks
  *
- * @phpstan-type YieldedEventsToPublish \Generator<int, EventsToPublish, CommitResult|null, void>
+ * @phpstan-type YieldedEventsToPublish \Generator<int, EventsToPublish>
  * @internal no public API, because commands are no extension points of the CR
  */
 interface CommandHandlerInterface

diff --git a/Neos.ContentRepository.Core/Classes/ContentRepository.php b/Neos.ContentRepository.Core/Classes/ContentRepository.php
@@ -110,23 +110,28 @@ public function handle(CommandInterface $command): void
 
         // control-flow aware command handling via generator
         try {
-            $yieldedEventsToPublish = $toPublish->current();
-            while ($yieldedEventsToPublish !== null) {
+            foreach ($toPublish as $yieldedEventsToPublish) {
                 if ($yieldedEventsToPublish->events->isEmpty()) {
-                    $yieldedEventsToPublish = $toPublish->send(null);
                     continue;
                 }
                 $eventsToPublish = $this->enrichEventsToPublishWithMetadata($yieldedEventsToPublish);
                 try {
-                    $commitResult = $this->eventPersister->publishWithoutCatchup($eventsToPublish);
+                    $this->eventPersister->publishWithoutCatchup($eventsToPublish);
                 } catch (ConcurrencyException $concurrencyException) {
+                    // we pass the exception into the generator (->throw), so it could be try-caught and reacted upon:
+                    //
+                    //   try {
+                    //      yield EventsToPublish(...);
+                    //   } catch (ConcurrencyException $e) {
+                    //      yield $this->reopenContentStream();
+                    //      throw $e;
+                    //   }
                     $yieldedErrorStrategy = $toPublish->throw($concurrencyException);
                     if ($yieldedErrorStrategy instanceof EventsToPublish) {
                         $this->eventPersister->publishWithoutCatchup($yieldedErrorStrategy);
                     }
                     throw $concurrencyException;
                 }
-                $yieldedEventsToPublish = $toPublish->send($commitResult);
             }
         } finally {
             // We always NEED to catchup even if there was an unexpected ConcurrencyException to make sure previous commits are handled.

diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php
@@ -247,15 +247,17 @@ static function ($handle) use ($rebaseableCommands): void {
             throw WorkspaceRebaseFailed::duringPublish($commandSimulator->getCommandsThatFailed());
         }
 
+        $eventsOfWorkspaceToPublish = $this->getCopiedEventsOfEventStream(
+            $baseWorkspace->workspaceName,
+            $baseWorkspace->currentContentStreamId,
+            $commandSimulator->eventStream(),
+        );
+
         try {
-            $commitResult = yield new EventsToPublish(
+            yield new EventsToPublish(
                 ContentStreamEventStreamName::fromContentStreamId($baseWorkspace->currentContentStreamId)
                     ->getEventStreamName(),
-                $this->getCopiedEventsOfEventStream(
-                    $baseWorkspace->workspaceName,
-                    $baseWorkspace->currentContentStreamId,
-                    $commandSimulator->eventStream(),
-                ),
+                $eventsOfWorkspaceToPublish,
                 ExpectedVersion::fromVersion($baseWorkspaceContentStreamVersion)
             );
         } catch (ConcurrencyException $concurrencyException) {
@@ -268,7 +270,7 @@ static function ($handle) use ($rebaseableCommands): void {
         yield $this->forkContentStream(
             $newContentStreamId,
             $baseWorkspace->currentContentStreamId,
-            $commitResult->highestCommittedVersion
+            Version::fromInteger($baseWorkspaceContentStreamVersion->value + $eventsOfWorkspaceToPublish->count())
         );
 
         yield new EventsToPublish(
@@ -507,16 +509,18 @@ static function ($handle) use ($commandSimulator, $matchingCommands, $remainingC
             throw WorkspaceRebaseFailed::duringPublish($commandSimulator->getCommandsThatFailed());
         }
 
+        // this could empty and a no-op for the rare case when a command returns empty events e.g. the node was already tagged with this subtree tag
+        $selectedEventsOfWorkspaceToPublish = $this->getCopiedEventsOfEventStream(
+            $baseWorkspace->workspaceName,
+            $baseWorkspace->currentContentStreamId,
+            $commandSimulator->eventStream()->withMaximumSequenceNumber($highestSequenceNumberForMatching),
+        );
+
         try {
-            // this could be a no-op for the rare case when a command returns empty events e.g. the node was already tagged with this subtree tag, meaning we actually just rebase
-            $commitResult = yield new EventsToPublish(
+            yield new EventsToPublish(
                 ContentStreamEventStreamName::fromContentStreamId($baseWorkspace->currentContentStreamId)
                     ->getEventStreamName(),
-                $this->getCopiedEventsOfEventStream(
-                    $baseWorkspace->workspaceName,
-                    $baseWorkspace->currentContentStreamId,
-                    $commandSimulator->eventStream()->withMaximumSequenceNumber($highestSequenceNumberForMatching),
-                ),
+                $selectedEventsOfWorkspaceToPublish,
                 ExpectedVersion::fromVersion($baseWorkspaceContentStreamVersion)
             );
         } catch (ConcurrencyException $concurrencyException) {
@@ -529,8 +533,7 @@ static function ($handle) use ($commandSimulator, $matchingCommands, $remainingC
         yield from $this->forkNewContentStreamAndApplyEvents(
             $command->contentStreamIdForRemainingPart,
             $baseWorkspace->currentContentStreamId,
-            // todo otherwise Features/W8-IndividualNodePublication/03-MoreBasicFeatures.feature:185 fails, see comment about emptiness above ... or should we manually count?
-            $commitResult?->highestCommittedVersion ?: $baseWorkspaceContentStreamVersion,
+            Version::fromInteger($baseWorkspaceContentStreamVersion->value + $selectedEventsOfWorkspaceToPublish->count()),
             new EventsToPublish(
                 WorkspaceEventStreamName::fromWorkspaceName($command->workspaceName)->getEventStreamName(),
                 Events::fromArray([