Converted Role constants to enum

module/CLI/src/Command/Api/GenerateKeyCommand.php

@@ -73,13 +73,16 @@ class GenerateKeyCommand extends Command
                 $authorOnly,
                 'a',
                 InputOption::VALUE_NONE,
-                sprintf('Adds the "%s" role to the new API key.', Role::AUTHORED_SHORT_URLS),
+                sprintf('Adds the "%s" role to the new API key.', Role::AUTHORED_SHORT_URLS->value),
             )
             ->addOption(
                 $domainOnly,
                 'd',
                 InputOption::VALUE_REQUIRED,
-                sprintf('Adds the "%s" role to the new API key, with the domain provided.', Role::DOMAIN_SPECIFIC),
+                sprintf(
+                    'Adds the "%s" role to the new API key, with the domain provided.',
+                    Role::DOMAIN_SPECIFIC->value,
+                ),
             )
             ->setHelp($help);
     }
@@ -99,7 +102,7 @@ class GenerateKeyCommand extends Command
         if (! $apiKey->isAdmin()) {
             ShlinkTable::default($io)->render(
                 ['Role name', 'Role metadata'],
-                $apiKey->mapRoles(fn (string $name, array $meta) => [$name, arrayToString($meta, 0)]),
+                $apiKey->mapRoles(fn (Role $role, array $meta) => [$role->value, arrayToString($meta, 0)]),
                 null,
                 'Roles',
             );

module/CLI/src/Command/Api/ListKeysCommand.php

@@ -60,10 +60,10 @@ class ListKeysCommand extends Command
             }
             $rowData[] = $expiration?->toAtomString() ?? '-';
             $rowData[] = $apiKey->isAdmin() ? 'Admin' : implode("\n", $apiKey->mapRoles(
-                fn (string $roleName, array $meta) =>
+                fn (Role $role, array $meta) =>
                     empty($meta)
-                        ? Role::toFriendlyName($roleName)
+                        ? Role::toFriendlyName($role)
-                        : sprintf('%s: %s', Role::toFriendlyName($roleName), Role::domainAuthorityFromMeta($meta)),
+                        : sprintf('%s: %s', Role::toFriendlyName($role), Role::domainAuthorityFromMeta($meta)),
             ));
 
             return $rowData;

module/CLI/src/Exception/InvalidRoleConfigException.php

@@ -16,7 +16,7 @@ class InvalidRoleConfigException extends InvalidArgumentException implements Exc
         return new self(sprintf(
             'You cannot create an API key with the "%s" role attached to the default domain. '
             . 'The role is currently limited to non-default domains.',
-            Role::DOMAIN_SPECIFIC,
+            Role::DOMAIN_SPECIFIC->value,
         ));
     }
 }

module/CLI/test/Exception/InvalidRoleConfigExceptionTest.php

@@ -20,7 +20,7 @@ class InvalidRoleConfigExceptionTest extends TestCase
         self::assertEquals(sprintf(
             'You cannot create an API key with the "%s" role attached to the default domain. '
             . 'The role is currently limited to non-default domains.',
-            Role::DOMAIN_SPECIFIC,
+            Role::DOMAIN_SPECIFIC->value,
         ), $e->getMessage());
     }
 }

module/Core/src/Domain/Repository/DomainRepository.php

@@ -7,7 +7,6 @@ namespace Shlinkio\Shlink\Core\Domain\Repository;
 use Doctrine\ORM\Query\Expr\Join;
 use Doctrine\ORM\QueryBuilder;
 use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
-use Happyr\DoctrineSpecification\Spec;
 use Shlinkio\Shlink\Core\Domain\Spec\IsDomain;
 use Shlinkio\Shlink\Core\Entity\Domain;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
@@ -77,10 +76,9 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe
         // FIXME The $apiKey->spec() method cannot be used here, as it returns a single spec which assumes the
         //       ShortUrl is the root entity. Here, the Domain is the root entity.
         //       Think on a way to centralize the conditional behavior and make $apiKey->spec() more flexible.
