Skip to content

Commit

Permalink
TASK: Prefer simple dirty flag over counting publishable changes
Browse files Browse the repository at this point in the history
  • Loading branch information
mhsdesign committed Oct 31, 2024
1 parent 7597fe0 commit 03f41b1
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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'],
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ private function createContentStream(ContentStreamId $contentStreamId, ?ContentS
'version' => 0,
'sourceContentStreamId' => $sourceContentStreamId?->value,
'sourceContentStreamVersion' => $sourceVersion?->value,
'publishableEvents' => 0
'closed' => 0,
'dirty' => 0
]);
}

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

/**
Expand Down
14 changes: 8 additions & 6 deletions Neos.Neos/Classes/Command/WorkspaceCommandController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
13 changes: 8 additions & 5 deletions Neos.Workspace.Ui/Classes/Controller/WorkspaceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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'
Expand Down

0 comments on commit 03f41b1

Please sign in to comment.