From 41b5453d5116f1939cfe62f938888bbe5d83550d Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 15 Nov 2024 10:49:39 +0100 Subject: [PATCH 1/6] BUGFIX: #5364 throw better exceptions if workspace was up to date and publish didnt work --- .../Classes/Feature/WorkspaceCommandHandler.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php index a29b777754..214d65dde5 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php @@ -507,7 +507,11 @@ static function ($handle) use ($commandSimulator, $matchingCommands, $remainingC yield $this->reopenContentStreamWithoutConstraintChecks( $workspace->currentContentStreamId ); - + if ($workspace->status === WorkspaceStatus::UP_TO_DATE) { + throw new \RuntimeException('TODO cannot publish changeset as the leftover changes would not be applicable.'); + } + // todo either its a conflict with not applicable changes because the one change belongs to another OR the base workspace contains changes and we need to rebase first. + // we assume the latter and let the user up date the workspace first! throw WorkspaceRebaseFailed::duringPublish($commandSimulator->getConflictingEvents()); } @@ -630,6 +634,11 @@ static function ($handle) use ($commandsToKeep): void { yield $this->reopenContentStreamWithoutConstraintChecks( $workspace->currentContentStreamId ); + if ($workspace->status === WorkspaceStatus::UP_TO_DATE) { + throw new \RuntimeException('TODO cannot discard changeset as the leftover changes would not be applicable.'); + } + // todo either its a conflict with not applicable changes because the one change belongs to another OR the base workspace contains changes and we need to rebase first. + // we assume the latter and let the user up date the workspace first! throw WorkspaceRebaseFailed::duringDiscard($commandSimulator->getConflictingEvents()); } From 20cbe6bd30536da804fff7d3436436fd2484208a Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 15 Nov 2024 10:43:23 +0100 Subject: [PATCH 2/6] TASK: Cleanup method signature of `discardWorkspace` --- .../Classes/Feature/WorkspaceCommandHandler.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php index 214d65dde5..d004b54ccb 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php @@ -612,7 +612,6 @@ private function handleDiscardIndividualNodesFromWorkspace( // quick path everything was discarded yield from $this->discardWorkspace( $workspace, - $workspaceContentStreamVersion, $baseWorkspace, $baseWorkspaceContentStreamVersion, $command->newContentStreamId @@ -684,12 +683,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 @@ -701,7 +699,6 @@ private function handleDiscardWorkspace( */ private function discardWorkspace( Workspace $workspace, - Version $workspaceContentStreamVersion, Workspace $baseWorkspace, Version $baseWorkspaceContentStreamVersion, ContentStreamId $newContentStream From 73911e177b751c5f34116b348912d49de2f23c5e Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 1 Dec 2024 10:33:43 +0100 Subject: [PATCH 3/6] TASK: Introduce proper `PartialWorkspaceRebaseFailed` exception with information what exactly went wrong The Neos Ui should be able to catch this and tell the user to publish something else first or publish all. --- .../Feature/WorkspaceCommandHandler.php | 29 +++++--- .../PartialWorkspaceRebaseFailed.php | 74 +++++++++++++++++++ .../Exception/WorkspaceRebaseFailed.php | 2 +- 3 files changed, 92 insertions(+), 13 deletions(-) create mode 100644 Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/PartialWorkspaceRebaseFailed.php diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php index d004b54ccb..c82f6d7a6d 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php @@ -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; @@ -507,12 +508,14 @@ static function ($handle) use ($commandSimulator, $matchingCommands, $remainingC yield $this->reopenContentStreamWithoutConstraintChecks( $workspace->currentContentStreamId ); - if ($workspace->status === WorkspaceStatus::UP_TO_DATE) { - throw new \RuntimeException('TODO cannot publish changeset as the leftover changes would not be applicable.'); - } - // todo either its a conflict with not applicable changes because the one change belongs to another OR the base workspace contains changes and we need to rebase first. - // we assume the latter and let the user up date the workspace first! - 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 @@ -633,12 +636,14 @@ static function ($handle) use ($commandsToKeep): void { yield $this->reopenContentStreamWithoutConstraintChecks( $workspace->currentContentStreamId ); - if ($workspace->status === WorkspaceStatus::UP_TO_DATE) { - throw new \RuntimeException('TODO cannot discard changeset as the leftover changes would not be applicable.'); - } - // todo either its a conflict with not applicable changes because the one change belongs to another OR the base workspace contains changes and we need to rebase first. - // we assume the latter and let the user up date the workspace first! - 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( diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/PartialWorkspaceRebaseFailed.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/PartialWorkspaceRebaseFailed.php new file mode 100644 index 0000000000..c16946b59a --- /dev/null +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/PartialWorkspaceRebaseFailed.php @@ -0,0 +1,74 @@ +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); + } +} diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/WorkspaceRebaseFailed.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/WorkspaceRebaseFailed.php index 499ea7a247..45849b859d 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/WorkspaceRebaseFailed.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceRebase/Exception/WorkspaceRebaseFailed.php @@ -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, From 5f234c77aac4bd799898a9377d07c5d2d00e7661 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 1 Dec 2024 11:00:35 +0100 Subject: [PATCH 4/6] TASK: Introduce test for #5364 expecting a `PartialWorkspaceRebaseFailed` --- .../01-ConstraintChecks.feature | 35 ++++++++++++++++++- ...ricCommandExecutionAndEventPublication.php | 15 +++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W8-IndividualNodePublication/01-ConstraintChecks.feature b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W8-IndividualNodePublication/01-ConstraintChecks.feature index 60bf7aadc2..e21feac7c8 100644 --- a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W8-IndividualNodePublication/01-ConstraintChecks.feature +++ b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W8-IndividualNodePublication/01-ConstraintChecks.feature @@ -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" | @@ -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 | @@ -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 | diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php index 0afbf60c15..6a37dd2a9d 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php @@ -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; @@ -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.'); } @@ -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) { From 725a211611af715791e1bffa5a6c79c2c75469cf Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 1 Dec 2024 11:02:45 +0100 Subject: [PATCH 5/6] Hey Linter! ich komme gleich mit der rute!!! --- .../Classes/Feature/WorkspaceCommandHandler.php | 4 ++-- .../Bootstrap/GenericCommandExecutionAndEventPublication.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php index c82f6d7a6d..3f166babf2 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php @@ -508,7 +508,7 @@ static function ($handle) use ($commandSimulator, $matchingCommands, $remainingC yield $this->reopenContentStreamWithoutConstraintChecks( $workspace->currentContentStreamId ); - match($workspace->status) { + 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()), @@ -636,7 +636,7 @@ static function ($handle) use ($commandsToKeep): void { yield $this->reopenContentStreamWithoutConstraintChecks( $workspace->currentContentStreamId ); - match($workspace->status) { + 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()), diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php index 6a37dd2a9d..0f4b3d45fa 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php @@ -341,7 +341,7 @@ public function theLastCommandShouldHaveThrownTheWorkspaceRebaseFailedWith(strin $exception = $this->lastCommandException; Assert::assertNotNull($exception, 'Command did not throw exception'); - match($shortExceptionName) { + 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())), }; From d5a616f35bf20941d6cebe2cdb91b2bb8e44a9ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Sun, 1 Dec 2024 12:24:59 +0100 Subject: [PATCH 6/6] Change test to expect PartialWorkspaceRebaseFailed --- .../W10-IndividualNodeDiscarding/01-ConstraintChecks.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W10-IndividualNodeDiscarding/01-ConstraintChecks.feature b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W10-IndividualNodeDiscarding/01-ConstraintChecks.feature index bb15c251e2..0965e664d4 100644 --- a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W10-IndividualNodeDiscarding/01-ConstraintChecks.feature +++ b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W10-IndividualNodeDiscarding/01-ConstraintChecks.feature @@ -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 |