diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index ee822bee..5c0c3c8a 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -40,11 +40,7 @@ class GenerateKeyCommandTest extends TestCase /** @test */ public function noExpirationDateIsDefinedIfNotProvided(): void { - $this->apiKeyService->create( - null, // Expiration date - null, // Name - )->shouldBeCalledOnce() - ->willReturn(new ApiKey()); + $this->apiKeyService->create(null, null)->shouldBeCalledOnce()->willReturn(ApiKey::create()); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); @@ -55,11 +51,9 @@ class GenerateKeyCommandTest extends TestCase /** @test */ public function expirationDateIsDefinedIfProvided(): void { - $this->apiKeyService->create( - Argument::type(Chronos::class), // Expiration date - null, // Name - )->shouldBeCalledOnce() - ->willReturn(new ApiKey()); + $this->apiKeyService->create(Argument::type(Chronos::class), null)->shouldBeCalledOnce()->willReturn( + ApiKey::create(), + ); $this->commandTester->execute([ '--expiration-date' => '2016-01-01', @@ -69,11 +63,9 @@ class GenerateKeyCommandTest extends TestCase /** @test */ public function nameIsDefinedIfProvided(): void { - $this->apiKeyService->create( - null, // Expiration date - Argument::type('string'), // Name - )->shouldBeCalledOnce() - ->willReturn(new ApiKey()); + $this->apiKeyService->create(null, Argument::type('string'))->shouldBeCalledOnce()->willReturn( + ApiKey::create(), + ); $this->commandTester->execute([ '--name' => 'Alice', diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php index 0e806e87..fa6816d2 100644 --- a/module/CLI/test/Command/Api/ListKeysCommandTest.php +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -9,6 +9,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Api\ListKeysCommand; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; @@ -49,66 +50,66 @@ class ListKeysCommandTest extends TestCase public function provideKeysAndOutputs(): iterable { yield 'all keys' => [ - [ApiKey::withKey('foo'), ApiKey::withKey('bar'), ApiKey::withKey('baz')], + [$apiKey1 = ApiKey::create(), $apiKey2 = ApiKey::create(), $apiKey3 = ApiKey::create()], false, << [ - [ApiKey::withKey('foo')->disable(), ApiKey::withKey('bar')], + [$apiKey1 = ApiKey::create()->disable(), $apiKey2 = ApiKey::create()], true, << [ [ - ApiKey::withKey('foo'), - $this->apiKeyWithRoles('bar', [RoleDefinition::forAuthoredShortUrls()]), - $this->apiKeyWithRoles('baz', [RoleDefinition::forDomain((new Domain('example.com'))->setId('1'))]), - ApiKey::withKey('foo2'), - $this->apiKeyWithRoles('baz2', [ + $apiKey1 = ApiKey::create(), + $apiKey2 = $this->apiKeyWithRoles([RoleDefinition::forAuthoredShortUrls()]), + $apiKey3 = $this->apiKeyWithRoles([RoleDefinition::forDomain((new Domain('example.com'))->setId('1'))]), + $apiKey4 = ApiKey::create(), + $apiKey5 = $this->apiKeyWithRoles([ RoleDefinition::forAuthoredShortUrls(), RoleDefinition::forDomain((new Domain('example.com'))->setId('1')), ]), - ApiKey::withKey('foo3'), + $apiKey6 = ApiKey::create(), ], true, << [ [ - $apiKey1 = ApiKey::withName('Alice'), - $apiKey2 = ApiKey::withName('Alice and Bob'), - $apiKey3 = ApiKey::withName(''), - $apiKey4 = new ApiKey(), + $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::withName('Alice')), + $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withName('Alice and Bob')), + $apiKey3 = ApiKey::fromMeta(ApiKeyMeta::withName('')), + $apiKey4 = ApiKey::create(), ], true, <<registerRole($role); } diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 00954049..62df2070 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -50,6 +50,7 @@ function parseDateRangeFromQuery(array $query, string $startDateName, string $en $startDate = parseDateFromQuery($query, $startDateName); $endDate = parseDateFromQuery($query, $endDateName); + // TODO Use match expression when migrating to PHP8 if ($startDate === null && $endDate === null) { return DateRange::emptyInstance(); } diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 49265eb0..aaa63d9f 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; @@ -53,9 +54,9 @@ class DomainRepositoryTest extends DatabaseTestCase /** @test */ public function findDomainsReturnsJustThoseMatchingProvidedApiKey(): void { - $authorApiKey = ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()); + $authorApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($authorApiKey); - $authorAndDomainApiKey = ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()); + $authorAndDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($authorAndDomainApiKey); $fooDomain = new Domain('foo.com'); @@ -74,10 +75,10 @@ class DomainRepositoryTest extends DatabaseTestCase $authorAndDomainApiKey->registerRole(RoleDefinition::forDomain($fooDomain)); - $fooDomainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($fooDomain)); + $fooDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($fooDomain))); $this->getEntityManager()->persist($fooDomainApiKey); - $barDomainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($barDomain)); + $barDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($barDomain))); $this->getEntityManager()->persist($fooDomainApiKey); $this->getEntityManager()->flush(); diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 48381857..35a6b709 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; @@ -335,13 +336,13 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $apiKey = ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()); + $apiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey); - $otherApiKey = ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()); + $otherApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($otherApiKey); - $wrongDomainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($wrongDomain)); + $wrongDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($wrongDomain))); $this->getEntityManager()->persist($wrongDomainApiKey); - $rightDomainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($rightDomain)); + $rightDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($rightDomain))); $this->getEntityManager()->persist($rightDomainApiKey); $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ diff --git a/module/Core/test-db/Repository/TagRepositoryTest.php b/module/Core/test-db/Repository/TagRepositoryTest.php index 34a06a40..eea2ed8c 100644 --- a/module/Core/test-db/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Repository/TagRepositoryTest.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; @@ -100,9 +101,9 @@ class TagRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($domain); $this->getEntityManager()->flush(); - $authorApiKey = ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()); + $authorApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($authorApiKey); - $domainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($domain)); + $domainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($domain))); $this->getEntityManager()->persist($domainApiKey); $names = ['foo', 'bar', 'baz', 'another']; diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index b6c23699..4e634493 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\IpGeolocation\Model\Location; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; @@ -176,7 +177,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $apiKey1 = ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()); + $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey1); $shortUrl = ShortUrl::fromMeta( ShortUrlMeta::fromRawData(['apiKey' => $apiKey1, 'domain' => $domain->getAuthority(), 'longUrl' => '']), @@ -185,7 +186,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 4); - $apiKey2 = ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()); + $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey2); $shortUrl2 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['apiKey' => $apiKey2, 'longUrl' => ''])); $this->getEntityManager()->persist($shortUrl2); @@ -198,7 +199,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($shortUrl3); $this->createVisitsForShortUrl($shortUrl3, 7); - $domainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($domain)); + $domainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($domain))); $this->getEntityManager()->persist($domainApiKey); // Visits not linked to any short URL diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index 46e39c5a..0306f387 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -50,8 +51,10 @@ class DomainServiceTest extends TestCase public function provideExcludedDomains(): iterable { $default = new DomainItem('default.com', true); - $adminApiKey = new ApiKey(); - $domainSpecificApiKey = ApiKey::withRoles(RoleDefinition::forDomain((new Domain(''))->setId('123'))); + $adminApiKey = ApiKey::create(); + $domainSpecificApiKey = ApiKey::fromMeta( + ApiKeyMeta::withRoles(RoleDefinition::forDomain((new Domain(''))->setId('123'))), + ); yield 'empty list without API key' => [[], [$default], null]; yield 'one item without API key' => [ diff --git a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index 93aba122..5420e4b6 100644 --- a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -66,7 +66,7 @@ class ShortUrlRepositoryAdapterTest extends TestCase 'startDate' => $startDate, 'endDate' => $endDate, ]); - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $params, $apiKey); $dateRange = $params->dateRange(); diff --git a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index 8dc88495..6c95a60f 100644 --- a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -44,7 +44,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase public function repoIsCalledOnlyOnceForCount(): void { $count = 3; - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByTag('foo', new DateRange(), $apiKey->spec())->willReturn(3); diff --git a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php index 436b4b7d..955e5ac5 100644 --- a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -47,7 +47,7 @@ class VisitsPaginatorAdapterTest extends TestCase public function repoIsCalledOnlyOnceForCount(): void { $count = 3; - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByShortCode('', null, new DateRange(), $apiKey->spec())->willReturn(3); diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 024957b0..67420edc 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -124,7 +124,7 @@ class ShortUrlServiceTest extends TestCase 'maxVisits' => 10, 'longUrl' => 'modifiedLongUrl', ], - ), new ApiKey()]; + ), ApiKey::create()]; yield 'long URL with validation' => [1, ShortUrlEdit::fromRawData( [ 'longUrl' => 'modifiedLongUrl', diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index 9b484791..33ae7be0 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; use Shlinkio\Shlink\Core\Tag\TagService; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use ShlinkioTest\Shlink\Core\Util\ApiKeyHelpersTrait; @@ -90,7 +91,10 @@ class TagServiceTest extends TestCase $this->expectExceptionMessage('You are not allowed to delete tags'); $delete->shouldNotBeCalled(); - $this->service->deleteTags(['foo', 'bar'], ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls())); + $this->service->deleteTags( + ['foo', 'bar'], + ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())), + ); } /** @test */ @@ -178,7 +182,7 @@ class TagServiceTest extends TestCase $this->service->renameTag( TagRenaming::fromNames('foo', 'bar'), - ApiKey::withRoles(RoleDefinition::forAuthoredShortUrls()), + ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())), ); } } diff --git a/module/Core/test/Util/ApiKeyHelpersTrait.php b/module/Core/test/Util/ApiKeyHelpersTrait.php index 0b21ed5f..6624c8dd 100644 --- a/module/Core/test/Util/ApiKeyHelpersTrait.php +++ b/module/Core/test/Util/ApiKeyHelpersTrait.php @@ -11,6 +11,6 @@ trait ApiKeyHelpersTrait public function provideAdminApiKeys(): iterable { yield 'no API key' => [null]; - yield 'admin API key' => [new ApiKey()]; + yield 'admin API key' => [ApiKey::create()]; } } diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index de2a3534..8e90a447 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -113,7 +113,7 @@ class VisitsStatsHelperTest extends TestCase public function throwsExceptionWhenRequestingVisitsForInvalidTag(): void { $tag = 'foo'; - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $repo = $this->prophesize(TagRepository::class); $tagExists = $repo->tagExists($tag, $apiKey)->willReturn(false); $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); diff --git a/module/Rest/src/ApiKey/Model/ApiKeyMeta.php b/module/Rest/src/ApiKey/Model/ApiKeyMeta.php index b79b9adf..aa3c117a 100644 --- a/module/Rest/src/ApiKey/Model/ApiKeyMeta.php +++ b/module/Rest/src/ApiKey/Model/ApiKeyMeta.php @@ -4,15 +4,57 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\ApiKey\Model; +use Cake\Chronos\Chronos; + final class ApiKeyMeta { - public static function withKey(string $key): self - { + private ?string $name = null; + private ?Chronos $expirationDate = null; + /** @var RoleDefinition[] */ + private array $roleDefinitions; + private function __construct(?string $name, ?Chronos $expirationDate, array $roleDefinitions) + { + $this->name = $name; + $this->expirationDate = $expirationDate; + $this->roleDefinitions = $roleDefinitions; } - public static function withName(string $key): self + public static function withName(string $name): self { + return new self($name, null, []); + } + public static function withExpirationDate(Chronos $expirationDate): self + { + return new self(null, $expirationDate, []); + } + + public static function withNameAndExpirationDate(string $name, Chronos $expirationDate): self + { + return new self($name, $expirationDate, []); + } + + public static function withRoles(RoleDefinition ...$roleDefinitions): self + { + return new self(null, null, $roleDefinitions); + } + + public function name(): ?string + { + return $this->name; + } + + public function expirationDate(): ?Chronos + { + return $this->expirationDate; + } + + /** + * @return RoleDefinition[] + */ + public function roleDefinitions(): array + { + return $this->roleDefinitions; } } diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index e15c59f8..0317390e 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -12,6 +12,7 @@ use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\Specification; use Ramsey\Uuid\Uuid; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\ApiKey\Role; @@ -27,7 +28,7 @@ class ApiKey extends AbstractEntity /** * @throws Exception */ - public function __construct(?Chronos $expirationDate = null, ?string $name = null) + private function __construct(?string $name = null, ?Chronos $expirationDate = null) { $this->key = Uuid::uuid4()->toString(); $this->expirationDate = $expirationDate; @@ -36,30 +37,21 @@ class ApiKey extends AbstractEntity $this->roles = new ArrayCollection(); } - public static function withRoles(RoleDefinition ...$roleDefinitions): self + public static function create(): ApiKey { - $apiKey = new self(); + return new self(); + } - foreach ($roleDefinitions as $roleDefinition) { + public static function fromMeta(ApiKeyMeta $meta): self + { + $apiKey = new self($meta->name(), $meta->expirationDate()); + foreach ($meta->roleDefinitions() as $roleDefinition) { $apiKey->registerRole($roleDefinition); } return $apiKey; } - public static function withKey(string $key, ?Chronos $expirationDate = null, ?string $name = null): self - { - $apiKey = new self($expirationDate, $name); - $apiKey->key = $key; - - return $apiKey; - } - - public static function withName(string $name): self - { - return new self(null, $name); - } - public function getExpirationDate(): ?Chronos { return $this->expirationDate; diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index ef60ee08..e81c446f 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Rest\Service; use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -26,7 +27,7 @@ class ApiKeyService implements ApiKeyServiceInterface ?string $name = null, RoleDefinition ...$roleDefinitions ): ApiKey { - $key = new ApiKey($expirationDate, $name); + $key = $this->buildApiKeyWithParams($expirationDate, $name); foreach ($roleDefinitions as $definition) { $key->registerRole($definition); } @@ -37,6 +38,24 @@ class ApiKeyService implements ApiKeyServiceInterface return $key; } + private function buildApiKeyWithParams(?Chronos $expirationDate, ?string $name): ApiKey + { + // TODO Use match expression when migrating to PHP8 + if ($expirationDate === null && $name === null) { + return ApiKey::create(); + } + + if ($expirationDate !== null && $name !== null) { + return ApiKey::fromMeta(ApiKeyMeta::withNameAndExpirationDate($name, $expirationDate)); + } + + if ($name === null) { + return ApiKey::fromMeta(ApiKeyMeta::withExpirationDate($expirationDate)); + } + + return ApiKey::fromMeta(ApiKeyMeta::withName($name)); + } + public function check(string $key): ApiKeyCheckResult { $apiKey = $this->getByKey($key); diff --git a/module/Rest/test-api/Fixtures/ApiKeyFixture.php b/module/Rest/test-api/Fixtures/ApiKeyFixture.php index c6383968..ef6d1781 100644 --- a/module/Rest/test-api/Fixtures/ApiKeyFixture.php +++ b/module/Rest/test-api/Fixtures/ApiKeyFixture.php @@ -8,7 +8,9 @@ use Cake\Chronos\Chronos; use Doctrine\Common\DataFixtures\AbstractFixture; use Doctrine\Common\DataFixtures\DependentFixtureInterface; use Doctrine\Persistence\ObjectManager; +use ReflectionObject; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -41,7 +43,11 @@ class ApiKeyFixture extends AbstractFixture implements DependentFixtureInterface private function buildApiKey(string $key, bool $enabled, ?Chronos $expiresAt = null): ApiKey { - $apiKey = ApiKey::withKey($key, $expiresAt); + $apiKey = $expiresAt !== null ? ApiKey::fromMeta(ApiKeyMeta::withExpirationDate($expiresAt)) : ApiKey::create(); + $ref = new ReflectionObject($apiKey); + $keyProp = $ref->getProperty('key'); + $keyProp->setAccessible(true); + $keyProp->setValue($apiKey, $key); if (! $enabled) { $apiKey->disable(); diff --git a/module/Rest/test/Action/Domain/ListDomainsActionTest.php b/module/Rest/test/Action/Domain/ListDomainsActionTest.php index d6dcc4a3..cbe43895 100644 --- a/module/Rest/test/Action/Domain/ListDomainsActionTest.php +++ b/module/Rest/test/Action/Domain/ListDomainsActionTest.php @@ -30,7 +30,7 @@ class ListDomainsActionTest extends TestCase /** @test */ public function domainsAreProperlyListed(): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $domains = [ new DomainItem('bar.com', true), new DomainItem('baz.com', false), diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index f8e95659..ffcd6c62 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -40,7 +40,7 @@ class CreateShortUrlActionTest extends TestCase /** @test */ public function properShortcodeConversionReturnsData(): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $shortUrl = ShortUrl::createEmpty(); $expectedMeta = $body = [ 'longUrl' => 'http://www.domain.com/foo/bar', @@ -80,7 +80,7 @@ class CreateShortUrlActionTest extends TestCase $request = (new ServerRequest())->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', 'domain' => $domain, - ])->withAttribute(ApiKey::class, new ApiKey()); + ])->withAttribute(ApiKey::class, ApiKey::create()); $this->expectException(ValidationException::class); $urlToShortCode->shouldNotBeCalled(); diff --git a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php index 9be06756..9705cd59 100644 --- a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php @@ -29,7 +29,7 @@ class DeleteShortUrlActionTest extends TestCase /** @test */ public function emptyResponseIsReturnedIfProperlyDeleted(): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $deleteByShortCode = $this->service->deleteByShortCode(Argument::any(), false, $apiKey)->will( function (): void { }, diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php index eee75dbf..e1f434df 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php @@ -48,7 +48,7 @@ class EditShortUrlActionTest extends TestCase public function correctShortCodeReturnsSuccess(): void { $request = (new ServerRequest())->withAttribute('shortCode', 'abc123') - ->withAttribute(ApiKey::class, new ApiKey()) + ->withAttribute(ApiKey::class, ApiKey::create()) ->withParsedBody([ 'maxVisits' => 5, ]); diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php index a345046a..59c55d84 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php @@ -58,6 +58,6 @@ class EditShortUrlTagsActionTest extends TestCase private function createRequestWithAPiKey(): ServerRequestInterface { - return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, new ApiKey()); + return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, ApiKey::create()); } } diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 2683b514..712d605d 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -51,7 +51,7 @@ class ListShortUrlsActionTest extends TestCase ?string $startDate = null, ?string $endDate = null ): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $request = (new ServerRequest())->withQueryParams($query)->withAttribute(ApiKey::class, $apiKey); $listShortUrls = $this->service->listShortUrls(ShortUrlsParams::fromRawData([ 'page' => $expectedPage, diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index 748ab642..6f8ddbb9 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -37,7 +37,7 @@ class ResolveShortUrlActionTest extends TestCase public function correctShortCodeReturnsSuccess(): void { $shortCode = 'abc123'; - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey)->willReturn( ShortUrl::withLongUrl('http://domain.com/foo/bar'), )->shouldBeCalledOnce(); diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index f78a9de5..8bb1482a 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -39,7 +39,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase /** @test */ public function properDataIsPassedWhenGeneratingShortCode(): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $request = (new ServerRequest())->withQueryParams([ 'longUrl' => 'http://foobar.com', diff --git a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php b/module/Rest/test/Action/Tag/DeleteTagsActionTest.php index 957c01a5..4812649d 100644 --- a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php +++ b/module/Rest/test/Action/Tag/DeleteTagsActionTest.php @@ -34,7 +34,7 @@ class DeleteTagsActionTest extends TestCase { $request = (new ServerRequest()) ->withQueryParams(['tags' => $tags]) - ->withAttribute(ApiKey::class, new ApiKey()); + ->withAttribute(ApiKey::class, ApiKey::create()); $deleteTags = $this->tagService->deleteTags($tags ?: [], Argument::type(ApiKey::class)); $response = $this->action->handle($request); diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index 9bdad15b..8b7378fd 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -83,6 +83,6 @@ class ListTagsActionTest extends TestCase private function requestWithApiKey(): ServerRequestInterface { - return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, new ApiKey()); + return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, ApiKey::create()); } } diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index 681e68f6..d7b398db 100644 --- a/module/Rest/test/Action/Tag/UpdateTagActionTest.php +++ b/module/Rest/test/Action/Tag/UpdateTagActionTest.php @@ -70,6 +70,6 @@ class UpdateTagActionTest extends TestCase private function requestWithApiKey(): ServerRequestInterface { - return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, new ApiKey()); + return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, ApiKey::create()); } } diff --git a/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php b/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php index d53cb20d..829b820b 100644 --- a/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/GlobalVisitsActionTest.php @@ -30,7 +30,7 @@ class GlobalVisitsActionTest extends TestCase /** @test */ public function statsAreReturnedFromHelper(): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $stats = new VisitsStats(5, 3); $getStats = $this->helper->getVisitsStats($apiKey)->willReturn($stats); diff --git a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php index 6b149877..d0c67e7c 100644 --- a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php @@ -73,6 +73,6 @@ class ShortUrlVisitsActionTest extends TestCase private function requestWithApiKey(): ServerRequestInterface { - return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, new ApiKey()); + return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, ApiKey::create()); } } diff --git a/module/Rest/test/Action/Visit/TagVisitsActionTest.php b/module/Rest/test/Action/Visit/TagVisitsActionTest.php index da046f26..be3ce914 100644 --- a/module/Rest/test/Action/Visit/TagVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/TagVisitsActionTest.php @@ -33,7 +33,7 @@ class TagVisitsActionTest extends TestCase public function providingCorrectShortCodeReturnsVisits(): void { $tag = 'foo'; - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $getVisits = $this->visitsHelper->visitsForTag($tag, Argument::type(VisitsParams::class), $apiKey)->willReturn( new Paginator(new ArrayAdapter([])), ); diff --git a/module/Rest/test/ApiKey/RoleTest.php b/module/Rest/test/ApiKey/RoleTest.php index 60a55ca5..278d37ff 100644 --- a/module/Rest/test/ApiKey/RoleTest.php +++ b/module/Rest/test/ApiKey/RoleTest.php @@ -28,7 +28,7 @@ class RoleTest extends TestCase public function provideRoles(): iterable { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); yield 'inline invalid role' => [new ApiKeyRole('invalid', [], $apiKey), true, Spec::andX()]; yield 'not inline invalid role' => [new ApiKeyRole('invalid', [], $apiKey), false, Spec::andX()]; diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index 2edbe5e6..68503b58 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -138,7 +138,7 @@ class AuthenticationMiddlewareTest extends TestCase /** @test */ public function validApiKeyFallsBackToNextMiddleware(): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $key = $apiKey->toString(); $request = ServerRequestFactory::fromGlobals() ->withAttribute( diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index af50be23..addebbcd 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -13,6 +13,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyService; @@ -82,14 +83,14 @@ class ApiKeyServiceTest extends TestCase public function provideInvalidApiKeys(): iterable { yield 'non-existent api key' => [null]; - yield 'disabled api key' => [(new ApiKey())->disable()]; - yield 'expired api key' => [new ApiKey(Chronos::now()->subDay())]; + yield 'disabled api key' => [ApiKey::create()->disable()]; + yield 'expired api key' => [ApiKey::fromMeta(ApiKeyMeta::withExpirationDate(Chronos::now()->subDay()))]; } /** @test */ public function checkReturnsTrueWhenConditionsAreFavorable(): void { - $apiKey = new ApiKey(); + $apiKey = ApiKey::create(); $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn($apiKey) @@ -118,7 +119,7 @@ class ApiKeyServiceTest extends TestCase /** @test */ public function disableReturnsDisabledApiKeyWhenFound(): void { - $key = new ApiKey(); + $key = ApiKey::create(); $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn($key) ->shouldBeCalledOnce(); @@ -135,7 +136,7 @@ class ApiKeyServiceTest extends TestCase /** @test */ public function listFindsAllApiKeys(): void { - $expectedApiKeys = [new ApiKey(), new ApiKey(), new ApiKey()]; + $expectedApiKeys = [ApiKey::create(), ApiKey::create(), ApiKey::create()]; $repo = $this->prophesize(EntityRepository::class); $repo->findBy([])->willReturn($expectedApiKeys) @@ -150,7 +151,7 @@ class ApiKeyServiceTest extends TestCase /** @test */ public function listEnabledFindsOnlyEnabledApiKeys(): void { - $expectedApiKeys = [new ApiKey(), new ApiKey(), new ApiKey()]; + $expectedApiKeys = [ApiKey::create(), ApiKey::create(), ApiKey::create()]; $repo = $this->prophesize(EntityRepository::class); $repo->findBy(['enabled' => true])->willReturn($expectedApiKeys)