Merge pull request #5406 from mhsdesign/bugfix/orphaned-asset-usage-e…

…ntries

BUGFIX: Asset usage prevent orphaned asset usage entries in case of a rebase with conflicts

Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W6-WorkspaceRebasing/01-BasicFeatures.feature

@@ -62,6 +62,14 @@ Feature: Rebasing with no conflict
       | rebaseErrorHandlingStrategy | "force"               |
     Then I expect the content stream "user-cs-identifier" to not exist
 
+    Then I expect exactly 2 events to be published on stream with prefix "Workspace:user-test"
+    And event at index 1 is of type "WorkspaceWasRebased" with payload:
+      | Key                     | Expected             |
+      | workspaceName           | "user-test"          |
+      | newContentStreamId      | "user-cs-rebased"    |
+      | previousContentStreamId | "user-cs-identifier" |
+      | 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;{}
 

Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W6-WorkspaceRebasing/03-RebasingWithConflictingChanges.feature

@@ -179,6 +179,14 @@ Feature: Workspace rebasing - conflicting changes
       | rebasedContentStreamId      | "user-cs-identifier-rebased" |
       | rebaseErrorHandlingStrategy | "force"                      |
 
+    Then I expect exactly 2 events to be published on stream with prefix "Workspace:user-ws"
+    And event at index 1 is of type "WorkspaceWasRebased" with payload:
+      | Key                     | Expected                     |
+      | workspaceName           | "user-ws"                    |
+      | newContentStreamId      | "user-cs-identifier-rebased" |
+      | previousContentStreamId | "user-cs-identifier"         |
+      | 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
 

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,6 +289,7 @@ private function rebaseWorkspaceWithoutChanges(
                     $workspace->workspaceName,
                     $newContentStreamId,
                     $workspace->currentContentStreamId,
+                    skippedEvents: []
                 ),
             ),
             ExpectedVersion::ANY()
@@ -404,6 +406,8 @@ static function ($handle) use ($rebaseableCommands): void {
                         $command->workspaceName,
                         $command->rebasedContentStreamId,
                         $workspace->currentContentStreamId,
+                        skippedEvents: $commandSimulator->getConflictingEvents()
+                            ->map(fn (ConflictingEvent $conflictingEvent) => $conflictingEvent->getSequenceNumber())
                     ),
                 ),
                 ExpectedVersion::ANY()

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);
+    }
 }

Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Event/WorkspaceWasRebased.php

@@ -16,8 +16,10 @@
 
 use Neos\ContentRepository\Core\EventStore\EventInterface;
 use Neos\ContentRepository\Core\Feature\Common\EmbedsWorkspaceName;
+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
@@ -34,6 +36,11 @@ public function __construct(
          * The old content stream ID (which is not active anymore now)
          */
         public ContentStreamId $previousContentStreamId,
+        /**
+         * @var list<SequenceNumber>
+         * @internal actually conflicting event's sequence numbers: only for debugging & testing please use {@see hasSkippedEvents()} which is API instead.
+         */
+        public array $skippedEvents
     ) {
     }
 
@@ -42,17 +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']),
+            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)
+        ];
     }
 }

Neos.Neos/Classes/AssetUsage/CatchUpHook/AssetUsageCatchUpHook.php

@@ -16,6 +16,7 @@
 use Neos\ContentRepository\Core\Feature\NodeVariation\Event\NodePeerVariantWasCreated;
 use Neos\ContentRepository\Core\Feature\NodeVariation\Event\NodeSpecializationVariantWasCreated;
 use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasDiscarded;
+use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Event\WorkspaceWasRebased;
 use Neos\ContentRepository\Core\Projection\CatchUpHook\CatchUpHookInterface;
 use Neos\ContentRepository\Core\Projection\ContentGraph\ContentGraphReadModelInterface;
 use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindDescendantNodesFilter;
@@ -73,6 +74,7 @@ public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $event
             }
         }
 
+        // Note that we don't need to update the index for WorkspaceWasPublished, as updateNode will be invoked already with the published node and then clean up its previous usages in nested workspaces
         match ($eventInstance::class) {
             NodeAggregateWithNodeWasCreated::class => $this->updateNode($eventInstance->getWorkspaceName(), $eventInstance->nodeAggregateId, $eventInstance->originDimensionSpacePoint->toDimensionSpacePoint()),
             NodePeerVariantWasCreated::class => $this->updateNode($eventInstance->getWorkspaceName(), $eventInstance->nodeAggregateId, $eventInstance->peerOrigin->toDimensionSpacePoint()),
@@ -81,6 +83,8 @@ public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $event
             NodePropertiesWereSet::class => $this->updateNode($eventInstance->getWorkspaceName(), $eventInstance->nodeAggregateId, $eventInstance->originDimensionSpacePoint->toDimensionSpacePoint()),
             WorkspaceWasDiscarded::class => $this->discardWorkspace($eventInstance->getWorkspaceName()),
             DimensionSpacePointWasMoved::class => $this->updateDimensionSpacePoint($eventInstance->getWorkspaceName(), $eventInstance->source, $eventInstance->target),
+            // 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)
+            WorkspaceWasRebased::class => $eventInstance->hasSkippedEvents() && $this->discardWorkspace($eventInstance->getWorkspaceName()),
             default => null
         };
     }