Skip to content

Commit

Permalink
Merge pull request #5370 from mhsdesign/bugfix/5364-throw-better-exce…
Browse files Browse the repository at this point in the history
…ptions-if-workspace-was-up-to-date-and-publish-didnt-work

BUGFIX: Throw `PartialWorkspaceRebaseFailed` if workspace was up to date and publish didnt work
  • Loading branch information
kitsunet authored Dec 1, 2024
2 parents 76547ec + d5a616f commit 7d648f2
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Feature: Workspace discarding - complex chained functionality
| workspaceName | "user-ws" |
| nodesToDiscard | [{"workspaceName": "user-ws", "dimensionSpacePoint": {"language": "en"}, "nodeAggregateId": "sir-david-nodenborough"}, {"workspaceName": "user-ws", "dimensionSpacePoint": {"language": "en"}, "nodeAggregateId": "sir-david-nodenborough"}] |
| newContentStreamId | "user-cs-id-rebased" |
Then the last command should have thrown the WorkspaceRebaseFailed exception with:
Then the last command should have thrown the PartialWorkspaceRebaseFailed exception with:
| SequenceNumber | Event | Exception |
| 11 | NodeGeneralizationVariantWasCreated | NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ Feature: Workspace publication - complex chained functionality
| originDimensionSpacePoint | {"language": "de"} |
| propertyValues | {"text": "Modified text"} |

Then workspace user-ws has status OUTDATED

When the command PublishIndividualNodesFromWorkspace is executed with payload and exceptions are caught:
| Key | Value |
| workspaceName | "user-ws" |
Expand All @@ -101,12 +103,14 @@ Feature: Workspace publication - complex chained functionality
| sourceOrigin | {"language": "de"} |
| targetOrigin | {"language": "en"} |

Then workspace user-ws has status UP_TO_DATE

When the command PublishIndividualNodesFromWorkspace is executed with payload and exceptions are caught:
| Key | Value |
| workspaceName | "user-ws" |
| nodesToPublish | [{"workspaceName": "user-ws", "dimensionSpacePoint": {"language": "en"}, "nodeAggregateId": "nody-mc-nodeface"}] |
| newContentStreamId | "user-cs-id-rebased" |
Then the last command should have thrown the WorkspaceRebaseFailed exception with:
Then the last command should have thrown the PartialWorkspaceRebaseFailed exception with:
| SequenceNumber | Event | Exception |
| 13 | NodeGeneralizationVariantWasCreated | NodeAggregateDoesCurrentlyNotCoverDimensionSpacePoint |

Expand All @@ -116,3 +120,32 @@ Feature: Workspace publication - complex chained functionality
| newContentStreamId | "user-cs-id-yet-again-rebased" |
When I am in workspace "user-ws" and dimension space point {"language": "de"}
Then I expect node aggregate identifier "nody-mc-nodeface" to lead to node user-cs-id-yet-again-rebased;nody-mc-nodeface;{"language": "de"}

Scenario: Publish a deletion and try to keep a move node from its descendants
see issue: https://github.com/neos/neos-development-collection/issues/5364

When the command MoveNodeAggregate is executed with payload:
| Key | Value |
| workspaceName | "user-ws" |
| nodeAggregateId | "nody-mc-nodeface" |
| dimensionSpacePoint | {"language": "de"} |
| newParentNodeAggregateId | "sir-nodebelig" |
| relationDistributionStrategy | "gatherAll" |

When the command RemoveNodeAggregate is executed with payload:
| Key | Value |
| workspaceName | "user-ws" |
| nodeAggregateId | "sir-david-nodenborough" |
| coveredDimensionSpacePoint | {"language": "de"} |
| nodeVariantSelectionStrategy | "allVariants" |

Then workspace user-ws has status UP_TO_DATE

