From 3e7035fe6c8c71bfac79c56c64358a0cc8f33207 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 31 Oct 2024 11:56:28 +0100 Subject: [PATCH 1/5] FEATURE: Introduce `publishableEvents` count on content stream --- .../src/ContentGraphReadModelAdapter.php | 5 +- .../DoctrineDbalContentGraphProjection.php | 2 +- .../DoctrineDbalContentGraphSchemaBuilder.php | 1 + .../Projection/Feature/ContentStream.php | 30 ++++++-- .../02-PublishWorkspace.feature | 5 +- .../PublishableToWorkspaceInterface.php | 3 +- .../Feature/WorkspaceCommandHandler.php | 74 ++++++------------- .../SharedModel/Workspace/Workspace.php | 20 ++++- .../src/NodeMigrationService.php | 11 +-- .../Command/WorkspaceCommandController.php | 17 ++--- .../Controller/WorkspaceController.php | 27 ++----- .../Private/Translations/en/Main.xlf | 3 - 12 files changed, 85 insertions(+), 113 deletions(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php index fc9c6f4ddc7..2aeab76b054 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php @@ -159,7 +159,7 @@ private function getBasicWorkspaceQuery(): QueryBuilder $queryBuilder = $this->dbal->createQueryBuilder(); return $queryBuilder - ->select('ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion = scs.version as upToDateWithBase') + ->select('ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion = scs.version as upToDateWithBase, cs.publishableEvents as publishableEventsOnStream') ->from($this->tableNames->workspace(), 'ws') ->join('ws', $this->tableNames->contentStream(), 'cs', 'cs.id = ws.currentcontentstreamid') ->leftJoin('cs', $this->tableNames->contentStream(), 'scs', 'scs.id = cs.sourceContentStreamId'); @@ -188,11 +188,12 @@ private static function workspaceFromDatabaseRow(array $row): Workspace $baseWorkspaceName, ContentStreamId::fromString($row['currentContentStreamId']), $status, + $baseWorkspaceName === null ? 0 : $row['publishableEventsOnStream'], ); } /** - * @param array $row todo fetch source content stream version and use for publishing as expected version + * @param array $row */ private static function contentStreamFromDatabaseRow(array $row): ContentStream { diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php index 477453656a9..6521cdddc86 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php @@ -237,7 +237,7 @@ public function apply(EventInterface $event, EventEnvelope $eventEnvelope): void default => $event instanceof EmbedsContentStreamId || throw new \InvalidArgumentException(sprintf('Unsupported event %s', get_debug_type($event))), }; if ($event instanceof EmbedsContentStreamId && ContentStreamEventStreamName::isContentStreamStreamName($eventEnvelope->streamName)) { - $this->updateContentStreamVersion($event->getContentStreamId(), $eventEnvelope->version); + $this->updateContentStreamVersion($event, $eventEnvelope->version); } } diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php index 378b481654f..6c26bb966e3 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php @@ -130,6 +130,7 @@ private function createContentStreamTable(): Table DbalSchemaFactory::columnForContentStreamId('sourceContentStreamId')->setNotnull(false), (new Column('sourceContentStreamVersion', Type::getType(Types::INTEGER)))->setNotnull(false), (new Column('closed', Type::getType(Types::BOOLEAN)))->setDefault(false)->setNotnull(true), + (new Column('publishableEvents', Type::getType(Types::INTEGER)))->setNotnull(true), ]); return $contentStreamTable->setPrimaryKey(['id']); diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php index 50cd71bb02e..1ac406ee1f1 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php @@ -4,6 +4,9 @@ namespace Neos\ContentGraph\DoctrineDbalAdapter\Domain\Projection\Feature; +use Neos\ContentRepository\Core\EventStore\EventInterface; +use Neos\ContentRepository\Core\Feature\Common\EmbedsContentStreamId; +use Neos\ContentRepository\Core\Feature\Common\PublishableToWorkspaceInterface; use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId; use Neos\EventStore\Model\Event\Version; @@ -20,7 +23,8 @@ private function createContentStream(ContentStreamId $contentStreamId, ?ContentS 'id' => $contentStreamId->value, 'version' => 0, 'sourceContentStreamId' => $sourceContentStreamId?->value, - 'sourceContentStreamVersion' => $sourceVersion?->value + 'sourceContentStreamVersion' => $sourceVersion?->value, + 'publishableEvents' => 0 ]); } @@ -49,12 +53,24 @@ private function removeContentStream(ContentStreamId $contentStreamId): void ]); } - private function updateContentStreamVersion(ContentStreamId $contentStreamId, Version $version): void + private function updateContentStreamVersion(EventInterface&EmbedsContentStreamId $event, Version $version): void { - $this->dbal->update($this->tableNames->contentStream(), [ - 'version' => $version->value, - ], [ - 'id' => $contentStreamId->value, - ]); + // todo make fork content stream `EmbedsContentStreamId` but then just ignore it here because we set the version already + $isPublishableEvent = $event instanceof PublishableToWorkspaceInterface; + if ($isPublishableEvent) { + $this->dbal->executeStatement( + "UPDATE {$this->tableNames->contentStream()} SET version=:version, publishableEvents=publishableEvents+1 WHERE id=:id", + [ + 'version' => $version->value, + 'id' => $event->getContentStreamId()->value, + ] + ); + } else { + $this->dbal->update($this->tableNames->contentStream(), [ + 'version' => $version->value, + ], [ + 'id' => $event->getContentStreamId()->value, + ]); + } } } diff --git a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W7-WorkspacePublication/02-PublishWorkspace.feature b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W7-WorkspacePublication/02-PublishWorkspace.feature index d7d79c62e87..43372bfcad3 100644 --- a/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W7-WorkspacePublication/02-PublishWorkspace.feature +++ b/Neos.ContentRepository.BehavioralTests/Tests/Behavior/Features/W7-WorkspacePublication/02-PublishWorkspace.feature @@ -177,10 +177,7 @@ Feature: Workspace based content publishing # the user and live workspace are unchanged Then I expect exactly 1 event to be published on stream "Workspace:user-test" - Then I expect exactly 3 event to be published on stream "ContentStream:user-cs-identifier" - And event at index 2 is of type "ContentStreamWasReopened" with payload: - | Key | Expected | - | contentStreamId | "user-cs-identifier" | + Then I expect exactly 1 event to be published on stream "ContentStream:user-cs-identifier" Then I expect node aggregate identifier "nody-mc-nodeface" to lead to node user-cs-identifier;nody-mc-nodeface;{} diff --git a/Neos.ContentRepository.Core/Classes/Feature/Common/PublishableToWorkspaceInterface.php b/Neos.ContentRepository.Core/Classes/Feature/Common/PublishableToWorkspaceInterface.php index 2660b94b623..0d319209ac6 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/Common/PublishableToWorkspaceInterface.php +++ b/Neos.ContentRepository.Core/Classes/Feature/Common/PublishableToWorkspaceInterface.php @@ -14,6 +14,7 @@ namespace Neos\ContentRepository\Core\Feature\Common; +use Neos\ContentRepository\Core\EventStore\EventInterface; use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; @@ -26,7 +27,7 @@ * * @internal used internally for the publishing mechanism of workspaces */ -interface PublishableToWorkspaceInterface +interface PublishableToWorkspaceInterface extends EventInterface { public function withWorkspaceNameAndContentStreamId(WorkspaceName $targetWorkspaceName, ContentStreamId $contentStreamId): self; } diff --git a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php index 2eb194f36f4..20143536272 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php +++ b/Neos.ContentRepository.Core/Classes/Feature/WorkspaceCommandHandler.php @@ -179,6 +179,11 @@ private function handlePublishWorkspace( ): \Generator { $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); + if (!$workspace->hasPublishableChanges()) { + // no-op + return; + } + if (!$commandHandlingDependencies->contentStreamExists($workspace->currentContentStreamId)) { throw new \RuntimeException('Cannot publish nodes on a workspace with a stateless content stream', 1729711258); } @@ -197,15 +202,6 @@ private function handlePublishWorkspace( ) ); - if ($rebaseableCommands->isEmpty()) { - // we have no changes, we just reopen; partial no-op - yield $this->reopenContentStream( - $workspace->currentContentStreamId, - $commandHandlingDependencies - ); - return; - } - try { yield from $this->publishWorkspace( $workspace, @@ -319,7 +315,6 @@ private function getCopiedEventsOfEventStream( $event = $this->eventNormalizer->denormalize($eventEnvelope->event); if ($event instanceof PublishableToWorkspaceInterface) { - /** @var EventInterface $copiedEvent */ $copiedEvent = $event->withWorkspaceNameAndContentStreamId($targetWorkspaceName, $targetContentStreamId); // We need to add the event metadata here for rebasing in nested workspace situations // (and for exporting) @@ -358,14 +353,7 @@ private function handleRebaseWorkspace( $commandHandlingDependencies ); - $rebaseableCommands = RebaseableCommands::extractFromEventStream( - $this->eventStore->load( - ContentStreamEventStreamName::fromContentStreamId($workspace->currentContentStreamId) - ->getEventStreamName() - ) - ); - - if ($rebaseableCommands->isEmpty()) { + if (!$workspace->hasPublishableChanges()) { // if we have no changes in the workspace we can fork from the base directly yield from $this->rebaseWorkspaceWithoutChanges( $workspace, @@ -376,6 +364,13 @@ private function handleRebaseWorkspace( return; } + $rebaseableCommands = RebaseableCommands::extractFromEventStream( + $this->eventStore->load( + ContentStreamEventStreamName::fromContentStreamId($workspace->currentContentStreamId) + ->getEventStreamName() + ) + ); + $commandSimulator = $this->commandSimulatorFactory->createSimulatorForWorkspace($baseWorkspace->workspaceName); $commandSimulator->run( @@ -438,16 +433,17 @@ private function handlePublishIndividualNodesFromWorkspace( PublishIndividualNodesFromWorkspace $command, CommandHandlingDependencies $commandHandlingDependencies, ): \Generator { - if ($command->nodesToPublish->isEmpty()) { + $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); + $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); + if ($command->nodesToPublish->isEmpty() || !$workspace->hasPublishableChanges()) { // noop return; } - $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); + // todo check that fetching workspace throws if there is no content stream id for it if (!$commandHandlingDependencies->contentStreamExists($workspace->currentContentStreamId)) { throw new \RuntimeException('Cannot publish nodes on a workspace with a stateless content stream', 1710410114); } - $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); $this->requireContentStreamToNotBeClosed($baseWorkspace->currentContentStreamId, $commandHandlingDependencies); $baseContentStreamVersion = $commandHandlingDependencies->getContentStreamVersion($baseWorkspace->currentContentStreamId); @@ -572,16 +568,17 @@ private function handleDiscardIndividualNodesFromWorkspace( DiscardIndividualNodesFromWorkspace $command, CommandHandlingDependencies $commandHandlingDependencies, ): \Generator { - if ($command->nodesToDiscard->isEmpty()) { + $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); + $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); + + if ($command->nodesToDiscard->isEmpty() || !$workspace->hasPublishableChanges()) { // noop return; } - $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); if (!$commandHandlingDependencies->contentStreamExists($workspace->currentContentStreamId)) { throw new \RuntimeException('Cannot discard nodes on a workspace with a stateless content stream', 1710408112); } - $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); yield $this->closeContentStream( $workspace->currentContentStreamId, @@ -673,7 +670,7 @@ private function handleDiscardWorkspace( $workspace = $this->requireWorkspace($command->workspaceName, $commandHandlingDependencies); $baseWorkspace = $this->requireBaseWorkspace($workspace, $commandHandlingDependencies); - if (!$this->hasEventsInContentStreamExceptForking(ContentStreamEventStreamName::fromContentStreamId($workspace->currentContentStreamId))) { + if (!$workspace->hasPublishableChanges()) { return; } @@ -885,33 +882,8 @@ private function requireNonCircularRelationBetweenWorkspaces(Workspace $workspac */ private function requireEmptyWorkspace(Workspace $workspace): void { - $workspaceContentStreamName = ContentStreamEventStreamName::fromContentStreamId( - $workspace->currentContentStreamId - ); - if ($this->hasEventsInContentStreamExceptForking($workspaceContentStreamName)) { + if ($workspace->hasPublishableChanges()) { throw new WorkspaceIsNotEmptyException('The user workspace needs to be empty before switching the base workspace.', 1681455989); } } - - /** - * @return bool - */ - private function hasEventsInContentStreamExceptForking( - ContentStreamEventStreamName $workspaceContentStreamName, - ): bool { - // todo introduce workspace has changes instead - $workspaceContentStream = $this->eventStore->load($workspaceContentStreamName->getEventStreamName()); - - $fullQualifiedEventClassName = ContentStreamWasForked::class; - $shortEventClassName = substr($fullQualifiedEventClassName, strrpos($fullQualifiedEventClassName, '\\') + 1); - - foreach ($workspaceContentStream as $eventEnvelope) { - if ($eventEnvelope->event->type->value === EventType::fromString($shortEventClassName)->value) { - continue; - } - return true; - } - - return false; - } } diff --git a/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php b/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php index b3fd69556fe..4e15eb038d4 100644 --- a/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php +++ b/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php @@ -32,7 +32,14 @@ private function __construct( public ?WorkspaceName $baseWorkspaceName, public ContentStreamId $currentContentStreamId, public WorkspaceStatus $status, + private int $countOfPublishableChanges ) { + if ($this->countOfPublishableChanges < 0) { + throw new \InvalidArgumentException('The number of changes must be greater than 0', 1730371545); + } + if ($this->isRootWorkspace() && $this->countOfPublishableChanges !== 0) { + throw new \InvalidArgumentException('Root workspaces cannot have changes', 1730371566); + } } /** @@ -43,8 +50,19 @@ public static function create( ?WorkspaceName $baseWorkspaceName, ContentStreamId $currentContentStreamId, WorkspaceStatus $status, + int $countOfPublishableChanges ): self { - return new self($workspaceName, $baseWorkspaceName, $currentContentStreamId, $status); + return new self($workspaceName, $baseWorkspaceName, $currentContentStreamId, $status, $countOfPublishableChanges); + } + + public function hasPublishableChanges(): bool + { + return $this->countOfPublishableChanges !== 0; + } + + public function countPublishableChanges(): int + { + return $this->countOfPublishableChanges; } /** diff --git a/Neos.ContentRepository.NodeMigration/src/NodeMigrationService.php b/Neos.ContentRepository.NodeMigration/src/NodeMigrationService.php index d60adda0a33..61401187d24 100644 --- a/Neos.ContentRepository.NodeMigration/src/NodeMigrationService.php +++ b/Neos.ContentRepository.NodeMigration/src/NodeMigrationService.php @@ -16,7 +16,6 @@ use Neos\ContentRepository\NodeMigration\Filter\FiltersFactory; use Neos\ContentRepository\NodeMigration\Filter\InvalidMigrationFilterSpecified; use Neos\ContentRepository\NodeMigration\Transformation\TransformationsFactory; -use Neos\Neos\PendingChangesProjection\ChangeFinder; /** * Node Migrations are manually written adjustments to the Node tree; @@ -68,7 +67,7 @@ public function executeMigration(ExecuteMigration $command): void $targetWorkspaceWasCreated = false; if ($targetWorkspace = $this->contentRepository->findWorkspaceByName($command->targetWorkspaceName)) { - if (!$this->workspaceIsEmpty($targetWorkspace)) { + if ($targetWorkspace->hasPublishableChanges()) { throw new MigrationException(sprintf('Target workspace "%s" already exists an is not empty. Please clear the workspace before.', $targetWorkspace->workspaceName->value)); } @@ -196,12 +195,4 @@ protected function executeSubMigration( } } } - - private function workspaceIsEmpty(Workspace $workspace): bool - { - // todo introduce Workspace::hasPendingChanges - return $this->contentRepository - ->projectionState(ChangeFinder::class) - ->countByContentStreamId($workspace->currentContentStreamId) === 0; - } } diff --git a/Neos.Neos/Classes/Command/WorkspaceCommandController.php b/Neos.Neos/Classes/Command/WorkspaceCommandController.php index 2fdd87931c2..2acd849db5c 100644 --- a/Neos.Neos/Classes/Command/WorkspaceCommandController.php +++ b/Neos.Neos/Classes/Command/WorkspaceCommandController.php @@ -396,20 +396,15 @@ public function deleteCommand(string $workspace, bool $force = false, string $co $this->quit(3); } - - try { - $nodesCount = $this->workspacePublishingService->countPendingWorkspaceChanges($contentRepositoryId, $workspaceName); - } catch (\Exception $exception) { - $this->outputLine('Could not fetch unpublished nodes for workspace %s, nothing was deleted. %s', [$workspaceName->value, $exception->getMessage()]); - $this->quit(4); - } - - if ($nodesCount > 0) { + if ($crWorkspace->hasPublishableChanges()) { if ($force === false) { $this->outputLine( - 'Did not delete workspace "%s" because it contains %s unpublished node(s).' + 'Did not delete workspace "%s" because it contains %d publishable changes.' . ' Use --force to delete it nevertheless.', - [$workspaceName->value, $nodesCount] + [ + $crWorkspace->countPublishableChanges(), + $workspaceName->value + ] ); $this->quit(5); } diff --git a/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php b/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php index 61ed35b2493..aea6f8d3f7c 100644 --- a/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php +++ b/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php @@ -373,30 +373,13 @@ public function deleteAction(WorkspaceName $workspaceName): void $this->redirect('index'); } - $nodesCount = 0; - - try { - $nodesCount = $contentRepository->projectionState(ChangeFinder::class) - ->countByContentStreamId( - $workspace->currentContentStreamId - ); - } catch (\Exception $exception) { - $message = $this->translator->translateById( - 'workspaces.notDeletedErrorWhileFetchingUnpublishedNodes', - [$workspaceMetadata->title->value], - null, - null, - 'Main', - 'Neos.Workspace.Ui' - ) ?: 'workspaces.notDeletedErrorWhileFetchingUnpublishedNodes'; - $this->addFlashMessage($message, '', Message::SEVERITY_WARNING); - $this->redirect('index'); - } - if ($nodesCount > 0) { + if ($workspace->hasPublishableChanges()) { + // todo indicate to the user that theses changes ARE NOT change projection changes + $changesCount = $workspace->countPublishableChanges(); $message = $this->translator->translateById( 'workspaces.workspaceCannotBeDeletedBecauseOfUnpublishedNodes', - [$workspaceMetadata->title->value, $nodesCount], - $nodesCount, + [$workspaceMetadata->title->value, $changesCount], + $changesCount, null, 'Main', 'Neos.Workspace.Ui' diff --git a/Neos.Workspace.Ui/Resources/Private/Translations/en/Main.xlf b/Neos.Workspace.Ui/Resources/Private/Translations/en/Main.xlf index da39fa6b61e..bd8b046baef 100644 --- a/Neos.Workspace.Ui/Resources/Private/Translations/en/Main.xlf +++ b/Neos.Workspace.Ui/Resources/Private/Translations/en/Main.xlf @@ -168,9 +168,6 @@ Workspace "{0}" cannot be deleted because the following workspaces are based on it: {1} - - An error occurred while fetching unpublished nodes from workspace "{0}", nothing was deleted. - Your personal workspace contains changes, please publish or discard them first. From b7f92370efe5eadccba96d3172822c4feefaee13 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 31 Oct 2024 13:57:50 +0100 Subject: [PATCH 2/5] TASK: Prefer simple dirty flag over counting publishable changes --- .../src/ContentGraphReadModelAdapter.php | 6 +++-- .../DoctrineDbalContentGraphSchemaBuilder.php | 4 ++-- .../Projection/Feature/ContentStream.php | 23 ++++++++----------- .../SharedModel/Workspace/Workspace.php | 21 +++++++---------- .../Command/WorkspaceCommandController.php | 14 ++++++----- .../Controller/WorkspaceController.php | 13 +++++++---- 6 files changed, 39 insertions(+), 42 deletions(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php index 2aeab76b054..a95df6236c9 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php @@ -159,7 +159,7 @@ private function getBasicWorkspaceQuery(): QueryBuilder $queryBuilder = $this->dbal->createQueryBuilder(); return $queryBuilder - ->select('ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion = scs.version as upToDateWithBase, cs.publishableEvents as publishableEventsOnStream') + ->select('ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion = scs.version as upToDateWithBase, cs.dirty as workspaceHasChanges') ->from($this->tableNames->workspace(), 'ws') ->join('ws', $this->tableNames->contentStream(), 'cs', 'cs.id = ws.currentcontentstreamid') ->leftJoin('cs', $this->tableNames->contentStream(), 'scs', 'scs.id = cs.sourceContentStreamId'); @@ -188,7 +188,9 @@ private static function workspaceFromDatabaseRow(array $row): Workspace $baseWorkspaceName, ContentStreamId::fromString($row['currentContentStreamId']), $status, - $baseWorkspaceName === null ? 0 : $row['publishableEventsOnStream'], + $baseWorkspaceName === null + ? false + : (bool)$row['workspaceHasChanges'], ); } diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php index 6c26bb966e3..2683b06b86c 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php @@ -129,8 +129,8 @@ private function createContentStreamTable(): Table (new Column('version', Type::getType(Types::INTEGER)))->setNotnull(true), DbalSchemaFactory::columnForContentStreamId('sourceContentStreamId')->setNotnull(false), (new Column('sourceContentStreamVersion', Type::getType(Types::INTEGER)))->setNotnull(false), - (new Column('closed', Type::getType(Types::BOOLEAN)))->setDefault(false)->setNotnull(true), - (new Column('publishableEvents', Type::getType(Types::INTEGER)))->setNotnull(true), + (new Column('closed', Type::getType(Types::BOOLEAN)))->setNotnull(true), + (new Column('dirty', Type::getType(Types::BOOLEAN)))->setNotnull(true), ]); return $contentStreamTable->setPrimaryKey(['id']); diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php index 1ac406ee1f1..cc5abaf69a8 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php @@ -24,7 +24,8 @@ private function createContentStream(ContentStreamId $contentStreamId, ?ContentS 'version' => 0, 'sourceContentStreamId' => $sourceContentStreamId?->value, 'sourceContentStreamVersion' => $sourceVersion?->value, - 'publishableEvents' => 0 + 'closed' => 0, + 'dirty' => 0 ]); } @@ -57,20 +58,14 @@ private function updateContentStreamVersion(EventInterface&EmbedsContentStreamId { // todo make fork content stream `EmbedsContentStreamId` but then just ignore it here because we set the version already $isPublishableEvent = $event instanceof PublishableToWorkspaceInterface; + $updatePayload = [ + 'version' => $version->value, + ]; if ($isPublishableEvent) { - $this->dbal->executeStatement( - "UPDATE {$this->tableNames->contentStream()} SET version=:version, publishableEvents=publishableEvents+1 WHERE id=:id", - [ - 'version' => $version->value, - 'id' => $event->getContentStreamId()->value, - ] - ); - } else { - $this->dbal->update($this->tableNames->contentStream(), [ - 'version' => $version->value, - ], [ - 'id' => $event->getContentStreamId()->value, - ]); + $updatePayload['dirty'] = 1; } + $this->dbal->update($this->tableNames->contentStream(), $updatePayload, [ + 'id' => $event->getContentStreamId()->value, + ]); } } diff --git a/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php b/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php index 4e15eb038d4..5a9a9985e2e 100644 --- a/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php +++ b/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/Workspace.php @@ -32,12 +32,9 @@ private function __construct( public ?WorkspaceName $baseWorkspaceName, public ContentStreamId $currentContentStreamId, public WorkspaceStatus $status, - private int $countOfPublishableChanges + private bool $hasPublishableChanges ) { - if ($this->countOfPublishableChanges < 0) { - throw new \InvalidArgumentException('The number of changes must be greater than 0', 1730371545); - } - if ($this->isRootWorkspace() && $this->countOfPublishableChanges !== 0) { + if ($this->isRootWorkspace() && $this->hasPublishableChanges) { throw new \InvalidArgumentException('Root workspaces cannot have changes', 1730371566); } } @@ -50,19 +47,17 @@ public static function create( ?WorkspaceName $baseWorkspaceName, ContentStreamId $currentContentStreamId, WorkspaceStatus $status, - int $countOfPublishableChanges + bool $hasPublishableChanges ): self { - return new self($workspaceName, $baseWorkspaceName, $currentContentStreamId, $status, $countOfPublishableChanges); + return new self($workspaceName, $baseWorkspaceName, $currentContentStreamId, $status, $hasPublishableChanges); } + /** + * Indicates if the workspace contains changed to be published + */ public function hasPublishableChanges(): bool { - return $this->countOfPublishableChanges !== 0; - } - - public function countPublishableChanges(): int - { - return $this->countOfPublishableChanges; + return $this->hasPublishableChanges; } /** diff --git a/Neos.Neos/Classes/Command/WorkspaceCommandController.php b/Neos.Neos/Classes/Command/WorkspaceCommandController.php index 2acd849db5c..5e58e39f5a5 100644 --- a/Neos.Neos/Classes/Command/WorkspaceCommandController.php +++ b/Neos.Neos/Classes/Command/WorkspaceCommandController.php @@ -398,13 +398,15 @@ public function deleteCommand(string $workspace, bool $force = false, string $co if ($crWorkspace->hasPublishableChanges()) { if ($force === false) { + try { + $nodesCount = $this->workspacePublishingService->countPendingWorkspaceChanges($contentRepositoryId, $workspaceName); + } catch (\Exception) { + $nodesCount = 'NAN'; + } $this->outputLine( - 'Did not delete workspace "%s" because it contains %d publishable changes.' - . ' Use --force to delete it nevertheless.', - [ - $crWorkspace->countPublishableChanges(), - $workspaceName->value - ] + 'Did not delete workspace "%s" because it contains %s unpublished node(s).' + . ' Use --force to delete it nevertheless.', + [$workspaceName->value, $nodesCount] ); $this->quit(5); } diff --git a/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php b/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php index aea6f8d3f7c..1ba6afe6e20 100644 --- a/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php +++ b/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php @@ -27,11 +27,11 @@ use Neos\ContentRepository\Core\Projection\ContentGraph\Node; use Neos\ContentRepository\Core\Projection\ContentGraph\Nodes; use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints; -use Neos\ContentRepository\Core\SharedModel\Workspace\Workspace; use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId; use Neos\ContentRepository\Core\SharedModel\Node\NodeAddress; use Neos\ContentRepository\Core\SharedModel\Node\NodeName; use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId; +use Neos\ContentRepository\Core\SharedModel\Workspace\Workspace; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Diff\Diff; @@ -374,12 +374,15 @@ public function deleteAction(WorkspaceName $workspaceName): void } if ($workspace->hasPublishableChanges()) { - // todo indicate to the user that theses changes ARE NOT change projection changes - $changesCount = $workspace->countPublishableChanges(); + try { + $nodesCount = $this->workspacePublishingService->countPendingWorkspaceChanges($contentRepositoryId, $workspaceName); + } catch (\Exception) { + $nodesCount = null; + } $message = $this->translator->translateById( 'workspaces.workspaceCannotBeDeletedBecauseOfUnpublishedNodes', - [$workspaceMetadata->title->value, $changesCount], - $changesCount, + [$workspaceMetadata->title->value, $nodesCount ?? 'NAN'], + $nodesCount, null, 'Main', 'Neos.Workspace.Ui' From c86bd584fdeb6ebf3570c8b33ba5d83b6911c098 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 31 Oct 2024 17:52:05 +0100 Subject: [PATCH 3/5] TASK: Do not try catch `countPendingWorkspaceChanges` ... because it would lead to hard to debug code and is not expected to fail --- Neos.Neos/Classes/Command/WorkspaceCommandController.php | 8 ++------ .../Classes/Controller/WorkspaceController.php | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/Neos.Neos/Classes/Command/WorkspaceCommandController.php b/Neos.Neos/Classes/Command/WorkspaceCommandController.php index 5e58e39f5a5..ddd62e7a8b3 100644 --- a/Neos.Neos/Classes/Command/WorkspaceCommandController.php +++ b/Neos.Neos/Classes/Command/WorkspaceCommandController.php @@ -398,17 +398,13 @@ public function deleteCommand(string $workspace, bool $force = false, string $co if ($crWorkspace->hasPublishableChanges()) { if ($force === false) { - try { - $nodesCount = $this->workspacePublishingService->countPendingWorkspaceChanges($contentRepositoryId, $workspaceName); - } catch (\Exception) { - $nodesCount = 'NAN'; - } + $nodesCount = $this->workspacePublishingService->countPendingWorkspaceChanges($contentRepositoryId, $workspaceName); $this->outputLine( 'Did not delete workspace "%s" because it contains %s unpublished node(s).' . ' Use --force to delete it nevertheless.', [$workspaceName->value, $nodesCount] ); - $this->quit(5); + $this->quit(4); } $this->workspacePublishingService->discardAllWorkspaceChanges($contentRepositoryId, $workspaceName); } diff --git a/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php b/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php index 1ba6afe6e20..dc1c0aa5bb0 100644 --- a/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php +++ b/Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php @@ -374,14 +374,10 @@ public function deleteAction(WorkspaceName $workspaceName): void } if ($workspace->hasPublishableChanges()) { - try { - $nodesCount = $this->workspacePublishingService->countPendingWorkspaceChanges($contentRepositoryId, $workspaceName); - } catch (\Exception) { - $nodesCount = null; - } + $nodesCount = $this->workspacePublishingService->countPendingWorkspaceChanges($contentRepositoryId, $workspaceName); $message = $this->translator->translateById( 'workspaces.workspaceCannotBeDeletedBecauseOfUnpublishedNodes', - [$workspaceMetadata->title->value, $nodesCount ?? 'NAN'], + [$workspaceMetadata->title->value, $nodesCount], $nodesCount, null, 'Main', From 489db4c0be2962151def68a407a6db77afea090f Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 31 Oct 2024 18:02:33 +0100 Subject: [PATCH 4/5] TASK: Make `ContentStreamWasForked` implement `EmbedsContentStreamId` ... and dont `updateContentStreamVersion` for it https://neos-project.slack.com/archives/C04PYL8H3/p1730292774760689 --- .../src/DoctrineDbalContentGraphProjection.php | 13 +++++++++++-- .../src/Domain/Projection/Feature/ContentStream.php | 11 +++-------- .../Event/ContentStreamWasForked.php | 8 +++++++- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php index 6521cdddc86..b9d62d38da2 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php @@ -24,6 +24,7 @@ use Neos\ContentRepository\Core\EventStore\InitiatingEventMetadata; use Neos\ContentRepository\Core\Feature\Common\EmbedsContentStreamId; use Neos\ContentRepository\Core\Feature\Common\InterdimensionalSiblings; +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\ContentStreamCreation\Event\ContentStreamWasCreated; @@ -236,8 +237,16 @@ public function apply(EventInterface $event, EventEnvelope $eventEnvelope): void WorkspaceWasRemoved::class => $this->whenWorkspaceWasRemoved($event), default => $event instanceof EmbedsContentStreamId || throw new \InvalidArgumentException(sprintf('Unsupported event %s', get_debug_type($event))), }; - if ($event instanceof EmbedsContentStreamId && ContentStreamEventStreamName::isContentStreamStreamName($eventEnvelope->streamName)) { - $this->updateContentStreamVersion($event, $eventEnvelope->version); + if ( + $event instanceof EmbedsContentStreamId + && ContentStreamEventStreamName::isContentStreamStreamName($eventEnvelope->streamName) + && !( + // special case as we dont need to update anything. The handling above takes care of setting the version to 0 + $event instanceof ContentStreamWasForked + || $event instanceof ContentStreamWasCreated + ) + ) { + $this->updateContentStreamVersion($event->getContentStreamId(), $eventEnvelope->version, $event instanceof PublishableToWorkspaceInterface); } } diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php index cc5abaf69a8..cbddc75ce2d 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php @@ -4,9 +4,6 @@ namespace Neos\ContentGraph\DoctrineDbalAdapter\Domain\Projection\Feature; -use Neos\ContentRepository\Core\EventStore\EventInterface; -use Neos\ContentRepository\Core\Feature\Common\EmbedsContentStreamId; -use Neos\ContentRepository\Core\Feature\Common\PublishableToWorkspaceInterface; use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId; use Neos\EventStore\Model\Event\Version; @@ -54,18 +51,16 @@ private function removeContentStream(ContentStreamId $contentStreamId): void ]); } - private function updateContentStreamVersion(EventInterface&EmbedsContentStreamId $event, Version $version): void + private function updateContentStreamVersion(ContentStreamId $contentStreamId, Version $version, bool $markAsDirty): void { - // todo make fork content stream `EmbedsContentStreamId` but then just ignore it here because we set the version already - $isPublishableEvent = $event instanceof PublishableToWorkspaceInterface; $updatePayload = [ 'version' => $version->value, ]; - if ($isPublishableEvent) { + if ($markAsDirty) { $updatePayload['dirty'] = 1; } $this->dbal->update($this->tableNames->contentStream(), $updatePayload, [ - 'id' => $event->getContentStreamId()->value, + 'id' => $contentStreamId->value, ]); } } diff --git a/Neos.ContentRepository.Core/Classes/Feature/ContentStreamForking/Event/ContentStreamWasForked.php b/Neos.ContentRepository.Core/Classes/Feature/ContentStreamForking/Event/ContentStreamWasForked.php index 9c476787fc9..a2418445221 100644 --- a/Neos.ContentRepository.Core/Classes/Feature/ContentStreamForking/Event/ContentStreamWasForked.php +++ b/Neos.ContentRepository.Core/Classes/Feature/ContentStreamForking/Event/ContentStreamWasForked.php @@ -15,13 +15,14 @@ */ use Neos\ContentRepository\Core\EventStore\EventInterface; +use Neos\ContentRepository\Core\Feature\Common\EmbedsContentStreamId; use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId; use Neos\EventStore\Model\Event\Version; /** * @api events are the persistence-API of the content repository */ -final readonly class ContentStreamWasForked implements EventInterface +final readonly class ContentStreamWasForked implements EventInterface, EmbedsContentStreamId { public function __construct( /** @@ -33,6 +34,11 @@ public function __construct( ) { } + public function getContentStreamId(): ContentStreamId + { + return $this->newContentStreamId; + } + public static function fromArray(array $values): self { return new self( From 2e5eb478193f681d01058461dc2a2af8366fb5be Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 1 Nov 2024 11:36:19 +0100 Subject: [PATCH 5/5] TASK: rename `dirty` to `hasChanges` --- .../src/ContentGraphReadModelAdapter.php | 4 ++-- .../src/DoctrineDbalContentGraphSchemaBuilder.php | 2 +- .../src/Domain/Projection/Feature/ContentStream.php | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php index a95df6236c9..7e5e2051a5f 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php @@ -159,7 +159,7 @@ private function getBasicWorkspaceQuery(): QueryBuilder $queryBuilder = $this->dbal->createQueryBuilder(); return $queryBuilder - ->select('ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion = scs.version as upToDateWithBase, cs.dirty as workspaceHasChanges') + ->select('ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.hasChanges, cs.sourceContentStreamVersion = scs.version as upToDateWithBase') ->from($this->tableNames->workspace(), 'ws') ->join('ws', $this->tableNames->contentStream(), 'cs', 'cs.id = ws.currentcontentstreamid') ->leftJoin('cs', $this->tableNames->contentStream(), 'scs', 'scs.id = cs.sourceContentStreamId'); @@ -190,7 +190,7 @@ private static function workspaceFromDatabaseRow(array $row): Workspace $status, $baseWorkspaceName === null ? false - : (bool)$row['workspaceHasChanges'], + : (bool)$row['hasChanges'], ); } diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php index 2683b06b86c..d19bb944f78 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php @@ -130,7 +130,7 @@ private function createContentStreamTable(): Table DbalSchemaFactory::columnForContentStreamId('sourceContentStreamId')->setNotnull(false), (new Column('sourceContentStreamVersion', Type::getType(Types::INTEGER)))->setNotnull(false), (new Column('closed', Type::getType(Types::BOOLEAN)))->setNotnull(true), - (new Column('dirty', Type::getType(Types::BOOLEAN)))->setNotnull(true), + (new Column('hasChanges', Type::getType(Types::BOOLEAN)))->setNotnull(true), ]); return $contentStreamTable->setPrimaryKey(['id']); diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php index cbddc75ce2d..08e2e151824 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/ContentStream.php @@ -22,7 +22,7 @@ private function createContentStream(ContentStreamId $contentStreamId, ?ContentS 'sourceContentStreamId' => $sourceContentStreamId?->value, 'sourceContentStreamVersion' => $sourceVersion?->value, 'closed' => 0, - 'dirty' => 0 + 'hasChanges' => 0 ]); } @@ -57,7 +57,7 @@ private function updateContentStreamVersion(ContentStreamId $contentStreamId, Ve 'version' => $version->value, ]; if ($markAsDirty) { - $updatePayload['dirty'] = 1; + $updatePayload['hasChanges'] = 1; } $this->dbal->update($this->tableNames->contentStream(), $updatePayload, [ 'id' => $contentStreamId->value,