From 29cdfaed39b0305b6fdfe7b5c64585d81916d729 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 4 Jan 2021 13:32:44 +0100 Subject: [PATCH] Changed ShortUrlMeta so that it expects an ApiKey instance instead of the key as string --- module/Core/src/Entity/ShortUrl.php | 2 +- module/Core/src/Model/ShortUrlMeta.php | 5 +-- .../PersistenceShortUrlRelationResolver.php | 12 ------- .../ShortUrlRelationResolverInterface.php | 3 -- .../SimpleShortUrlRelationResolver.php | 6 ---- .../Validation/ShortUrlMetaInputFilter.php | 7 +++- ...ersistenceShortUrlRelationResolverTest.php | 35 ------------------- .../SimpleShortUrlRelationResolverTest.php | 15 -------- .../Action/ShortUrl/CreateShortUrlAction.php | 2 +- .../SingleStepCreateShortUrlAction.php | 2 +- .../ShortUrl/CreateShortUrlActionTest.php | 2 +- .../SingleStepCreateShortUrlActionTest.php | 6 ++-- 12 files changed, 16 insertions(+), 81 deletions(-) diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 6f7493aa..67d41136 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -59,7 +59,7 @@ class ShortUrl extends AbstractEntity $this->shortCodeLength = $meta->getShortCodeLength(); $this->shortCode = $meta->getCustomSlug() ?? generateRandomShortCode($this->shortCodeLength); $this->domain = $relationResolver->resolveDomain($meta->getDomain()); - $this->authorApiKey = $relationResolver->resolveApiKey($meta->getApiKey()); + $this->authorApiKey = $meta->getApiKey(); } public static function fromImport( diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index fa82919e..0df792be 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Model; use Cake\Chronos\Chronos; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter; use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter; @@ -24,7 +25,7 @@ final class ShortUrlMeta private ?string $domain = null; private int $shortCodeLength = 5; private ?bool $validateUrl = null; - private ?string $apiKey = null; + private ?ApiKey $apiKey = null; // Enforce named constructors private function __construct() @@ -135,7 +136,7 @@ final class ShortUrlMeta return $this->validateUrl; } - public function getApiKey(): ?string + public function getApiKey(): ?ApiKey { return $this->apiKey; } diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index d898fb37..0e3afa23 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\Domain; -use Shlinkio\Shlink\Rest\Entity\ApiKey; class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInterface { @@ -27,15 +26,4 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt $existingDomain = $this->em->getRepository(Domain::class)->findOneBy(['authority' => $domain]); return $existingDomain ?? new Domain($domain); } - - public function resolveApiKey(?string $key): ?ApiKey - { - if ($key === null) { - return null; - } - - /** @var ApiKey|null $existingApiKey */ - $existingApiKey = $this->em->getRepository(ApiKey::class)->findOneBy(['key' => $key]); - return $existingApiKey; - } } diff --git a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php index 0a708cf6..bc576dbd 100644 --- a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php +++ b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php @@ -5,11 +5,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; use Shlinkio\Shlink\Core\Entity\Domain; -use Shlinkio\Shlink\Rest\Entity\ApiKey; interface ShortUrlRelationResolverInterface { public function resolveDomain(?string $domain): ?Domain; - - public function resolveApiKey(?string $key): ?ApiKey; } diff --git a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php index 9de156ee..4e4620f5 100644 --- a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; use Shlinkio\Shlink\Core\Entity\Domain; -use Shlinkio\Shlink\Rest\Entity\ApiKey; class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterface { @@ -13,9 +12,4 @@ class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterfac { return $domain !== null ? new Domain($domain) : null; } - - public function resolveApiKey(?string $key): ?ApiKey - { - return null; - } } diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php index 9d3f8ec5..ca29ad14 100644 --- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php +++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php @@ -11,6 +11,7 @@ use Laminas\InputFilter\InputFilter; use Laminas\Validator; use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\Util\CocurSymfonySluggerBridge; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use const Shlinkio\Shlink\Core\CUSTOM_SLUGS_REGEXP; use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH; @@ -73,7 +74,11 @@ class ShortUrlMetaInputFilter extends InputFilter $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); $this->add($domain); - $this->add($this->createInput(self::API_KEY, false)); + $apiKeyInput = new Input(self::API_KEY); + $apiKeyInput + ->setRequired(false) + ->getValidatorChain()->attach(new Validator\IsInstanceOf(['className' => ApiKey::class])); + $this->add($apiKeyInput); } private function createPositiveNumberInput(string $name, int $min = 1): Input diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 5791d579..9cea7883 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -11,7 +11,6 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; -use Shlinkio\Shlink\Rest\Entity\ApiKey; class PersistenceShortUrlRelationResolverTest extends TestCase { @@ -63,38 +62,4 @@ class PersistenceShortUrlRelationResolverTest extends TestCase yield 'not found domain' => [null, $authority]; yield 'found domain' => [new Domain($authority), $authority]; } - - /** @test */ - public function returnsEmptyWhenNoApiKeyIsProvided(): void - { - $getRepository = $this->em->getRepository(ApiKey::class); - - self::assertNull($this->resolver->resolveApiKey(null)); - $getRepository->shouldNotHaveBeenCalled(); - } - - /** - * @test - * @dataProvider provideFoundApiKeys - */ - public function triesToFindApiKeyWhenValueIsProvided(?ApiKey $foundApiKey, string $key): void - { - $repo = $this->prophesize(ObjectRepository::class); - $find = $repo->findOneBy(['key' => $key])->willReturn($foundApiKey); - $getRepository = $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); - - $result = $this->resolver->resolveApiKey($key); - - self::assertSame($result, $foundApiKey); - $find->shouldHaveBeenCalledOnce(); - $getRepository->shouldHaveBeenCalledOnce(); - } - - public function provideFoundApiKeys(): iterable - { - $key = 'abc123'; - - yield 'not found api key' => [null, $key]; - yield 'found api key' => [new ApiKey(), $key]; - } } diff --git a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php index e2d0822c..84d838b9 100644 --- a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php @@ -38,19 +38,4 @@ class SimpleShortUrlRelationResolverTest extends TestCase yield 'empty domain' => [null]; yield 'non-empty domain' => ['domain.com']; } - - /** - * @test - * @dataProvider provideKeys - */ - public function alwaysReturnsNullForApiKeys(?string $key): void - { - self::assertNull($this->resolver->resolveApiKey($key)); - } - - public function provideKeys(): iterable - { - yield 'empty api key' => [null]; - yield 'non-empty api key' => ['abc123']; - } } diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index b3db6460..28941579 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -28,7 +28,7 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction ]); } - $payload[ShortUrlMetaInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request)->toString(); + $payload[ShortUrlMetaInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request); $meta = ShortUrlMeta::fromRawData($payload); return new CreateShortUrlData($payload['longUrl'], (array) ($payload['tags'] ?? []), $meta); diff --git a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php index 996e59a6..cbb06386 100644 --- a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php @@ -50,7 +50,7 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction } return new CreateShortUrlData($longUrl, [], ShortUrlMeta::fromRawData([ - ShortUrlMetaInputFilter::API_KEY => $apiKeyResult->apiKey()->toString(), + ShortUrlMetaInputFilter::API_KEY => $apiKeyResult->apiKey(), ])); } } diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index 082b1783..80ccfc17 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -53,7 +53,7 @@ class CreateShortUrlActionTest extends TestCase { $apiKey = new ApiKey(); $shortUrl = new ShortUrl(''); - $expectedMeta['apiKey'] = $apiKey->toString(); + $expectedMeta['apiKey'] = $apiKey; $shorten = $this->urlShortener->shorten( Argument::type('string'), diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index eb1d6cd2..b42b95fb 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -78,12 +78,12 @@ class SingleStepCreateShortUrlActionTest extends TestCase ]); $findApiKey = $this->apiKeyService->check($key)->willReturn(new ApiKeyCheckResult($apiKey)); $generateShortCode = $this->urlShortener->shorten( - Argument::that(function (string $argument): string { + Argument::that(function (string $argument): bool { Assert::assertEquals('http://foobar.com', $argument); - return $argument; + return true; }), [], - ShortUrlMeta::fromRawData(['apiKey' => $key]), + ShortUrlMeta::fromRawData(['apiKey' => $apiKey]), )->willReturn(new ShortUrl('')); $resp = $this->action->handle($request);