From 1efa29839a18e24de64f5fd01e3cf9a2fe05a0d0 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:10:42 +0100 Subject: [PATCH 1/3] !!! TASK: Throw exceptions in unnecessary rebase and publish cases --- .../Feature/WorkspaceCommandHandler.php | 58 ++++--------------- 1 file changed, 12 insertions(+), 46 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php index 20143536272..84209dd2df1 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php @@ -20,14 +20,12 @@ use Neos\ContentRepository\Core\CommandHandler\CommandSimulatorFactory; use Neos\ContentRepository\Core\ContentRepository; use Neos\ContentRepository\Core\EventStore\DecoratedEvent; -use Neos\ContentRepository\Core\EventStore\EventInterface; use Neos\ContentRepository\Core\EventStore\EventNormalizer; use Neos\ContentRepository\Core\EventStore\Events; use Neos\ContentRepository\Core\EventStore\EventsToPublish; use Neos\ContentRepository\Core\Feature\Common\PublishableToWorkspaceInterface; use Neos\ContentRepository\Core\Feature\ContentStreamClosing\Event\ContentStreamWasClosed; use Neos\ContentRepository\Core\Feature\ContentStreamClosing\Event\ContentStreamWasReopened; -use Neos\ContentRepository\Core\Feature\ContentStreamForking\Event\ContentStreamWasForked; use Neos\ContentRepository\Core\Feature\WorkspaceCreation\Command\CreateRootWorkspace; use Neos\ContentRepository\Core\Feature\WorkspaceCreation\Command\CreateWorkspace; use Neos\ContentRepository\Core\Feature\WorkspaceCreation\Event\RootWorkspaceWasCreated; @@ -49,6 +47,7 @@ use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasPartiallyDiscarded; use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasPartiallyPublished; use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasPublished; +use Neos\ContentRepository\Core\Feature\WorkspacePublication\Exception\NoChangesException; use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Command\RebaseWorkspace; use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Dto\RebaseErrorHandlingStrategy; use Neos\ContentRepository\Core\Feature\WorkspaceRebase\Event\WorkspaceWasRebased; @@ -62,7 +61,6 @@ use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceStatus; use Neos\EventStore\EventStoreInterface; -use Neos\EventStore\Model\Event\EventType; use Neos\EventStore\Model\Event\SequenceNumber; use Neos\EventStore\Model\Event\Version; use Neos\EventStore\Model\EventStream\EventStreamInterface; @@ -180,8 +178,7 @@ private function handlePublishWorkspace( $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); if (!$workspace->hasPublishableChanges()) { - // no-op - return; + throw NoChangesException::inWorkspaceToPublish($command->workspaceName); } if (!$commandHandlingDependencies->contentStreamExists($workspace->currentContentStreamId)) { @@ -275,33 +272,6 @@ static function ($handle) use ($rebaseableCommands): void { yield $this->removeContentStream($workspace->currentContentStreamId, $commandHandlingDependencies); } - private function rebaseWorkspaceWithoutChanges( - Workspace $workspace, - Workspace $baseWorkspace, - ContentStreamId $newContentStreamId, - CommandHandlingDependencies $commandHandlingDependencies, - ): \Generator { - yield $this->forkContentStream( - $newContentStreamId, - $baseWorkspace->currentContentStreamId, - $commandHandlingDependencies - ); - - yield new EventsToPublish( - WorkspaceEventStreamName::fromWorkspaceName($workspace->workspaceName)->getEventStreamName(), - Events::with( - new WorkspaceWasRebased( - $workspace->workspaceName, - $newContentStreamId, - $workspace->currentContentStreamId, - ), - ), - ExpectedVersion::ANY() - ); - - yield $this->removeContentStream($workspace->currentContentStreamId, $commandHandlingDependencies); - } - /** * Copy all events from the passed event stream which implement the {@see PublishableToOtherContentStreamsInterface} */ @@ -345,25 +315,19 @@ private function handleRebaseWorkspace( && $command->rebaseErrorHandlingStrategy !== RebaseErrorHandlingStrategy::STRATEGY_FORCE ) { // no-op if workspace is not outdated and not forcing it + // TODO throw here too? return; } + if (!$workspace->hasPublishableChanges()) { + NoChangesException::noChangesToRebase($command->workspaceName); + } + yield $this->closeContentStream( $workspace->currentContentStreamId, $commandHandlingDependencies ); - if (!$workspace->hasPublishableChanges()) { - // if we have no changes in the workspace we can fork from the base directly - yield from $this->rebaseWorkspaceWithoutChanges( - $workspace, - $baseWorkspace, - $command->rebasedContentStreamId, - $commandHandlingDependencies - ); - return; - } - $rebaseableCommands = RebaseableCommands::extractFromEventStream( $this->eventStore->load( ContentStreamEventStreamName::fromContentStreamId($workspace->currentContentStreamId) @@ -435,9 +399,11 @@ private function handlePublishIndividualNodesFromWorkspace( ): \Generator { $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); - if ($command->nodesToPublish->isEmpty() || !$workspace->hasPublishableChanges()) { - // noop - return; + if ($command->nodesToPublish->isEmpty()) { + throw NoChangesException::nothingSelectedForPublish($command->workspaceName); + } + if (!$workspace->hasPublishableChanges()) { + throw NoChangesException::inWorkspaceToPublish($command->workspaceName); } // todo check that fetching workspace throws if there is no content stream id for it From deb0cf94e89cf43a7ee599d11bd2def2650ec292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Mu=CC=88ller?= Date: Fri, 1 Nov 2024 13:57:22 +0100 Subject: [PATCH 2/3] WIP: Add exception on no-op publishes/rebases --- .../Exception/NoChangesException.php | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php new file mode 100644 index 00000000000..a2f68361924 --- /dev/null +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php @@ -0,0 +1,38 @@ +value), 1730463156); + } + + public static function nothingSelectedForPublish(WorkspaceName $workspaceName): self + { + return new self(sprintf('Cannot publish workspace "%s" because no changes were selected.', $workspaceName->value), 1730463510); + } + + public static function noChangesToRebase(WorkspaceName $workspaceName): self + { + return new self(sprintf('Cannot rebase workspace "%s" because it has no changes.', $workspaceName->value), 1730463693); + } +} From 8117aebf63bb2aceb3003b77f7d77a42f94d70d8 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 3 Nov 2024 19:27:40 +0100 Subject: [PATCH 3/3] TASK: Adjust documentation --- .../WorkspacePublication/Exception/NoChangesException.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php index a2f68361924..7293a8e2830 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspacePublication/Exception/NoChangesException.php @@ -17,9 +17,9 @@ use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; /** - * @api thrown as part of command handling in case + * @api thrown as part of command handling in case of empty publish and rebase operations */ -class NoChangesException extends \Exception +class NoChangesException extends \RuntimeException { public static function inWorkspaceToPublish(WorkspaceName $workspaceName): self {