Skip to content

Commit

Permalink
Merge pull request #5274 from neos/bugfix/4508-mark-dependands-of-liv…
Browse files Browse the repository at this point in the history
…e-outdated-after-direct-change

BUGFIX 4508 mark dependands of live outdated after direct change
  • Loading branch information
mhsdesign authored Oct 24, 2024
2 parents 6597802 + b8a2bce commit a21f159
Show file tree
Hide file tree
Showing 13 changed files with 214 additions and 128 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Query\QueryBuilder;
use Neos\ContentGraph\DoctrineDbalAdapter\Domain\Repository\ContentGraph;
use Neos\ContentGraph\DoctrineDbalAdapter\Domain\Repository\NodeFactory;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentGraphReadModelInterface;
use Neos\ContentRepository\Core\NodeType\NodeTypeManager;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentGraphReadModelInterface;
use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId;
use Neos\ContentRepository\Core\SharedModel\Exception\WorkspaceDoesNotExist;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStream;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreams;
Expand All @@ -45,26 +47,44 @@ public function __construct(
) {
}

public function buildContentGraph(WorkspaceName $workspaceName, ContentStreamId $contentStreamId): ContentGraph
{
return new ContentGraph($this->dbal, $this->nodeFactory, $this->contentRepositoryId, $this->nodeTypeManager, $this->tableNames, $workspaceName, $contentStreamId);
}

