Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 5 commits into
base: 9.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;{}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -288,6 +289,7 @@ private function rebaseWorkspaceWithoutChanges(
$workspace->workspaceName,
$newContentStreamId,
$workspace->currentContentStreamId,
skippedEvents: []
),
),
ExpectedVersion::ANY()
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
) {
}

Expand All @@ -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)
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()),
Expand All @@ -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
};
}
Expand Down
Loading