TASK: Rename to hasSkippedEvents and store originally conflicting s…

…equence numbers in force-rebase event Storing the skipped sequence numbers should ease debugging rather than having 'just' a boolean indicator. The public api does not need to know about that thus the getter.
neos · Jan 9, 2025 · 1c4f941 · 1c4f941
1 parent e7d185c
commit 1c4f941
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 9 deletions.
diff --git a/...ory.BehavioralTests/Tests/Behavior/Features/W6-WorkspaceRebasing/01-BasicFeatures.feature b/...ory.BehavioralTests/Tests/Behavior/Features/W6-WorkspaceRebasing/01-BasicFeatures.feature
@@ -68,7 +68,7 @@ Feature: Rebasing with no conflict
       | workspaceName           | "user-test"          |
       | newContentStreamId      | "user-cs-rebased"    |
       | previousContentStreamId | "user-cs-identifier" |
-      | hadConflicts            | false                |
+      | skippedEvents           | []                   |
 
     When I am in workspace "user-test" and dimension space point {}
     Then I expect node aggregate identifier "sir-david-nodenborough" to lead to node user-cs-rebased;sir-david-nodenborough;{}

diff --git a/...ts/Tests/Behavior/Features/W6-WorkspaceRebasing/03-RebasingWithConflictingChanges.feature b/...ts/Tests/Behavior/Features/W6-WorkspaceRebasing/03-RebasingWithConflictingChanges.feature
@@ -185,7 +185,7 @@ Feature: Workspace rebasing - conflicting changes
       | workspaceName           | "user-ws"                    |
       | newContentStreamId      | "user-cs-identifier-rebased" |
       | previousContentStreamId | "user-cs-identifier"         |
-      | hadConflicts            | true                         |
+      | skippedEvents           | [12,14]                      |
 
     Then I expect the content stream "user-cs-identifier" to not exist
     Then I expect the content stream "user-cs-identifier-rebased" to exist

diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php
@@ -49,6 +49,7 @@
 use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasDiscarded;
 use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasPublished;
 use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Command\RebaseWorkspace;
+use Neos\ContentRepository\Core\Feature\WorkspaceRebase\ConflictingEvent;
 use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Dto\RebaseErrorHandlingStrategy;
 use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Event\WorkspaceWasRebased;
 use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\PartialWorkspaceRebaseFailed;
@@ -288,7 +289,7 @@ private function rebaseWorkspaceWithoutChanges(
                     $workspace->workspaceName,
                     $newContentStreamId,
                     $workspace->currentContentStreamId,
-                    hadConflicts: false
+                    skippedEvents: []
                 ),
             ),
             ExpectedVersion::ANY()
@@ -405,7 +406,8 @@ static function ($handle) use ($rebaseableCommands): void {
                         $command->workspaceName,
                         $command->rebasedContentStreamId,
                         $workspace->currentContentStreamId,
-                        hadConflicts: $commandSimulator->hasConflicts()
+                        skippedEvents: $commandSimulator->getConflictingEvents()
+                            ->map(fn (ConflictingEvent $conflictingEvent) => $conflictingEvent->getSequenceNumber())
                     ),
                 ),
                 ExpectedVersion::ANY()

diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/ConflictingEvents.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/ConflictingEvents.php
@@ -55,4 +55,14 @@ public function count(): int
     {
         return count($this->items);
     }
+
+    /**
+     * @template T
+     * @param \Closure(ConflictingEvent): T $callback
+     * @return list<T>
+     */
+    public function map(\Closure $callback): array
+    {
+        return array_map($callback, $this->items);
+    }
 }
diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Event/WorkspaceWasRebased.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Event/WorkspaceWasRebased.php
@@ -19,6 +19,7 @@
 use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Dto\RebaseErrorHandlingStrategy;
 use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
 use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;
+use Neos\EventStore\Model\Event\SequenceNumber;
 
 /**
  * @api events are the persistence-API of the content repository
@@ -36,9 +37,10 @@ public function __construct(
          */
         public ContentStreamId $previousContentStreamId,
         /**
-         * Indicates if failing changes were discarded during a forced rebase {@see RebaseErrorHandlingStrategy::STRATEGY_FORCE} or if all events in the workspace were kept.
+         * @var list<SequenceNumber>
+         * @internal actually conflicting event's sequence numbers: only for debugging & testing please use {@see hasSkippedEvents()} which is API instead.
          */
-        public bool $hadConflicts
+        public array $skippedEvents
     ) {
     }
 
@@ -47,18 +49,32 @@ public function getWorkspaceName(): WorkspaceName
         return $this->workspaceName;
     }
 
+    /**
+     * Indicates if failing changes were discarded during a forced rebase {@see RebaseErrorHandlingStrategy::STRATEGY_FORCE} or if all events in the workspace were kept.
+     */
+    public function hasSkippedEvents(): bool
+    {
+        return $this->skippedEvents !== [];
+    }
+
     public static function fromArray(array $values): self
     {
         return new self(
             WorkspaceName::fromString($values['workspaceName']),
             ContentStreamId::fromString($values['newContentStreamId']),
             ContentStreamId::fromString($values['previousContentStreamId']),
-            $values['hadConflicts'] ?? false
+            array_map(SequenceNumber::fromInteger(...), $values['skippedEvents'] ?? [])
         );
     }
 
     public function jsonSerialize(): array
     {
-        return get_object_vars($this);
+        return [
+            'workspaceName' => $this->workspaceName,
+            'newContentStreamId' => $this->newContentStreamId,
+            'previousContentStreamId' => $this->previousContentStreamId,
+            // todo SequenceNumber is NOT jsonSerializeAble!!!
+            'skippedEvents' => array_map(fn (SequenceNumber $sequenceNumber) => $sequenceNumber->value, $this->skippedEvents)
+        ];
     }
 }
diff --git a/Neos.Neos/Classes/AssetUsage/CatchUpHook/AssetUsageCatchUpHook.php b/Neos.Neos/Classes/AssetUsage/CatchUpHook/AssetUsageCatchUpHook.php
@@ -74,7 +74,7 @@ public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $event
             }
         }
 
-        if ($eventInstance instanceof WorkspaceWasRebased && $eventInstance->hadConflicts) {
+        if ($eventInstance instanceof WorkspaceWasRebased && $eventInstance->hasSkippedEvents()) {
             // because we don't know which changes were discarded in a conflict, we discard all changes and will build up the index on succeeding calls (with the kept reapplied events)
             $this->discardWorkspace($eventInstance->getWorkspaceName());
             return;