When the command PublishIndividualNodesFromWorkspace is executed with payload and exceptions are caught:
| Key | Value |
| workspaceName | "user-ws" |
| nodesToPublish | [{"dimensionSpacePoint": {"language": "de"}, "nodeAggregateId": "sir-david-nodenborough"}] |
| newContentStreamId | "user-cs-id-rebased" |
Then the last command should have thrown the PartialWorkspaceRebaseFailed exception with:
| SequenceNumber | Event | Exception |
| 11 | NodeAggregateWasMoved | NodeAggregateCurrentlyDoesNotExist |
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Command\RebaseWorkspace;
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Dto\RebaseErrorHandlingStrategy;
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Event\WorkspaceWasRebased;
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\PartialWorkspaceRebaseFailed;
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\WorkspaceRebaseFailed;
use Neos\ContentRepository\Core\SharedModel\Exception\ContentStreamAlreadyExists;
use Neos\ContentRepository\Core\SharedModel\Exception\ContentStreamDoesNotExistYet;
Expand Down Expand Up @@ -507,8 +508,14 @@ static function ($handle) use ($commandSimulator, $matchingCommands, $remainingC
yield $this->reopenContentStreamWithoutConstraintChecks(
$workspace->currentContentStreamId
);

throw WorkspaceRebaseFailed::duringPublish($commandSimulator->getConflictingEvents());
match ($workspace->status) {
// If the workspace is up-to-date it must be a problem regarding that the order of events cannot be changed
WorkspaceStatus::UP_TO_DATE =>
throw PartialWorkspaceRebaseFailed::duringPartialPublish($commandSimulator->getConflictingEvents()),
// If the workspace is outdated we cannot know for sure but suspect that the conflict arose due to changes in the base workspace.
WorkspaceStatus::OUTDATED =>
throw WorkspaceRebaseFailed::duringPublish($commandSimulator->getConflictingEvents())
};
}

