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.

Sign up for GitHub

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: Fix workspace role assignment after migration, import and site creation #5283

Merged
merged 2 commits into from
Oct 11, 2024
+108 −159
Merged

BUGFIX: Fix workspace role assignment after migration, import and site creation #5283

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e3c72f
BUGFIX: Fix workspace role assignment after migration, import and sit…
bwaidelich Oct 11, 2024
f7f5242
Fix DI for `WorkspaceService` in `SiteService`
bwaidelich Oct 11, 2024
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
86 changes: 0 additions & 86 deletions Neos.ContentRepository.Core/Classes/Service/ContentRepositoryBootstrapper.php
View file
Open in desktop

This file was deleted.

100 changes: 39 additions & 61 deletions Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Neos\ContentRepositoryRegistry\Service;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
use Neos\ContentRepository\Core\Factory\ContentRepositoryServiceInterface;
use Neos\ContentRepository\Core\Feature\Common\RebasableToOtherWorkspaceInterface;
use Neos\ContentRepository\Core\Feature\NodeCreation\Command\CreateNodeAggregateWithNodeAndSerializedProperties;
Expand Down Expand Up @@ -498,31 +499,14 @@ public function migrateWorkspaceMetadataToWorkspaceService(\Closure $outputFn):

switch ($eventType) {
case 'RootWorkspaceWasCreated':
$eventData = self::decodeEventPayload($eventEnvelope);
if (!isset($eventData['workspaceTitle'])) {
// without the field it's not a legacy workspace creation event
continue 2;
}
$workspaces[$eventData['workspaceName']] = [
'workspaceName' => $eventData['workspaceName'],
'workspaceTitle' => $eventData['workspaceTitle'],
'workspaceDescription' => $eventData['workspaceDescription'],
'baseWorkspaceName' => null,
'workspaceOwner' => null
];
break;
case 'WorkspaceWasCreated':
$eventData = self::decodeEventPayload($eventEnvelope);
if (!isset($eventData['workspaceTitle'])) {
// without the field it's not a legacy workspace creation event
continue 2;
}
Comment on lines -516 to -519
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this statement

'Found %d legacy workspace events resulting in %d workspaces.

is wrong. .. also this is okay not to be there? We have runtime migration fallback? Why did we remove this check?

$workspaces[$eventData['workspaceName']] = [
'workspaceName' => $eventData['workspaceName'],
'baseWorkspaceName' => $eventData['baseWorkspaceName'],
'workspaceTitle' => $eventData['workspaceTitle'],
'workspaceDescription' => $eventData['workspaceDescription'],
'workspaceOwner' => $eventData['workspaceOwner'] ?? null
'workspaceTitle' => !empty($eventData['workspaceTitle']) ? $eventData['workspaceTitle'] : $eventData['workspaceName'],
'workspaceDescription' => $eventData['workspaceDescription'] ?? '',
'baseWorkspaceName' => $eventData['baseWorkspaceName'] ?? null,
'workspaceOwner' => $eventData['workspaceOwner'] ?? null,
];
break;
case 'WorkspaceBaseWorkspaceWasChanged':
Expand Down Expand Up @@ -574,78 +558,72 @@ public function migrateWorkspaceMetadataToWorkspaceService(\Closure $outputFn):

$outputFn(sprintf('Found %d legacy workspace events resulting in %d workspaces.', $numberOfHandledWorkspaceEvents, count($workspaces)));

// adding metadata
// adding metadata & role assignments
$addedWorkspaceMetadata = 0;
foreach ($workspaces as $workspaceRow) {
$workspaceName = WorkspaceName::fromString($workspaceRow['workspaceName']);
$outputFn(sprintf('Workspace "%s"', $workspaceName->value));
$baseWorkspaceName = isset($workspaceRow['baseWorkspaceName']) ? WorkspaceName::fromString($workspaceRow['baseWorkspaceName']) : null;
$workspaceOwner = $workspaceRow['workspaceOwner'] ?? null;
$isPersonalWorkspace = str_starts_with($workspaceName->value, 'user-');
$isPrivateWorkspace = $workspaceOwner !== null && !$isPersonalWorkspace;
$isInternalWorkspace = $baseWorkspaceName !== null && $workspaceOwner === null;

$query = <<<SQL
SELECT COUNT(1) FROM neos_neos_workspace_metadata WHERE
content_repository_id = :contentRepositoryId
AND workspace_name = :workspaceName
SQL;
$metadataExists = $this->connection->fetchOne($query, [
'contentRepositoryId' => $this->contentRepositoryId->value,
'workspaceName' => $workspaceName->value,
]);
if ($metadataExists !== 0) {
$outputFn(sprintf('Metadata for "%s" exists already.', $workspaceName->value));
continue;
}
if ($baseWorkspaceName === null) {
$classification = WorkspaceClassification::ROOT;
} elseif ($isPersonalWorkspace) {
$classification = WorkspaceClassification::PERSONAL;
} else {
$classification = WorkspaceClassification::SHARED;
}
$this->connection->insert('neos_neos_workspace_metadata', [
'content_repository_id' => $this->contentRepositoryId->value,
'workspace_name' => $workspaceName->value,
'title' => $workspaceRow['workspaceTitle'],
'description' => $workspaceRow['workspaceDescription'],
'classification' => $classification->value,
'owner_user_id' => $isPersonalWorkspace ? $workspaceOwner : null,
]);
if ($workspaceName->isLive()) {
$this->connection->insert('neos_neos_workspace_role', [
try {
$this->connection->insert('neos_neos_workspace_metadata', [
'content_repository_id' => $this->contentRepositoryId->value,
'workspace_name' => $workspaceName->value,
'title' => $workspaceRow['workspaceTitle'],
'description' => $workspaceRow['workspaceDescription'],
'classification' => $classification->value,
'owner_user_id' => $isPersonalWorkspace ? $workspaceOwner : null,
]);
$outputFn(' Added metadata');
} catch (UniqueConstraintViolationException) {
$outputFn(' Metadata already exists');
}
$roleAssignment = [];
if ($workspaceName->isLive()) {
$roleAssignment = [
'subject_type' => WorkspaceRoleSubjectType::GROUP->value,
'subject' => 'Neos.Neos:LivePublisher',
'role' => WorkspaceRole::COLLABORATOR->value,
]);
];
} elseif ($isInternalWorkspace) {
$this->connection->insert('neos_neos_workspace_role', [
'content_repository_id' => $this->contentRepositoryId->value,
'workspace_name' => $workspaceName->value,
$roleAssignment = [
'subject_type' => WorkspaceRoleSubjectType::GROUP->value,
'subject' => 'Neos.Neos:AbstractEditor',
'role' => WorkspaceRole::COLLABORATOR->value,
]);
];
} elseif ($isPrivateWorkspace) {
$this->connection->insert('neos_neos_workspace_role', [
'content_repository_id' => $this->contentRepositoryId->value,
'workspace_name' => $workspaceName->value,
$roleAssignment = [
'subject_type' => WorkspaceRoleSubjectType::USER->value,
'subject' => $workspaceOwner,
'role' => WorkspaceRole::COLLABORATOR->value,
]);
];
}
if ($roleAssignment !== []) {
try {
$this->connection->insert('neos_neos_workspace_role', [
'content_repository_id' => $this->contentRepositoryId->value,
'workspace_name' => $workspaceName->value,
...$roleAssignment,
]);
$outputFn(' Role assignment added');
} catch (UniqueConstraintViolationException) {
$outputFn(' Role assignment already exists');
}
}
$outputFn(sprintf('Added metadata for "%s".', $workspaceName->value));
$addedWorkspaceMetadata++;
}
if ($addedWorkspaceMetadata === 0) {
$outputFn('Migration was not necessary.');
return;
}

$outputFn(sprintf('Added metadata for %d workspaces.', $addedWorkspaceMetadata));
$outputFn(sprintf('Added metadata & role assignments for %d workspaces.', $addedWorkspaceMetadata));
}

/** ------------------------ */
Expand Down
8 changes: 8 additions & 0 deletions Neos.Neos/Classes/Command/CrCommandController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Neos\ContentRepository\Core\Projection\CatchUpOptions;
use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;
use Neos\ContentRepository\Export\ExportService;
use Neos\ContentRepository\Export\ExportServiceFactory;
use Neos\ContentRepository\Export\ImportService;
Expand All @@ -22,6 +23,9 @@
use Neos\Flow\ResourceManagement\ResourceRepository;
use Neos\Media\Domain\Repository\AssetRepository;
use Neos\Neos\AssetUsage\AssetUsageService;
use Neos\Neos\Domain\Model\WorkspaceRole;
use Neos\Neos\Domain\Model\WorkspaceRoleAssignment;
use Neos\Neos\Domain\Service\WorkspaceService;
use Neos\Utility\Files;

class CrCommandController extends CommandController
Expand All @@ -40,6 +44,7 @@ public function __construct(
private readonly ContentRepositoryRegistry $contentRepositoryRegistry,
private readonly ProjectionReplayServiceFactory $projectionReplayServiceFactory,
private readonly AssetUsageService $assetUsageService,
private readonly WorkspaceService $workspaceService,
) {
parent::__construct();
}
Expand Down Expand Up @@ -114,6 +119,9 @@ public function importCommand(string $path, string $contentRepository = 'default
$projectionService = $this->contentRepositoryRegistry->buildService($contentRepositoryId, $this->projectionReplayServiceFactory);
$projectionService->replayAllProjections(CatchUpOptions::create());

$this->outputLine('Assigning live workspace role');
$this->workspaceService->assignWorkspaceRole($contentRepositoryId, WorkspaceName::forLive(), WorkspaceRoleAssignment::createForGroup('Neos.Neos:LivePublisher', WorkspaceRole::COLLABORATOR));
Comment on lines +122 to +123
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo as hotfix we also should create a site entity for each imported site node here :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take care of that in a follow-up

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also thie needs to be try catched, as a workspace role might already be assigned if you run this import before and then run cr:prune as the pruning would not prune the assignments ...


$this->outputLine('<success>Done</success>');
}
}
10 changes: 8 additions & 2 deletions Neos.Neos/Classes/Domain/Service/SiteService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,20 @@ class SiteService
*/
protected $assetCollectionRepository;

/**
* @Flow\Inject
* @var WorkspaceService
*/
protected $workspaceService;

/**
* Remove given site all nodes for that site and all domains associated.
*/
public function pruneSite(Site $site): void
{
$siteServiceInternals = $this->contentRepositoryRegistry->buildService(
$site->getConfiguration()->contentRepositoryId,
new SiteServiceInternalsFactory()
new SiteServiceInternalsFactory($this->workspaceService)
);

try {
Expand Down Expand Up @@ -176,7 +182,7 @@ public function createSite(

$siteServiceInternals = $this->contentRepositoryRegistry->buildService(
$site->getConfiguration()->contentRepositoryId,
new SiteServiceInternalsFactory()
new SiteServiceInternalsFactory($this->workspaceService)
pKallert marked this conversation as resolved.
Show resolved Hide resolved
);
$siteServiceInternals->createSiteNodeIfNotExists($site, $nodeTypeName);

Expand Down
Loading
Loading