Merge pull request #2248 from acelaya-forks/feature/api-key-simplification

Simplify ApiKey entity by exposing key as a readonly prop

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

@@ -109,7 +109,7 @@ class GenerateKeyCommand extends Command
         ));
 
         $io = new SymfonyStyle($input, $output);
-        $io->success(sprintf('Generated API key: "%s"', $apiKey->toString()));
+        $io->success(sprintf('Generated API key: "%s"', $apiKey->key));
 
         if (! ApiKey::isAdmin($apiKey)) {
             ShlinkTable::default($io)->render(

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

@@ -54,7 +54,7 @@ class ListKeysCommand extends Command
             $messagePattern = $this->determineMessagePattern($apiKey);
 
             // Set columns for this row
-            $rowData = [sprintf($messagePattern, $apiKey), sprintf($messagePattern, $apiKey->name ?? '-')];
+            $rowData = [sprintf($messagePattern, $apiKey->key), sprintf($messagePattern, $apiKey->name ?? '-')];
             if (! $enabledOnly) {
                 $rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
             }

module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php

@@ -244,7 +244,7 @@ class ListShortUrlsCommand extends Command
         }
         if ($input->getOption('show-api-key')) {
             $columnsMap['API Key'] = static fn (array $_, ShortUrl $shortUrl): string =>
-                $shortUrl->authorApiKey?->__toString() ?? '';
+                $shortUrl->authorApiKey?->key ?? '';
         }
         if ($input->getOption('show-api-key-name')) {
             $columnsMap['API Key Name'] = static fn (array $_, ShortUrl $shortUrl): string|null =>

module/CLI/test/Command/Api/ListKeysCommandTest.php

@@ -55,11 +55,11 @@ class ListKeysCommandTest extends TestCase
             +--------------------------------------+------+------------+---------------------------+-------+
             | Key                                  | Name | Is enabled | Expiration date           | Roles |
             +--------------------------------------+------+------------+---------------------------+-------+
-            | {$apiKey1} | -    | ---        | -                         | Admin |
+            | {$apiKey1->key} | -    | ---        | -                         | Admin |
             +--------------------------------------+------+------------+---------------------------+-------+
-            | {$apiKey2} | -    | ---        | 2020-01-01T00:00:00+00:00 | Admin |
+            | {$apiKey2->key} | -    | ---        | 2020-01-01T00:00:00+00:00 | Admin |
             +--------------------------------------+------+------------+---------------------------+-------+
-            | {$apiKey3} | -    | +++        | -                         | Admin |
+            | {$apiKey3->key} | -    | +++        | -                         | Admin |
             +--------------------------------------+------+------------+---------------------------+-------+
 
             OUTPUT,
@@ -71,9 +71,9 @@ class ListKeysCommandTest extends TestCase
             +--------------------------------------+------+-----------------+-------+
             | Key                                  | Name | Expiration date | Roles |
             +--------------------------------------+------+-----------------+-------+
-            | {$apiKey1} | -    | -               | Admin |
+            | {$apiKey1->key} | -    | -               | Admin |
             +--------------------------------------+------+-----------------+-------+
-            | {$apiKey2} | -    | -               | Admin |
+            | {$apiKey2->key} | -    | -               | Admin |
             +--------------------------------------+------+-----------------+-------+
 
             OUTPUT,
@@ -97,18 +97,18 @@ class ListKeysCommandTest extends TestCase
             +--------------------------------------+------+-----------------+--------------------------+
             | Key                                  | Name | Expiration date | Roles                    |
             +--------------------------------------+------+-----------------+--------------------------+
-            | {$apiKey1} | -    | -               | Admin                    |
+            | {$apiKey1->key} | -    | -               | Admin                    |
             +--------------------------------------+------+-----------------+--------------------------+
-            | {$apiKey2} | -    | -               | Author only              |
+            | {$apiKey2->key} | -    | -               | Author only              |
             +--------------------------------------+------+-----------------+--------------------------+
-            | {$apiKey3} | -    | -               | Domain only: example.com |
+            | {$apiKey3->key} | -    | -               | Domain only: example.com |
             +--------------------------------------+------+-----------------+--------------------------+
-            | {$apiKey4} | -    | -               | Admin                    |
+            | {$apiKey4->key} | -    | -               | Admin                    |
             +--------------------------------------+------+-----------------+--------------------------+
-            | {$apiKey5} | -    | -               | Author only              |
+            | {$apiKey5->key} | -    | -               | Author only              |
             |                                      |      |                 | Domain only: example.com |
             +--------------------------------------+------+-----------------+--------------------------+
-            | {$apiKey6} | -    | -               | Admin                    |
+            | {$apiKey6->key} | -    | -               | Admin                    |
             +--------------------------------------+------+-----------------+--------------------------+
 
             OUTPUT,
@@ -125,13 +125,13 @@ class ListKeysCommandTest extends TestCase
             +--------------------------------------+---------------+-----------------+-------+
             | Key                                  | Name          | Expiration date | Roles |
             +--------------------------------------+---------------+-----------------+-------+
-            | {$apiKey1} | Alice         | -               | Admin |
+            | {$apiKey1->key} | Alice         | -               | Admin |
             +--------------------------------------+---------------+-----------------+-------+
-            | {$apiKey2} | Alice and Bob | -               | Admin |
+            | {$apiKey2->key} | Alice and Bob | -               | Admin |
             +--------------------------------------+---------------+-----------------+-------+
-            | {$apiKey3} |               | -               | Admin |
+            | {$apiKey3->key} |               | -               | Admin |
             +--------------------------------------+---------------+-----------------+-------+
-            | {$apiKey4} | -             | -               | Admin |
+            | {$apiKey4->key} | -             | -               | Admin |
             +--------------------------------------+---------------+-----------------+-------+
 
             OUTPUT,

module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php

@@ -140,7 +140,7 @@ class ListShortUrlsCommandTest extends TestCase
     public static function provideOptionalFlags(): iterable
     {
         $apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'my api key'));
-        $key = $apiKey->toString();
+        $key = $apiKey->key;
 
         yield 'tags only' => [
             ['--show-tags' => true],

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

@@ -7,16 +7,16 @@ namespace Shlinkio\Shlink\Rest\ApiKey\Model;
 use Cake\Chronos\Chronos;
 use Ramsey\Uuid\Uuid;
 
-final class ApiKeyMeta
+final readonly class ApiKeyMeta
 {
     /**
      * @param iterable<RoleDefinition> $roleDefinitions
      */
     private function __construct(
-        public readonly string $key,
+        public string $key,
-        public readonly string|null $name,
+        public string|null $name,
-        public readonly Chronos|null $expirationDate,
+        public Chronos|null $expirationDate,
-        public readonly iterable $roleDefinitions,
+        public iterable $roleDefinitions,
     ) {
     }
 

module/Rest/src/Entity/ApiKey.php

@@ -19,10 +19,9 @@ class ApiKey extends AbstractEntity
 {
     /**
      * @param Collection<string, ApiKeyRole> $roles
-     * @throws Exception
      */
     private function __construct(
-        private string $key,
+        public readonly string $key,
         public readonly string|null $name = null,
         public readonly Chronos|null $expirationDate = null,
         private bool $enabled = true,
@@ -75,16 +74,6 @@ class ApiKey extends AbstractEntity
         return $this->isEnabled() && ! $this->isExpired();
     }
 
-    public function __toString(): string
-    {
-        return $this->key;
-    }
-
-    public function toString(): string
-    {
-        return $this->key;
-    }
-
     public function spec(string|null $context = null): Specification
     {
         $specs = $this->roles->map(fn (ApiKeyRole $role) => Role::toSpec($role, $context))->getValues();

module/Rest/test-api/Fixtures/ApiKeyFixture.php

@@ -8,7 +8,6 @@ use Cake\Chronos\Chronos;
 use Doctrine\Common\DataFixtures\AbstractFixture;
 use Doctrine\Common\DataFixtures\DependentFixtureInterface;
 use Doctrine\Persistence\ObjectManager;
-use ReflectionObject;
 use Shlinkio\Shlink\Core\Domain\Entity\Domain;
 use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta;
 use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition;
@@ -51,12 +50,7 @@ class ApiKeyFixture extends AbstractFixture implements DependentFixtureInterface
 
     private function buildApiKey(string $key, bool $enabled, Chronos|null $expiresAt = null): ApiKey
     {
-        $apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(expirationDate: $expiresAt));
+        $apiKey = ApiKey::fromMeta(ApiKeyMeta::fromParams(key: $key, expirationDate: $expiresAt));
-        $ref = new ReflectionObject($apiKey);
-        $keyProp = $ref->getProperty('key');
-        $keyProp->setAccessible(true);
-        $keyProp->setValue($apiKey, $key);
-
         if (! $enabled) {
             $apiKey->disable();
         }

module/Rest/test/Middleware/AuthenticationMiddlewareTest.php

@@ -130,18 +130,17 @@ class AuthenticationMiddlewareTest extends TestCase
     public function validApiKeyFallsBackToNextMiddleware(): void
     {
         $apiKey = ApiKey::create();
-        $key = $apiKey->toString();
         $request = ServerRequestFactory::fromGlobals()
             ->withAttribute(
                 RouteResult::class,
                 RouteResult::fromRoute(new Route('bar', self::getDummyMiddleware()), []),
             )
-            ->withHeader('X-Api-Key', $key);
+            ->withHeader('X-Api-Key', $apiKey->key);
 
         $this->handler->expects($this->once())->method('handle')->with(
             $request->withAttribute(ApiKey::class, $apiKey),
         )->willReturn(new Response());
-        $this->apiKeyService->expects($this->once())->method('check')->with($key)->willReturn(
+        $this->apiKeyService->expects($this->once())->method('check')->with($apiKey->key)->willReturn(
             new ApiKeyCheckResult($apiKey),
         );