public function findWorkspaceByName(WorkspaceName $workspaceName): ?Workspace
public function getContentGraph(WorkspaceName $workspaceName): ContentGraph
{
$workspaceByNameStatement = <<<SQL
$currentContentStreamIdStatement = <<<SQL
SELECT
name, baseWorkspaceName, currentContentStreamId, status
currentContentStreamId
FROM
{$this->tableNames->workspace()}
WHERE
name = :workspaceName
LIMIT 1
SQL;
try {
$row = $this->dbal->fetchAssociative($workspaceByNameStatement, [
$row = $this->dbal->fetchAssociative($currentContentStreamIdStatement, [
'workspaceName' => $workspaceName->value,
]);
} catch (Exception $e) {
throw new \RuntimeException(sprintf('Failed to load current content stream id from database: %s', $e->getMessage()), 1716903166, $e);
}
if ($row === false) {
throw WorkspaceDoesNotExist::butWasSupposedTo($workspaceName);
}
$currentContentStreamId = ContentStreamId::fromString($row['currentContentStreamId']);
return new ContentGraph($this->dbal, $this->nodeFactory, $this->contentRepositoryId, $this->nodeTypeManager, $this->tableNames, $workspaceName, $currentContentStreamId);
}

public function buildContentGraph(WorkspaceName $workspaceName, ContentStreamId $contentStreamId): ContentGraph
{
return new ContentGraph($this->dbal, $this->nodeFactory, $this->contentRepositoryId, $this->nodeTypeManager, $this->tableNames, $workspaceName, $contentStreamId);
}

public function findWorkspaceByName(WorkspaceName $workspaceName): ?Workspace
{
$workspaceQuery = $this->getBasicWorkspaceQuery()
->where('ws.name = :workspaceName')
->setMaxResults(1)
->setParameter('workspaceName', $workspaceName->value);
try {
$row = $workspaceQuery->fetchAssociative();
} catch (Exception $e) {
throw new \RuntimeException(sprintf('Failed to load workspace from database: %s', $e->getMessage()), 1716486077, $e);
}
Expand All @@ -76,14 +96,9 @@ public function findWorkspaceByName(WorkspaceName $workspaceName): ?Workspace

public function findWorkspaces(): Workspaces
{
$workspacesStatement = <<<SQL
SELECT
name, baseWorkspaceName, currentContentStreamId, status
FROM
{$this->tableNames->workspace()}
SQL;
$workspacesQuery = $this->getBasicWorkspaceQuery();
try {
$rows = $this->dbal->fetchAllAssociative($workspacesStatement);
$rows = $workspacesQuery->fetchAllAssociative();
} catch (Exception $e) {
throw new \RuntimeException(sprintf('Failed to load workspaces from database: %s', $e->getMessage()), 1716902981, $e);
}
Expand Down Expand Up @@ -145,21 +160,41 @@ public function countNodes(): int
}
}

private function getBasicWorkspaceQuery(): QueryBuilder
{
$queryBuilder = $this->dbal->createQueryBuilder();

return $queryBuilder
->select('ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion != scs.version as baseWorkspaceChanged')
->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');
}

/**
* @param array<string, mixed> $row
*/
private static function workspaceFromDatabaseRow(array $row): Workspace
{
$baseWorkspaceName = $row['baseWorkspaceName'] !== null ? WorkspaceName::fromString($row['baseWorkspaceName']) : null;
$status = match ($row['baseWorkspaceChanged']) {
// no base workspace, a root is always up-to-date
null => WorkspaceStatus::UP_TO_DATE,
// base workspace didnt change (sql 0 is _false_)
0 => WorkspaceStatus::UP_TO_DATE,
default => WorkspaceStatus::OUTDATED,
};

return new Workspace(
WorkspaceName::fromString($row['name']),
isset($row['baseWorkspaceName']) ? WorkspaceName::fromString($row['baseWorkspaceName']) : null,
$baseWorkspaceName,
ContentStreamId::fromString($row['currentContentStreamId']),
WorkspaceStatus::from($row['status']),
$status,
);
}

/**
* @param array<string, mixed> $row
* @param array<string, mixed> $row todo fetch source content stream version and use for publishing as expected version
*/
private static function contentStreamFromDatabaseRow(array $row): ContentStream
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ private function whenContentStreamWasForked(ContentStreamWasForked $event): void
// NOTE: as reference edges are attached to Relation Anchor Points (and they are lazily copy-on-written),
// we do not need to copy reference edges here (but we need to do it during copy on write).

$this->createContentStream($event->newContentStreamId, ContentStreamStatus::FORKED, $event->sourceContentStreamId);
$this->createContentStream($event->newContentStreamId, ContentStreamStatus::FORKED, $event->sourceContentStreamId, $event->versionOfSourceContentStream);
}

private function whenContentStreamWasRemoved(ContentStreamWasRemoved $event): void
Expand Down Expand Up @@ -714,7 +714,6 @@ private function whenWorkspaceBaseWorkspaceWasChanged(WorkspaceBaseWorkspaceWasC

private function whenWorkspaceRebaseFailed(WorkspaceRebaseFailed $event): void
{
$this->markWorkspaceAsOutdatedConflict($event->workspaceName);
$this->updateContentStreamStatus($event->candidateContentStreamId, ContentStreamStatus::REBASE_ERROR);
}

Expand All @@ -729,8 +728,6 @@ private function whenWorkspaceWasCreated(WorkspaceWasCreated $event): void
private function whenWorkspaceWasDiscarded(WorkspaceWasDiscarded $event): void
{
$this->updateWorkspaceContentStreamId($event->workspaceName, $event->newContentStreamId);
$this->markWorkspaceAsOutdated($event->workspaceName);
$this->markDependentWorkspacesAsOutdated($event->workspaceName);

// the new content stream is in use now
$this->updateContentStreamStatus($event->newContentStreamId, ContentStreamStatus::IN_USE_BY_WORKSPACE);
Expand All @@ -741,7 +738,6 @@ private function whenWorkspaceWasDiscarded(WorkspaceWasDiscarded $event): void
private function whenWorkspaceWasPartiallyDiscarded(WorkspaceWasPartiallyDiscarded $event): void
{
$this->updateWorkspaceContentStreamId($event->workspaceName, $event->newContentStreamId);
$this->markDependentWorkspacesAsOutdated($event->workspaceName);

// the new content stream is in use now
$this->updateContentStreamStatus($event->newContentStreamId, ContentStreamStatus::IN_USE_BY_WORKSPACE);
Expand All @@ -754,12 +750,6 @@ private function whenWorkspaceWasPartiallyPublished(WorkspaceWasPartiallyPublish
{
// TODO: How do we test this method? – It's hard to design a BDD testcase that fails if this method is commented out...
$this->updateWorkspaceContentStreamId($event->sourceWorkspaceName, $event->newSourceContentStreamId);
$this->markDependentWorkspacesAsOutdated($event->targetWorkspaceName);

// NASTY: we need to set the source workspace name as non-outdated; as it has been made up-to-date again.
$this->markWorkspaceAsUpToDate($event->sourceWorkspaceName);

$this->markDependentWorkspacesAsOutdated($event->sourceWorkspaceName);

// the new content stream is in use now
$this->updateContentStreamStatus($event->newSourceContentStreamId, ContentStreamStatus::IN_USE_BY_WORKSPACE);
Expand All @@ -772,12 +762,6 @@ private function whenWorkspaceWasPublished(WorkspaceWasPublished $event): void
{
// TODO: How do we test this method? – It's hard to design a BDD testcase that fails if this method is commented out...
$this->updateWorkspaceContentStreamId($event->sourceWorkspaceName, $event->newSourceContentStreamId);
$this->markDependentWorkspacesAsOutdated($event->targetWorkspaceName);

// NASTY: we need to set the source workspace name as non-outdated; as it has been made up-to-date again.
$this->markWorkspaceAsUpToDate($event->sourceWorkspaceName);

$this->markDependentWorkspacesAsOutdated($event->sourceWorkspaceName);

// the new content stream is in use now
$this->updateContentStreamStatus($event->newSourceContentStreamId, ContentStreamStatus::IN_USE_BY_WORKSPACE);
Expand All @@ -789,10 +773,6 @@ private function whenWorkspaceWasPublished(WorkspaceWasPublished $event): void
private function whenWorkspaceWasRebased(WorkspaceWasRebased $event): void
{
$this->updateWorkspaceContentStreamId($event->workspaceName, $event->newContentStreamId);
$this->markDependentWorkspacesAsOutdated($event->workspaceName);

// When the rebase is successful, we can set the status of the workspace back to UP_TO_DATE.
$this->markWorkspaceAsUpToDate($event->workspaceName);

// the new content stream is in use now
$this->updateContentStreamStatus($event->newContentStreamId, ContentStreamStatus::IN_USE_BY_WORKSPACE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ private function createWorkspaceTable(): Table
DbalSchemaFactory::columnForWorkspaceName('name')->setNotnull(true),
DbalSchemaFactory::columnForWorkspaceName('baseWorkspaceName')->setNotnull(false),
DbalSchemaFactory::columnForContentStreamId('currentContentStreamId')->setNotNull(true),
(new Column('status', self::type(Types::BINARY)))->setLength(20)->setNotnull(false),
]);

return $workspaceTable->setPrimaryKey(['name']);
Expand All @@ -127,9 +126,10 @@ private function createContentStreamTable(): Table
DbalSchemaFactory::columnForContentStreamId('id')->setNotnull(true),
(new Column('version', Type::getType(Types::INTEGER)))->setNotnull(true),
DbalSchemaFactory::columnForContentStreamId('sourceContentStreamId')->setNotnull(false),
(new Column('sourceContentStreamVersion', Type::getType(Types::INTEGER)))->setNotnull(false),
// Should become a DB ENUM (unclear how to configure with DBAL) or int (latter needs adaption to code)
(new Column('status', Type::getType(Types::BINARY)))->setLength(20)->setNotnull(true),
(new Column('removed', Type::getType(Types::BOOLEAN)))->setDefault(false)->setNotnull(false)
(new Column('removed', Type::getType(Types::BOOLEAN)))->setDefault(false)->setNotnull(false),
]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
*/
trait ContentStream
{
private function createContentStream(ContentStreamId $contentStreamId, ContentStreamStatus $status, ?ContentStreamId $sourceContentStreamId = null): void
private function createContentStream(ContentStreamId $contentStreamId, ContentStreamStatus $status, ?ContentStreamId $sourceContentStreamId = null, ?Version $sourceVersion = null): void
{
$this->dbal->insert($this->tableNames->contentStream(), [
'id' => $contentStreamId->value,
'sourceContentStreamId' => $sourceContentStreamId?->value,
'version' => 0,
'sourceContentStreamId' => $sourceContentStreamId?->value,
'sourceContentStreamVersion' => $sourceVersion?->value,
'status' => $status->value,
]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Neos\ContentGraph\DoctrineDbalAdapter\Domain\Projection\Feature;

use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceStatus;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;

Expand All @@ -20,8 +19,7 @@ private function createWorkspace(WorkspaceName $workspaceName, ?WorkspaceName $b
$this->dbal->insert($this->tableNames->workspace(), [
'name' => $workspaceName->value,
'baseWorkspaceName' => $baseWorkspaceName?->value,
'currentContentStreamId' => $contentStreamId->value,
'status' => WorkspaceStatus::UP_TO_DATE->value
'currentContentStreamId' => $contentStreamId->value
]);
}

Expand Down Expand Up @@ -55,58 +53,4 @@ private function updateWorkspaceContentStreamId(
'name' => $workspaceName->value
]);
}

private function markWorkspaceAsUpToDate(WorkspaceName $workspaceName): void
{
$this->dbal->executeStatement('
UPDATE ' . $this->tableNames->workspace() . '
SET status = :upToDate
WHERE
name = :workspaceName
', [
'upToDate' => WorkspaceStatus::UP_TO_DATE->value,
'workspaceName' => $workspaceName->value
]);
}

private function markDependentWorkspacesAsOutdated(WorkspaceName $baseWorkspaceName): void
{
$this->dbal->executeStatement('
UPDATE ' . $this->tableNames->workspace() . '
SET status = :outdated
WHERE
baseWorkspaceName = :baseWorkspaceName
', [
'outdated' => WorkspaceStatus::OUTDATED->value,
'baseWorkspaceName' => $baseWorkspaceName->value
]);
}

private function markWorkspaceAsOutdated(WorkspaceName $workspaceName): void
{
$this->dbal->executeStatement('
UPDATE ' . $this->tableNames->workspace() . '
SET
status = :outdated
WHERE
name = :workspaceName
', [
'outdated' => WorkspaceStatus::OUTDATED->value,
'workspaceName' => $workspaceName->value
]);
}

private function markWorkspaceAsOutdatedConflict(WorkspaceName $workspaceName): void
{
$this->dbal->executeStatement('
UPDATE ' . $this->tableNames->workspace() . '
SET
status = :outdatedConflict
WHERE
name = :workspaceName
', [
'outdatedConflict' => WorkspaceStatus::OUTDATED_CONFLICT->value,
'workspaceName' => $workspaceName->value
]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Neos\ContentRepository\Core\NodeType\NodeTypeManager;
use Neos\ContentRepository\Core\Projection\ContentGraph\ContentGraphInterface;
use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId;
use Neos\ContentRepository\Core\SharedModel\Exception\WorkspaceDoesNotExist;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStream;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreams;
Expand All @@ -32,6 +33,15 @@ public function __construct(
) {
}

public function getContentGraph(WorkspaceName $workspaceName): ContentGraphInterface
{
$contentStreamId = $this->findWorkspaceByName($workspaceName)?->currentContentStreamId;
if ($contentStreamId === null) {
throw WorkspaceDoesNotExist::butWasSupposedTo($workspaceName);
}
return new ContentHyperGraph($this->dbal, $this->nodeFactory, $this->contentRepositoryId, $this->nodeTypeManager, $this->tableNamePrefix, $workspaceName, $contentStreamId);
}

public function buildContentGraph(WorkspaceName $workspaceName, ContentStreamId $contentStreamId): ContentGraphInterface
{
return new ContentHyperGraph($this->dbal, $this->nodeFactory, $this->contentRepositoryId, $this->nodeTypeManager, $this->tableNamePrefix, $workspaceName, $contentStreamId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Feature: Workspace rebasing - conflicting changes
| baseWorkspaceName | "live" |
| newContentStreamId | "user-cs-identifier" |

Scenario: Conflicting changes lead to OUTDATED_CONFLICT which can be recovered from via forced rebase
Scenario: Conflicting changes lead to OUTDATED which can be recovered from via forced rebase

When the command CreateWorkspace is executed with payload:
| Key | Value |
Expand All @@ -56,6 +56,8 @@ Feature: Workspace rebasing - conflicting changes
| baseWorkspaceName | "live" |
| newContentStreamId | "user-cs-two" |

Then workspaces live,user-ws-one,user-ws-two have status UP_TO_DATE

When the command RemoveNodeAggregate is executed with payload:
| Key | Value |
| nodeAggregateId | "nody-mc-nodeface" |
Expand Down Expand Up @@ -85,10 +87,13 @@ Feature: Workspace rebasing - conflicting changes
| originDimensionSpacePoint | {} |
| propertyValues | {"text": "The other node"} |

Then workspaces live,user-ws-one,user-ws-two have status UP_TO_DATE

And the command PublishWorkspace is executed with payload:
| Key | Value |
| workspaceName | "user-ws-one" |

Then workspaces live,user-ws-one have status UP_TO_DATE
Then workspace user-ws-two has status OUTDATED

When the command RebaseWorkspace is executed with payload:
Expand All @@ -97,5 +102,5 @@ Feature: Workspace rebasing - conflicting changes
| rebasedContentStreamId | "user-cs-two-rebased" |
| rebaseErrorHandlingStrategy | "force" |

Then workspace user-ws-two has status UP_TO_DATE
Then workspaces live,user-ws-one,user-ws-two have status UP_TO_DATE
And I expect a node identified by user-cs-two-rebased;noderus-secundus;{} to exist in the content graph
Loading

0 comments on commit a21f159

Please sign in to comment.