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

TASK: Add migration for workspaceName constraints #5202

Merged
merged 1 commit into from
Aug 15, 2024
Merged

TASK: Add migration for workspaceName constraints #5202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions Neos.ContentRepositoryRegistry/Classes/Command/MigrateEventsCommandController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,20 @@ public function migratePayloadToWorkspaceNameCommand(string $contentRepository =
$eventMigrationService = $this->contentRepositoryRegistry->buildService($contentRepositoryId, $this->eventMigrationServiceFactory);
$eventMigrationService->migratePayloadToWorkspaceName($this->outputLine(...));
}

/**
* Rewrites all workspaceNames, that are not matching new constraints.
*
* Needed for feature "Stabilize WorkspaceName value object": https://github.com/neos/neos-development-collection/pull/5193
*
* Included in August 2024 - before final Neos 9.0 release
*
* @param string $contentRepository Identifier of the Content Repository to migrate
*/
public function migratePayloadToValidWorkspaceNamesCommand(string $contentRepository = 'default'): void
{
$contentRepositoryId = ContentRepositoryId::fromString($contentRepository);
$eventMigrationService = $this->contentRepositoryRegistry->buildService($contentRepositoryId, $this->eventMigrationServiceFactory);
$eventMigrationService->migratePayloadToValidWorkspaceNames($this->outputLine(...));
}
}
38 changes: 38 additions & 0 deletions Neos.ContentRepositoryRegistry/Classes/Service/EventMigrationService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,44 @@ public function migratePayloadToWorkspaceName(\Closure $outputFn): void
$outputFn(sprintf('Migration applied to %s events.', $numberOfMigratedEvents));
}

/**
* Rewrites all workspaceNames, that are not matching new constraints.
*
* Needed for feature "Stabilize WorkspaceName value object": https://github.com/neos/neos-development-collection/pull/5193
*
* Included in August 2024 - before final Neos 9.0 release
*
* @param \Closure $outputFn
* @return void
*/
public function migratePayloadToValidWorkspaceNames(\Closure $outputFn): void
{
$backupEventTableName = DoctrineEventStoreFactory::databaseTableName($this->contentRepositoryId) . '_bkp_' . date('Y_m_d_H_i_s');
$outputFn('Backup: copying events table to %s', [$backupEventTableName]);
$this->copyEventTable($backupEventTableName);

$eventTableName = DoctrineEventStoreFactory::databaseTableName($this->contentRepositoryId);
$this->connection->beginTransaction();
$statement = <<<SQL
UPDATE {$eventTableName}
SET
payload = JSON_SET(payload, '$.workspaceName', SUBSTR(MD5(JSON_UNQUOTE(JSON_EXTRACT(payload, '$.workspaceName'))), 1, 30))
Copy link
Member

Choose a reason for hiding this comment

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

I read through this more in-depth now and I wonder why you used SUBSTR(..., 1, ...) – if the original value was uppercase it would still be invalid afterwards.
My original suggestion was:

CONCAT('w', MD5(JSON_UNQUOTE(JSON_EXTRACT(payload, '$.workspaceName')))))

(note the "w"-prefix)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The md5 hash is 32 chars long, but the workspaceName can just be 30 chars long. So I had to shorten it. To reduce the chance of collision, I removed the prefix to be able to keep 30 chars of the hash instead of 29.

But yeah...

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right, sorry

LEFT(CONCAT('w', MD5(JSON_UNQUOTE(JSON_EXTRACT(payload, '$.workspaceName'))))), 30)

could work

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I hadn't realized: SUBSTR(..., 1, x) is the same as LEFT(..., x) (since it's not zero-based like in PHP).
Still, the first character of an MD5 could be a digit

WHERE
JSON_EXTRACT(payload, '$.workspaceName') IS NOT NULL
AND JSON_UNQUOTE(JSON_EXTRACT(payload, '$.workspaceName')) NOT REGEXP '^[a-z][a-z0-9\-]{0,30}$'
SQL;
$affectedRows = $this->connection->executeStatement($statement);
$this->connection->commit();

if ($affectedRows === 0) {
$outputFn('Migration was not necessary.');
return;
}

$outputFn();
$outputFn(sprintf('Migration applied to %s events.', $affectedRows));
}

/** ------------------------ */

/**
Expand Down
Loading