// 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
Expand Down Expand Up @@ -608,7 +615,6 @@ private function handleDiscardIndividualNodesFromWorkspace(
// quick path everything was discarded
yield from $this->discardWorkspace(
$workspace,
$workspaceContentStreamVersion,
$baseWorkspace,
$baseWorkspaceContentStreamVersion,
$command->newContentStreamId
Expand All @@ -630,7 +636,14 @@ static function ($handle) use ($commandsToKeep): void {
yield $this->reopenContentStreamWithoutConstraintChecks(
$workspace->currentContentStreamId
);
throw WorkspaceRebaseFailed::duringDiscard($commandSimulator->getConflictingEvents());
match ($workspace->status) {
// If the workspace is up-to-date it must be a problem regarding that the order of events cannot be changed
WorkspaceStatus::UP_TO_DATE =>
throw PartialWorkspaceRebaseFailed::duringPartialDiscard($commandSimulator->getConflictingEvents()),
// If the workspace is outdated we cannot know for sure but suspect that the conflict arose due to changes in the base workspace.
WorkspaceStatus::OUTDATED =>
throw WorkspaceRebaseFailed::duringDiscard($commandSimulator->getConflictingEvents())
};
}

yield from $this->forkNewContentStreamAndApplyEvents(
Expand Down Expand Up @@ -675,12 +688,11 @@ private function handleDiscardWorkspace(
return;
}

$workspaceContentStreamVersion = $this->requireOpenContentStreamAndVersion($workspace, $commandHandlingDependencies);
$this->requireContentStreamToNotBeClosed($workspace->currentContentStreamId, $commandHandlingDependencies);
$baseWorkspaceContentStreamVersion = $this->requireOpenContentStreamAndVersion($baseWorkspace, $commandHandlingDependencies);

yield from $this->discardWorkspace(
$workspace,
$workspaceContentStreamVersion,
$baseWorkspace,
$baseWorkspaceContentStreamVersion,
$command->newContentStreamId
Expand All @@ -692,7 +704,6 @@ private function handleDiscardWorkspace(
*/
private function discardWorkspace(
Workspace $workspace,
Version $workspaceContentStreamVersion,
Workspace $baseWorkspace,
Version $baseWorkspaceContentStreamVersion,
ContentStreamId $newContentStream
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<?php

/*
* This file is part of the Neos.ContentRepository.Core package.
*
* (c) Contributors of the Neos Project - www.neos.io
*
* This package is Open Source Software. For the full copyright and license
* information, please view the LICENSE file which was distributed with this
* source code.
*/

declare(strict_types=1);

namespace Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception;

use Neos\ContentRepository\Core\Feature\WorkspaceRebase\ConflictingEvents;

/**
* Thrown if the partial publish/discard cannot work because the events cannot be reordered as filtered.
*
* This can happen for cases like attempting to publish a removal first and wanting as remaining change
* a node move out of the removed descendants or publishing a node variant creation before the node is created.
*
* We cannot reliably detect these cases in advance but in case the workspace is up-to-date its most likely such
* an ordering conflict.
*
* To solve the problem the partial operation should be retried with a different filter _or_ a full publish/discard is required.
*
* If the workspace is outdated we cannot know for sure but suspect first that the conflict arose due to changes
* in the base workspace, thus we throw {@see WorkspaceRebaseFailed} instead.
* A forced rebase then might not solve the problem if It's because the order of events cannot be changed.
* But attempting a second partial publish/discard (with up-to-date workspace) this exception will be thrown and can be reacted upon.
*
* @see WorkspaceRebaseFailed which is thrown instead if the workspace is also outdated
* @api this exception contains information which events cannot be reordered
*/
final class PartialWorkspaceRebaseFailed extends \RuntimeException
{
private function __construct(
public readonly ConflictingEvents $conflictingEvents,
string $message,
int $code,
?\Throwable $previous,
) {
parent::__construct($message, $code, $previous);
}

public static function duringPartialPublish(ConflictingEvents $conflictingEvents): self
{
return new self(
$conflictingEvents,
sprintf('Publication failed, events cannot be reordered as filtered: %s', self::renderMessage($conflictingEvents)),
1729974980,
$conflictingEvents->first()?->getException()
);
}

public static function duringPartialDiscard(ConflictingEvents $conflictingEvents): self
{
return new self(
$conflictingEvents,
sprintf('Discard failed, events cannot be reordered as filtered: %s', self::renderMessage($conflictingEvents)),
1729974982,
$conflictingEvents->first()?->getException()
);
}

private static function renderMessage(ConflictingEvents $conflictingEvents): string
{
$firstConflict = $conflictingEvents->first();
return sprintf('"%s" and %d further ordering conflicts', $firstConflict?->getException()->getMessage(), count($conflictingEvents) - 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/**
* @api this exception contains information about what exactly went wrong during rebase
*/
final class WorkspaceRebaseFailed extends \Exception
final class WorkspaceRebaseFailed extends \RuntimeException
{
private function __construct(
public readonly ConflictingEvents $conflictingEvents,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
use Neos\ContentRepository\Core\Feature\WorkspacePublication\Command\PublishIndividualNodesFromWorkspace;
use Neos\ContentRepository\Core\Feature\WorkspacePublication\Command\PublishWorkspace;
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Command\RebaseWorkspace;
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\PartialWorkspaceRebaseFailed;
use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Exception\WorkspaceRebaseFailed;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName;
Expand Down Expand Up @@ -310,7 +311,7 @@ protected function publishEvent(string $eventType, StreamName $streamName, array
*/
public function theLastCommandShouldHaveThrown(string $shortExceptionName, ?int $expectedCode = null, PyStringNode $expectedMessage = null): void
{
if ($shortExceptionName === 'WorkspaceRebaseFailed') {
if ($shortExceptionName === 'WorkspaceRebaseFailed' || $shortExceptionName === 'PartialWorkspaceRebaseFailed') {
throw new \RuntimeException('Please use the assertion "the last command should have thrown the WorkspaceRebaseFailed exception with" instead.');
}

Expand All @@ -332,14 +333,18 @@ public function theLastCommandShouldHaveThrown(string $shortExceptionName, ?int
}

/**
* @Then the last command should have thrown the WorkspaceRebaseFailed exception with:
* @Then /^the last command should have thrown the (WorkspaceRebaseFailed|PartialWorkspaceRebaseFailed) exception with:$/
*/
public function theLastCommandShouldHaveThrownTheWorkspaceRebaseFailedWith(TableNode $payloadTable)
public function theLastCommandShouldHaveThrownTheWorkspaceRebaseFailedWith(string $shortExceptionName, TableNode $payloadTable)
{
/** @var WorkspaceRebaseFailed $exception */
/** @var WorkspaceRebaseFailed|PartialWorkspaceRebaseFailed $exception */
$exception = $this->lastCommandException;
Assert::assertNotNull($exception, 'Command did not throw exception');
Assert::assertInstanceOf(WorkspaceRebaseFailed::class, $exception, sprintf('Actual exception: %s (%s): %s', get_class($exception), $exception->getCode(), $exception->getMessage()));

match ($shortExceptionName) {
'WorkspaceRebaseFailed' => Assert::assertInstanceOf(WorkspaceRebaseFailed::class, $exception, sprintf('Actual exception: %s (%s): %s', get_class($exception), $exception->getCode(), $exception->getMessage())),
'PartialWorkspaceRebaseFailed' => Assert::assertInstanceOf(PartialWorkspaceRebaseFailed::class, $exception, sprintf('Actual exception: %s (%s): %s', get_class($exception), $exception->getCode(), $exception->getMessage())),
};

$actualComparableHash = [];
foreach ($exception->conflictingEvents as $conflictingEvent) {
Expand Down

0 comments on commit 7d648f2

Please sign in to comment.