From f15eb5250b58741600517228a2e0b344c02462f9 Mon Sep 17 00:00:00 2001 From: Bastian Waidelich Date: Fri, 2 Aug 2024 11:31:56 +0200 Subject: [PATCH] FEATURE: Stabilize WorkspaceName value object Extracted from #5146 this just improves stability of the `WorkspaceName` value object by - Restricting the allowed value range to 30 lower case characters and properly enforce it - Adding a `tryFromString()` constructor - Exposing the `MAX_LENGTH` and use that for the corresponding database schemas - 100% test coverage Related: #4726 --- .../Workspace/WorkspaceProjection.php | 4 +- .../SharedModel/Workspace/WorkspaceName.php | 44 +++-- .../Workspace/WorkspaceNameTest.php | 154 ++++++++++++++++++ 3 files changed, 185 insertions(+), 17 deletions(-) create mode 100644 Neos.ContentRepository.Core/Tests/Unit/SharedModel/Workspace/WorkspaceNameTest.php diff --git a/Neos.ContentRepository.Core/Classes/Projection/Workspace/WorkspaceProjection.php b/Neos.ContentRepository.Core/Classes/Projection/Workspace/WorkspaceProjection.php index 72409d38bfe..2b1ea7960a6 100644 --- a/Neos.ContentRepository.Core/Classes/Projection/Workspace/WorkspaceProjection.php +++ b/Neos.ContentRepository.Core/Classes/Projection/Workspace/WorkspaceProjection.php @@ -113,8 +113,8 @@ private function determineRequiredSqlStatements(): array { $schemaManager = $this->dbal->createSchemaManager(); $workspaceTable = new Table($this->tableName, [ - (new Column('workspacename', Type::getType(Types::STRING)))->setLength(255)->setNotnull(true)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), - (new Column('baseworkspacename', Type::getType(Types::STRING)))->setLength(255)->setNotnull(false)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), + (new Column('workspacename', Type::getType(Types::STRING)))->setLength(WorkspaceName::MAX_LENGTH)->setNotnull(true)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), + (new Column('baseworkspacename', Type::getType(Types::STRING)))->setLength(WorkspaceName::MAX_LENGTH)->setNotnull(false)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), (new Column('workspacetitle', Type::getType(Types::STRING)))->setLength(255)->setNotnull(true)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), (new Column('workspacedescription', Type::getType(Types::STRING)))->setLength(255)->setNotnull(true)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), (new Column('workspaceowner', Type::getType(Types::STRING)))->setLength(255)->setNotnull(false)->setPlatformOption('collation', self::DEFAULT_TEXT_COLLATION), diff --git a/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/WorkspaceName.php b/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/WorkspaceName.php index 37ac2fc8c24..512c52aa368 100644 --- a/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/WorkspaceName.php +++ b/Neos.ContentRepository.Core/Classes/SharedModel/Workspace/WorkspaceName.php @@ -23,6 +23,10 @@ */ final class WorkspaceName implements \JsonSerializable { + public const MAX_LENGTH = 30; + + private const PATTERN = '/^[a-z][a-z0-9\-]{0,' . (self::MAX_LENGTH - 1) . '}$/'; + public const WORKSPACE_NAME_LIVE = 'live'; /** @@ -33,8 +37,8 @@ final class WorkspaceName implements \JsonSerializable private function __construct( public readonly string $value ) { - if (preg_match('/^[\p{L}\p{P}\d \.]{1,200}$/u', $value) !== 1) { - throw new \InvalidArgumentException('Invalid workspace name given.', 1505826610318); + if (!self::hasValidFormat($value)) { + throw new \InvalidArgumentException('Invalid workspace name given.', 1505826610); } } @@ -48,6 +52,11 @@ public static function fromString(string $value): self return self::instance($value); } + public static function tryFromString(string $value): ?self + { + return self::hasValidFormat($value) ? self::instance($value) : null; + } + public static function forLive(): self { return self::instance(self::WORKSPACE_NAME_LIVE); @@ -61,30 +70,30 @@ public static function forLive(): self */ public static function transliterateFromString(string $name): self { - try { - // Check if name already match name pattern to prevent unnecessary transliteration + if (self::hasValidFormat($name)) { return self::fromString($name); - } catch (\InvalidArgumentException $e) { - // Okay, let's transliterate } - $originalName = $name; + $originalName = strtolower($name); // Transliterate (transform 北京 to 'Bei Jing') $name = Transliterator::transliterate($name); - // Urlization (replace spaces with dash, special special characters) - $name = Transliterator::urlize($name); - // Ensure only allowed characters are left - $name = preg_replace('/[^a-z0-9\-]/', '', $name); + $name = (string)preg_replace('/[^a-z0-9\-]/', '', $name); - // Make sure we don't have an empty string left. - if (empty($name)) { - $name = 'workspace-' . strtolower(md5($originalName)); + // Ensure max length... + if (strlen($name) > self::MAX_LENGTH) { + $name = substr($name, 0, self::MAX_LENGTH); } - return new self($name); + // If the name is still invalid at this point, we fall back to md5 + if (!self::hasValidFormat($name)) { + $prefix = 'workspace-'; + $name = $prefix . substr(md5($originalName), 0, self::MAX_LENGTH - strlen($prefix)); + } + + return self::fromString($name); } public function isLive(): bool @@ -101,4 +110,9 @@ public function equals(self $other): bool { return $this === $other; } + + private static function hasValidFormat(string $value): bool + { + return preg_match(self::PATTERN, $value) === 1; + } } diff --git a/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Workspace/WorkspaceNameTest.php b/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Workspace/WorkspaceNameTest.php new file mode 100644 index 00000000000..6d01e897715 --- /dev/null +++ b/Neos.ContentRepository.Core/Tests/Unit/SharedModel/Workspace/WorkspaceNameTest.php @@ -0,0 +1,154 @@ +value, $value); + } + + /** + * @test + * @dataProvider validWorkspaceNames + */ + public function tryFromStringReturnsInstanceForValidValues(string $value): void + { + self::assertSame(WorkspaceName::tryFromString($value)->value, $value); + } + + private static function invalidWorkspaceNames(): iterable + { + yield 'empty string' => ['']; + yield 'only digits' => ['123']; + yield 'leading dash' => ['-invalid']; + yield 'upper case characters' => ['thisIsNotAllowed']; + yield 'whitespace' => ['this neither']; + yield 'exceeding max length' => ['this-is-just-a-little-too-long-']; + } + + /** + * @test + * @dataProvider invalidWorkspaceNames + */ + public function fromStringFailsForInvalidValues(string $value): void + { + $this->expectException(\InvalidArgumentException::class); + WorkspaceName::fromString($value); + } + + /** + * @test + * @dataProvider invalidWorkspaceNames + */ + public function tryFromStringReturnsNullForInvalidValues(string $value): void + { + self::assertNull(WorkspaceName::tryFromString($value)); + } + + /** + * @test + */ + public function forLiveReturnsAConstantInstance(): void + { + self::assertSame(WorkspaceName::fromString(WorkspaceName::WORKSPACE_NAME_LIVE), WorkspaceName::forLive()); + } + + private static function transliterateFromStringDataProvider(): iterable + { + yield 'valid name is not changed' => ['value' => 'already-valid', 'expectedResult' => 'already-valid']; + yield 'name is lower-cased' => ['value' => 'mixedCase', 'expectedResult' => 'mixedcase']; + yield 'chinese characters' => ['value' => '北京', 'expectedResult' => 'bei-jing']; + yield 'german umlauts' => ['value' => 'ümläute', 'expectedResult' => 'umlaute']; + yield 'white space' => ['value' => ' Contains spaces ', 'expectedResult' => 'contains-spaces']; + yield 'exceeding max length' => ['value' => 'This name is just a little too long', 'expectedResult' => 'this-name-is-just-a-little-too']; + yield 'only special characters' => ['value' => '-', 'expectedResult' => 'workspace-336d5ebc5436534e61d1']; + } + + /** + * @test + * @dataProvider transliterateFromStringDataProvider + */ + public function transliterateFromStringTests(string $value, string $expectedResult): void + { + self::assertSame($expectedResult, WorkspaceName::transliterateFromString($value)->value); + } + + /** + * @test + */ + public function isLiveReturnsFalseByDefault(): void + { + self::assertFalse(WorkspaceName::fromString('not-live')->isLive()); + } + + /** + * @test + */ + public function isLiveReturnsTrueForLiveWorkspace(): void + { + self::assertTrue(WorkspaceName::forLive()->isLive()); + } + + /** + * @test + */ + public function jsonSerializeReturnsPlainValue(): void + { + self::assertJsonStringEqualsJsonString(json_encode(WorkspaceName::forLive()), '"live"'); + } + + /** + * @test + */ + public function equalsReturnsFalseIfTwoInstancesDontMatch(): void + { + self::assertFalse(WorkspaceName::fromString('some-workspace')->equals(WorkspaceName::fromString('some-other-workspace'))); + } + + /** + * @test + */ + public function equalsReturnsTrueIfTwoInstancesMatch(): void + { + self::assertTrue(WorkspaceName::fromString('some-workspace')->equals(WorkspaceName::fromString('some-workspace'))); + } +}