-        yield from $apiKey?->mapRoles(fn (string $roleName, array $meta) => match ($roleName) {
+        yield from $apiKey?->mapRoles(fn (Role $role, array $meta) => match ($role) {
             Role::DOMAIN_SPECIFIC => ['d', new IsDomain(Role::domainIdFromMeta($meta))],
             Role::AUTHORED_SHORT_URLS => ['s', new BelongsToApiKey($apiKey)],
-            default => [null, Spec::andX()],
         }) ?? [];
     }
 }

module/Core/src/Repository/TagRepository.php

@@ -81,14 +81,13 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
             ->groupBy('t.id_0', 't.name_1');
 
         // Apply API key role conditions to the native query too, as they will affect the amounts on the aggregates
-        $apiKey?->mapRoles(static fn (string $roleName, array $meta) => match ($roleName) {
+        $apiKey?->mapRoles(static fn (Role $role, array $meta) => match ($role) {
             Role::DOMAIN_SPECIFIC => $nativeQb->andWhere(
                 $nativeQb->expr()->eq('s.domain_id', $conn->quote(Role::domainIdFromMeta($meta))),
             ),
             Role::AUTHORED_SHORT_URLS => $nativeQb->andWhere(
                 $nativeQb->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())),
             ),
-            default => $nativeQb,
         });
 
         if ($orderMainQuery) {

module/Rest/config/entities-mappings/Shlinkio.Shlink.Rest.Entity.ApiKeyRole.php

@@ -6,7 +6,9 @@ namespace Shlinkio\Shlink\Rest;
 
 use Doctrine\DBAL\Types\Types;
 use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
+use Doctrine\ORM\Mapping\Builder\FieldBuilder;
 use Doctrine\ORM\Mapping\ClassMetadata;
+use Shlinkio\Shlink\Rest\ApiKey\Role;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 use function Shlinkio\Shlink\Core\determineTableName;
@@ -22,11 +24,14 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
             ->option('unsigned', true)
             ->build();
 
-    $builder->createField('roleName', Types::STRING)
+    (new FieldBuilder($builder, [
-            ->columnName('role_name')
+        'fieldName' => 'roleName',
-            ->length(255)
+        'type' => Types::STRING,
-            ->nullable(false)
+        'enumType' => Role::class,
-            ->build();
+    ]))->columnName('role_name')
+       ->length(255)
+       ->nullable(false)
+       ->build();
 
     $builder->createField('meta', Types::JSON)
             ->columnName('meta')

module/Rest/src/ApiKey/Model/RoleDefinition.php

@@ -9,7 +9,7 @@ use Shlinkio\Shlink\Rest\ApiKey\Role;
 
 final class RoleDefinition
 {
-    private function __construct(public readonly string $roleName, public readonly array $meta)
+    private function __construct(public readonly Role $role, public readonly array $meta)
     {
     }
 

module/Rest/src/ApiKey/Role.php

@@ -2,6 +2,8 @@
 
 declare(strict_types=1);
 
+// phpcs:disable
+// TODO Enable coding style checks again once code sniffer 3.7 is released https://github.com/squizlabs/PHP_CodeSniffer/issues/3474
 namespace Shlinkio\Shlink\Rest\ApiKey;
 
 use Happyr\DoctrineSpecification\Spec;
@@ -12,31 +14,24 @@ use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToDomain;
 use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToDomainInlined;
 use Shlinkio\Shlink\Rest\Entity\ApiKeyRole;
 
-// TODO Convert to enum
+enum Role: string
-class Role
 {
-    public const AUTHORED_SHORT_URLS = 'AUTHORED_SHORT_URLS';
+    case AUTHORED_SHORT_URLS = 'AUTHORED_SHORT_URLS';
-    public const DOMAIN_SPECIFIC = 'DOMAIN_SPECIFIC';
+    case DOMAIN_SPECIFIC = 'DOMAIN_SPECIFIC';
-    private const ROLE_FRIENDLY_NAMES = [
-        self::AUTHORED_SHORT_URLS => 'Author only',
-        self::DOMAIN_SPECIFIC => 'Domain only',
-    ];
 
     public static function toSpec(ApiKeyRole $role, ?string $context = null): Specification
     {
-        return match ($role->name()) {
+        return match ($role->role()) {
             self::AUTHORED_SHORT_URLS => new BelongsToApiKey($role->apiKey(), $context),
             self::DOMAIN_SPECIFIC => new BelongsToDomain(self::domainIdFromMeta($role->meta()), $context),
-            default => Spec::andX(),
         };
     }
 
     public static function toInlinedSpec(ApiKeyRole $role): Specification
     {
-        return match ($role->name()) {
+        return match ($role->role()) {
             self::AUTHORED_SHORT_URLS => Spec::andX(new BelongsToApiKeyInlined($role->apiKey())),
             self::DOMAIN_SPECIFIC => Spec::andX(new BelongsToDomainInlined(self::domainIdFromMeta($role->meta()))),
-            default => Spec::andX(),
         };
     }
 
@@ -50,8 +45,11 @@ class Role
         return $meta['authority'] ?? '';
     }
 
-    public static function toFriendlyName(string $roleName): string
+    public static function toFriendlyName(Role $role): string
     {
-        return self::ROLE_FRIENDLY_NAMES[$roleName] ?? '';
+        return match ($role) {
+            self::AUTHORED_SHORT_URLS => 'Author only',
+            self::DOMAIN_SPECIFIC => 'Domain only',
+        };
     }
 }

module/Rest/src/Entity/ApiKey.php

@@ -113,40 +113,40 @@ class ApiKey extends AbstractEntity
         return $this->roles->isEmpty();
     }
 
-    public function hasRole(string $roleName): bool
+    public function hasRole(Role $role): bool
     {
-        return $this->roles->containsKey($roleName);
+        return $this->roles->containsKey($role->value);
     }
 
-    public function getRoleMeta(string $roleName): array
+    public function getRoleMeta(Role $role): array
     {
-        /** @var ApiKeyRole|null $role */
+        /** @var ApiKeyRole|null $apiKeyRole */
-        $role = $this->roles->get($roleName);
+        $apiKeyRole = $this->roles->get($role->value);
-        return $role?->meta() ?? [];
+        return $apiKeyRole?->meta() ?? [];
     }
 
     /**
      * @template T
-     * @param callable(string $roleName, array $meta): T $fun
+     * @param callable(Role $role, array $meta): T $fun
      * @return T[]
      */
     public function mapRoles(callable $fun): array
     {
-        return $this->roles->map(fn (ApiKeyRole $role) => $fun($role->name(), $role->meta()))->getValues();
+        return $this->roles->map(fn (ApiKeyRole $role) => $fun($role->role(), $role->meta()))->getValues();
     }
 
     public function registerRole(RoleDefinition $roleDefinition): void
     {
-        $roleName = $roleDefinition->roleName;
+        $role = $roleDefinition->role;
         $meta = $roleDefinition->meta;
 
-        if ($this->hasRole($roleName)) {
+        if ($this->hasRole($role)) {
-            /** @var ApiKeyRole $role */
+            /** @var ApiKeyRole $apiKeyRole */
-            $role = $this->roles->get($roleName);
+            $apiKeyRole = $this->roles->get($role);
-            $role->updateMeta($meta);
+            $apiKeyRole->updateMeta($meta);
         } else {
-            $role = new ApiKeyRole($roleDefinition->roleName, $roleDefinition->meta, $this);
+            $apiKeyRole = new ApiKeyRole($roleDefinition->role, $roleDefinition->meta, $this);
-            $this->roles[$roleName] = $role;
+            $this->roles[$role->value] = $apiKeyRole;
         }
     }
 }

module/Rest/src/Entity/ApiKeyRole.php

@@ -5,18 +5,25 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Rest\Entity;
 
 use Shlinkio\Shlink\Common\Entity\AbstractEntity;
+use Shlinkio\Shlink\Rest\ApiKey\Role;
 
 class ApiKeyRole extends AbstractEntity
 {
-    public function __construct(private string $roleName, private array $meta, private ApiKey $apiKey)
+    public function __construct(private Role $roleName, private array $meta, private ApiKey $apiKey)
     {
     }
 
-    public function name(): string
+    public function role(): Role
     {
         return $this->roleName;
     }
 
+    /** @deprecated Use role() instead */
+    public function name(): Role
+    {
+        return $this->role();
+    }
+
     public function meta(): array
     {
         return $this->meta;

module/Rest/test/ApiKey/Model/RoleDefinitionTest.php

@@ -16,7 +16,7 @@ class RoleDefinitionTest extends TestCase
     {
         $definition = RoleDefinition::forAuthoredShortUrls();
 
-        self::assertEquals(Role::AUTHORED_SHORT_URLS, $definition->roleName);
+        self::assertEquals(Role::AUTHORED_SHORT_URLS, $definition->role);
         self::assertEquals([], $definition->meta);
     }
 
@@ -26,7 +26,7 @@ class RoleDefinitionTest extends TestCase
         $domain = Domain::withAuthority('foo.com')->setId('123');
         $definition = RoleDefinition::forDomain($domain);
 
-        self::assertEquals(Role::DOMAIN_SPECIFIC, $definition->roleName);
+        self::assertEquals(Role::DOMAIN_SPECIFIC, $definition->role);
         self::assertEquals(['domain_id' => '123', 'authority' => 'foo.com'], $definition->meta);
     }
 }

module/Rest/test/ApiKey/RoleTest.php

@@ -30,7 +30,6 @@ class RoleTest extends TestCase
     {
         $apiKey = ApiKey::create();
 
-        yield 'invalid role' => [new ApiKeyRole('invalid', [], $apiKey), Spec::andX()];
         yield 'author role' => [
             new ApiKeyRole(Role::AUTHORED_SHORT_URLS, [], $apiKey),
             new BelongsToApiKey($apiKey),
@@ -54,7 +53,6 @@ class RoleTest extends TestCase
     {
         $apiKey = ApiKey::create();
 
-        yield 'invalid role' => [new ApiKeyRole('invalid', [], $apiKey), Spec::andX()];
         yield 'author role' => [
             new ApiKeyRole(Role::AUTHORED_SHORT_URLS, [], $apiKey),
             Spec::andX(new BelongsToApiKeyInlined($apiKey)),
@@ -101,15 +99,14 @@ class RoleTest extends TestCase
      * @test
      * @dataProvider provideRoleNames
      */
-    public function getsExpectedRoleFriendlyName(string $roleName, string $expectedFriendlyName): void
+    public function getsExpectedRoleFriendlyName(Role $roleName, string $expectedFriendlyName): void
     {
         self::assertEquals($expectedFriendlyName, Role::toFriendlyName($roleName));
     }
 
     public function provideRoleNames(): iterable
     {
-        yield 'unknown' => ['unknown', ''];
+        yield Role::AUTHORED_SHORT_URLS->value => [Role::AUTHORED_SHORT_URLS, 'Author only'];
-        yield Role::AUTHORED_SHORT_URLS => [Role::AUTHORED_SHORT_URLS, 'Author only'];
+        yield Role::DOMAIN_SPECIFIC->value => [Role::DOMAIN_SPECIFIC, 'Domain only'];
-        yield Role::DOMAIN_SPECIFIC => [Role::DOMAIN_SPECIFIC, 'Domain only'];
     }
 }

module/Rest/test/Service/ApiKeyServiceTest.php

@@ -46,7 +46,7 @@ class ApiKeyServiceTest extends TestCase
         self::assertEquals($date, $key->getExpirationDate());
         self::assertEquals($name, $key->name());
         foreach ($roles as $roleDefinition) {
-            self::assertTrue($key->hasRole($roleDefinition->roleName));
+            self::assertTrue($key->hasRole($roleDefinition->role));
         }
     }