Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX 4508 mark dependands of live outdated after direct change #5274

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
use Neos\ContentRepository\Core\SharedModel\Workspace\Workspaces;
use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceStatus;
use Neos\EventStore\Model\Event\Version;
use Neos\EventStore\Model\EventStream\ExpectedVersion;
use Neos\EventStore\Model\EventStream\MaybeVersion;

/**
* @internal only used inside the
Expand All @@ -55,11 +57,13 @@ public function findWorkspaceByName(WorkspaceName $workspaceName): ?Workspace
{
$workspaceByNameStatement = <<<SQL
SELECT
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved
name, baseWorkspaceName, currentContentStreamId, status
ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion as expectedSourceVersion, scs.version as sourceVersion
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved
FROM
{$this->tableNames->workspace()}
{$this->tableNames->workspace()} ws
JOIN {$this->tableNames->contentStream()} cs ON cs.id = ws.currentcontentstreamid
LEFT JOIN {$this->tableNames->contentStream()} scs ON scs.id = cs.sourceContentStreamId
WHERE
name = :workspaceName
ws.name = :workspaceName
LIMIT 1
SQL;
try {
Expand All @@ -79,9 +83,11 @@ public function findWorkspaces(): Workspaces
{
$workspacesStatement = <<<SQL
SELECT
name, baseWorkspaceName, currentContentStreamId, status
ws.name, ws.baseWorkspaceName, ws.currentContentStreamId, cs.sourceContentStreamVersion as expectedSourceVersion, scs.version as sourceVersion
FROM
{$this->tableNames->workspace()}
{$this->tableNames->workspace()} ws
JOIN {$this->tableNames->contentStream()} cs ON cs.id = ws.currentcontentstreamid
LEFT JOIN {$this->tableNames->contentStream()} scs ON scs.id = cs.sourceContentStreamId
SQL;
try {
$rows = $this->dbal->fetchAllAssociative($workspacesStatement);
Expand Down Expand Up @@ -136,11 +142,23 @@ public function findContentStreams(): ContentStreams
*/
private static function workspaceFromDatabaseRow(array $row): Workspace
{
$baseWorkspaceName = $row['baseWorkspaceName'] !== null ? WorkspaceName::fromString($row['baseWorkspaceName']) : null;

$status = WorkspaceStatus::UP_TO_DATE;
if ($baseWorkspaceName !== null) {
// root workspaces can never be out of date and dont have a source content stream
$sourceVersion = MaybeVersion::fromVersionOrNull(Version::fromInteger($row['sourceVersion']));
$expectedSourceVersion = ExpectedVersion::fromVersion(Version::fromInteger($row['expectedSourceVersion']));

if (!$expectedSourceVersion->isSatisfiedBy($sourceVersion)) {
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved
$status = WorkspaceStatus::OUTDATED;
}
mhsdesign marked this conversation as resolved.
Show resolved Hide resolved
}
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,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,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 @@ -733,7 +733,6 @@ private function whenWorkspaceBaseWorkspaceWasChanged(WorkspaceBaseWorkspaceWasC

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

Expand All @@ -748,8 +747,6 @@ private function whenWorkspaceWasCreated(WorkspaceWasCreated $event): void
private function whenWorkspaceWasDiscarded(WorkspaceWasDiscarded $event): void
{
$this->updateWorkspaceContentStreamId($event->workspaceName, $event->newContentStreamId);
$this->markWorkspaceAsOutdated($event->workspaceName);
nezaniel marked this conversation as resolved.
Show resolved Hide resolved
$this->markDependentWorkspacesAsOutdated($event->workspaceName);

// the new content stream is in use now
$this->updateContentStreamStatus($event->newContentStreamId, ContentStreamStatus::IN_USE_BY_WORKSPACE);
Expand All @@ -760,7 +757,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 @@ -773,12 +769,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 @@ -791,12 +781,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 @@ -808,10 +792,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 @@ -45,7 +45,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 @@ -58,6 +58,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 @@ -87,10 +89,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 @@ -99,5 +104,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
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ Feature: Workspace discarding - basic functionality
| Key | Value |
| text | "Modified in live workspace" |

Scenario: Conflicting changes lead to OUTDATED which can be recovered from via discard
Scenario: Conflicting changes lead to exception on rebase which can be recovered from via discard

When the command CreateWorkspace is executed with payload:
| Key | Value |
Expand All @@ -111,6 +111,8 @@ Feature: Workspace discarding - basic functionality
| 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 |
| workspaceName | "user-ws-one" |
Expand All @@ -129,12 +131,14 @@ Feature: Workspace discarding - basic functionality
| 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 and exceptions are caught:
| Key | Value |
| workspaceName | "user-ws-two" |
| rebasedContentStreamId | "user-cs-two-rebased" |
Then the last command should have thrown an exception of type "WorkspaceRebaseFailed"

Then workspace user-ws-two has status OUTDATED

Expand All @@ -143,5 +147,4 @@ Feature: Workspace discarding - basic functionality
| workspaceName | "user-ws-two" |
| newContentStreamId | "user-cs-two-discarded" |

Then workspace user-ws-two has status OUTDATED

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