Skip to content

Commit

Permalink
TASK: Ensure that the workspace owner is unique via sql index
Browse files Browse the repository at this point in the history
We still try to catch the error beforehand via php constraint (so that the `CreateWorkspace` is not handled), but in a race condition this will be thrown at last:

> Integrity constraint violation: 1062 Duplicate entry 'default-janedoe' for key 'owner''
  • Loading branch information
mhsdesign committed Dec 12, 2024
1 parent 29a9b6c commit ddccba1
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ public function addWorkspaceMetadata(ContentRepositoryId $contentRepositoryId, W
}
}

public function findPrimaryWorkspaceNameForUser(ContentRepositoryId $contentRepositoryId, UserId $userId): ?WorkspaceName
public function findWorkspaceNameByUser(ContentRepositoryId $contentRepositoryId, UserId $userId): ?WorkspaceName
{
$tableMetadata = self::TABLE_NAME_WORKSPACE_METADATA;
$query = <<<SQL
Expand All @@ -353,8 +353,6 @@ public function findPrimaryWorkspaceNameForUser(ContentRepositoryId $contentRepo
AND classification = :personalWorkspaceClassification
AND owner_user_id = :userId
SQL;
// We don't order the results and return the first matching workspace.
// In case multiple exist - which is not desired currently - and in the happy path not possible via the api (race conditions aside) - the order is not defined.
$workspaceName = $this->dbal->fetchOne($query, [
'contentRepositoryId' => $contentRepositoryId->value,
'personalWorkspaceClassification' => WorkspaceClassification::PERSONAL->value,
Expand Down
6 changes: 3 additions & 3 deletions Neos.Neos/Classes/Domain/Service/WorkspaceService.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function setWorkspaceDescription(ContentRepositoryId $contentRepositoryId
*/
public function getPersonalWorkspaceForUser(ContentRepositoryId $contentRepositoryId, UserId $userId): Workspace
{
$workspaceName = $this->metadataAndRoleRepository->findPrimaryWorkspaceNameForUser($contentRepositoryId, $userId);
$workspaceName = $this->metadataAndRoleRepository->findWorkspaceNameByUser($contentRepositoryId, $userId);
if ($workspaceName === null) {
throw new \RuntimeException(sprintf('No workspace is assigned to the user with id "%s")', $userId->value), 1718293801);
}
Expand Down Expand Up @@ -143,7 +143,7 @@ public function createRootWorkspace(ContentRepositoryId $contentRepositoryId, Wo
*/
public function createPersonalWorkspace(ContentRepositoryId $contentRepositoryId, WorkspaceName $workspaceName, WorkspaceTitle $title, WorkspaceDescription $description, WorkspaceName $baseWorkspaceName, UserId $ownerId): void
{
$existingUserWorkspace = $this->metadataAndRoleRepository->findPrimaryWorkspaceNameForUser($contentRepositoryId, $ownerId);
$existingUserWorkspace = $this->metadataAndRoleRepository->findWorkspaceNameByUser($contentRepositoryId, $ownerId);
if ($existingUserWorkspace !== null) {
throw new \RuntimeException(sprintf('Failed to create personal workspace "%s" for user with id "%s", because the workspace "%s" is already assigned to the user', $workspaceName->value, $ownerId->value, $existingUserWorkspace->value), 1733754904);
}
Expand Down Expand Up @@ -193,7 +193,7 @@ public function createSharedWorkspace(ContentRepositoryId $contentRepositoryId,
*/
public function createPersonalWorkspaceForUserIfMissing(ContentRepositoryId $contentRepositoryId, User $user): void
{
$existingWorkspaceName = $this->metadataAndRoleRepository->findPrimaryWorkspaceNameForUser($contentRepositoryId, $user->getId());
$existingWorkspaceName = $this->metadataAndRoleRepository->findWorkspaceNameByUser($contentRepositoryId, $user->getId());
if ($existingWorkspaceName !== null) {
$this->requireWorkspace($contentRepositoryId, $existingWorkspaceName);
return;
Expand Down
2 changes: 1 addition & 1 deletion Neos.Neos/Migrations/Mysql/Version20240425223900.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function up(Schema $schema): void
$tableWorkspaceMetadata->addColumn('classification', 'string', ['length' => 255]);
$tableWorkspaceMetadata->addColumn('owner_user_id', 'string', ['length' => 255, 'notnull' => false]);
$tableWorkspaceMetadata->setPrimaryKey(['content_repository_id', 'workspace_name']);
$tableWorkspaceMetadata->addIndex(['owner_user_id']);
$tableWorkspaceMetadata->addIndex(['owner_user_id'], 'IDX_D6197E562B18554A');

$tableWorkspaceRole = $schema->createTable('neos_neos_workspace_role');
$tableWorkspaceRole->addColumn('content_repository_id', 'string', ['length' => 16]);
Expand Down
40 changes: 40 additions & 0 deletions Neos.Neos/Migrations/Mysql/Version20241212000000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

namespace Neos\Flow\Persistence\Doctrine\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

final class Version20241212000000 extends AbstractMigration
{
public function getDescription(): string
{
return 'Restricts that the owner_user_id of the neos workspace metadata is unique';
}

public function up(Schema $schema): void
{
$this->abortIf(
!$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\MariaDBPlatform,
"Migration can only be executed safely on '\Doctrine\DBAL\Platforms\MariaDBPlatform'."
);

$tableWorkspaceMetadata = $schema->getTable('neos_neos_workspace_metadata');
$tableWorkspaceMetadata->addUniqueIndex(['content_repository_id', 'owner_user_id'], 'owner');
$tableWorkspaceMetadata->dropIndex('IDX_D6197E562B18554A');
}

public function down(Schema $schema): void
{
$this->abortIf(
!$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\MariaDBPlatform,
"Migration can only be executed safely on '\Doctrine\DBAL\Platforms\MariaDBPlatform'."
);

$tableWorkspaceMetadata = $schema->getTable('neos_neos_workspace_metadata');
$tableWorkspaceMetadata->addIndex(['owner_user_id'], 'IDX_D6197E562B18554A');
$tableWorkspaceMetadata->dropIndex('owner');
}
}
2 changes: 1 addition & 1 deletion Neos.Neos/Migrations/Postgresql/Version20240425223901.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function up(Schema $schema): void
$tableWorkspaceMetadata->addColumn('classification', 'string', ['length' => 255]);
$tableWorkspaceMetadata->addColumn('owner_user_id', 'string', ['length' => 255, 'notnull' => false]);
$tableWorkspaceMetadata->setPrimaryKey(['content_repository_id', 'workspace_name']);
$tableWorkspaceMetadata->addIndex(['owner_user_id']);
$tableWorkspaceMetadata->addIndex(['owner_user_id'], 'IDX_D6197E562B18554A');

$tableWorkspaceRole = $schema->createTable('neos_neos_workspace_role');
$tableWorkspaceRole->addColumn('content_repository_id', 'string', ['length' => 16]);
Expand Down
40 changes: 40 additions & 0 deletions Neos.Neos/Migrations/Postgresql/Version20241212000001.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

namespace Neos\Flow\Persistence\Doctrine\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

final class Version20241212000001 extends AbstractMigration
{
public function getDescription(): string
{
return 'Restricts that the owner_user_id of the neos workspace metadata is unique';
}

public function up(Schema $schema): void
{
$this->abortIf(
!$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\PostgreSQLPlatform,
"Migration can only be executed safely on '\Doctrine\DBAL\Platforms\PostgreSQLPlatform'."
);

$tableWorkspaceMetadata = $schema->getTable('neos_neos_workspace_metadata');
$tableWorkspaceMetadata->addUniqueIndex(['content_repository_id', 'owner_user_id'], 'owner');
$tableWorkspaceMetadata->dropIndex('IDX_D6197E562B18554A');
}

public function down(Schema $schema): void
{
$this->abortIf(
!$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\PostgreSQLPlatform,
"Migration can only be executed safely on '\Doctrine\DBAL\Platforms\PostgreSQLPlatform'."
);

$tableWorkspaceMetadata = $schema->getTable('neos_neos_workspace_metadata');
$tableWorkspaceMetadata->addIndex(['owner_user_id'], 'IDX_D6197E562B18554A');
$tableWorkspaceMetadata->dropIndex('owner');
}
}

0 comments on commit ddccba1

Please sign in to comment.