From 4a1e7b761aab2830a0e88c73e0ad2a8682bf5f34 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 3 Jan 2021 17:48:32 +0100 Subject: [PATCH] Applied API role specs to short URL visits --- .../Adapter/ShortUrlRepositoryAdapter.php | 10 ++++-- .../Adapter/VisitsPaginatorAdapter.php | 8 ++++- .../src/Repository/ShortUrlRepository.php | 4 +-- .../ShortUrlRepositoryInterface.php | 2 +- .../Core/src/Repository/VisitRepository.php | 25 +++++++++----- .../Repository/VisitRepositoryInterface.php | 10 ++++-- module/Core/src/Service/VisitsTracker.php | 9 +++-- .../src/Service/VisitsTrackerInterface.php | 3 +- .../src/ShortUrl/Spec/BelongsToApiKey.php | 2 +- .../Adapter/VisitsPaginatorAdapterTest.php | 7 ++-- .../Core/test/Service/VisitsTrackerTest.php | 10 +++--- .../src/Action/Visit/ShortUrlVisitsAction.php | 5 ++- .../Action/Visit/ShortUrlVisitsActionTest.php | 34 ++++++++++++------- 13 files changed, 87 insertions(+), 42 deletions(-) diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index 8f339dc0..93fd88c7 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; +use Happyr\DoctrineSpecification\Specification\Specification; use Laminas\Paginator\Adapter\AdapterInterface; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; @@ -31,7 +32,7 @@ class ShortUrlRepositoryAdapter implements AdapterInterface $this->params->tags(), $this->params->orderBy(), $this->params->dateRange(), - $this->apiKey !== null ? $this->apiKey->spec() : null, + $this->resolveSpec(), ); } @@ -41,7 +42,12 @@ class ShortUrlRepositoryAdapter implements AdapterInterface $this->params->searchTerm(), $this->params->tags(), $this->params->dateRange(), - $this->apiKey !== null ? $this->apiKey->spec() : null, + $this->resolveSpec(), ); } + + private function resolveSpec(): ?Specification + { + return $this->apiKey !== null ? $this->apiKey->spec() : null; + } } diff --git a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php index 404ae309..29498a6d 100644 --- a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; +use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; @@ -13,15 +14,18 @@ class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter private VisitRepositoryInterface $visitRepository; private ShortUrlIdentifier $identifier; private VisitsParams $params; + private ?Specification $spec; public function __construct( VisitRepositoryInterface $visitRepository, ShortUrlIdentifier $identifier, - VisitsParams $params + VisitsParams $params, + ?Specification $spec ) { $this->visitRepository = $visitRepository; $this->params = $params; $this->identifier = $identifier; + $this->spec = $spec; } public function getItems($offset, $itemCountPerPage): array // phpcs:ignore @@ -32,6 +36,7 @@ class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter $this->params->getDateRange(), $itemCountPerPage, $offset, + $this->spec, ); } @@ -41,6 +46,7 @@ class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter $this->identifier->shortCode(), $this->identifier->domain(), $this->params->getDateRange(), + $this->spec, ); } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 4fa6fbd1..d4bb1d16 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -177,9 +177,9 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU return $qb->getQuery()->getOneOrNullResult(); } - public function shortCodeIsInUse(string $slug, ?string $domain = null): bool + public function shortCodeIsInUse(string $slug, ?string $domain = null, ?Specification $spec = null): bool { - $qb = $this->createFindOneQueryBuilder($slug, $domain, null); + $qb = $this->createFindOneQueryBuilder($slug, $domain, $spec); $qb->select('COUNT(DISTINCT s.id)'); return ((int) $qb->getQuery()->getSingleScalarResult()) > 0; diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index fee546fe..a0131f6f 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -36,7 +36,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat public function findOne(string $shortCode, ?string $domain = null, ?Specification $spec = null): ?ShortUrl; - public function shortCodeIsInUse(string $slug, ?string $domain): bool; + public function shortCodeIsInUse(string $slug, ?string $domain, ?Specification $spec = null): bool; public function findOneMatching(string $url, array $tags, ShortUrlMeta $meta): ?ShortUrl; diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 458b8ef2..13447372 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -4,9 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; -use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\QueryBuilder; +use Happyr\DoctrineSpecification\EntitySpecificationRepository; +use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; @@ -14,7 +15,7 @@ use Shlinkio\Shlink\Core\Entity\VisitLocation; use const PHP_INT_MAX; -class VisitRepository extends EntityRepository implements VisitRepositoryInterface +class VisitRepository extends EntitySpecificationRepository implements VisitRepositoryInterface { /** * @return iterable|Visit[] @@ -84,15 +85,20 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ?string $domain = null, ?DateRange $dateRange = null, ?int $limit = null, - ?int $offset = null + ?int $offset = null, + ?Specification $spec = null ): array { - $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange, $spec); return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); } - public function countVisitsByShortCode(string $shortCode, ?string $domain = null, ?DateRange $dateRange = null): int - { - $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + public function countVisitsByShortCode( + string $shortCode, + ?string $domain = null, + ?DateRange $dateRange = null, + ?Specification $spec = null + ): int { + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange, $spec); $qb->select('COUNT(v.id)'); return (int) $qb->getQuery()->getSingleScalarResult(); @@ -101,11 +107,12 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa private function createVisitsByShortCodeQueryBuilder( string $shortCode, ?string $domain, - ?DateRange $dateRange + ?DateRange $dateRange, + ?Specification $spec = null ): QueryBuilder { /** @var ShortUrlRepositoryInterface $shortUrlRepo */ $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOne($shortCode, $domain); + $shortUrl = $shortUrlRepo->findOne($shortCode, $domain, $spec); $shortUrlId = $shortUrl !== null ? $shortUrl->getId() : -1; // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index 5a540171..804023c8 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -5,10 +5,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Persistence\ObjectRepository; +use Happyr\DoctrineSpecification\EntitySpecificationRepositoryInterface; +use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; -interface VisitRepositoryInterface extends ObjectRepository +interface VisitRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { public const DEFAULT_BLOCK_SIZE = 10000; @@ -35,13 +37,15 @@ interface VisitRepositoryInterface extends ObjectRepository ?string $domain = null, ?DateRange $dateRange = null, ?int $limit = null, - ?int $offset = null + ?int $offset = null, + ?Specification $spec = null ): array; public function countVisitsByShortCode( string $shortCode, ?string $domain = null, - ?DateRange $dateRange = null + ?DateRange $dateRange = null, + ?Specification $spec = null ): int; /** diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index e777af76..e12ddbec 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -21,6 +21,7 @@ use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class VisitsTracker implements VisitsTrackerInterface { @@ -52,17 +53,19 @@ class VisitsTracker implements VisitsTrackerInterface * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ - public function info(ShortUrlIdentifier $identifier, VisitsParams $params): Paginator + public function info(ShortUrlIdentifier $identifier, VisitsParams $params, ?ApiKey $apiKey = null): Paginator { + $spec = $apiKey !== null ? $apiKey->spec() : null; + /** @var ShortUrlRepositoryInterface $repo */ $repo = $this->em->getRepository(ShortUrl::class); - if (! $repo->shortCodeIsInUse($identifier->shortCode(), $identifier->domain())) { + if (! $repo->shortCodeIsInUse($identifier->shortCode(), $identifier->domain(), $spec)) { throw ShortUrlNotFoundException::fromNotFound($identifier); } /** @var VisitRepositoryInterface $repo */ $repo = $this->em->getRepository(Visit::class); - $paginator = new Paginator(new VisitsPaginatorAdapter($repo, $identifier, $params)); + $paginator = new Paginator(new VisitsPaginatorAdapter($repo, $identifier, $params, $spec)); $paginator->setItemCountPerPage($params->getItemsPerPage()) ->setCurrentPageNumber($params->getPage()); diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index 2c2759c2..68e6c854 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; +use Shlinkio\Shlink\Rest\Entity\ApiKey; interface VisitsTrackerInterface { @@ -21,7 +22,7 @@ interface VisitsTrackerInterface * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ - public function info(ShortUrlIdentifier $identifier, VisitsParams $params): Paginator; + public function info(ShortUrlIdentifier $identifier, VisitsParams $params, ?ApiKey $apiKey = null): Paginator; /** * @return Visit[]|Paginator diff --git a/module/Core/src/ShortUrl/Spec/BelongsToApiKey.php b/module/Core/src/ShortUrl/Spec/BelongsToApiKey.php index a1059168..9e094b90 100644 --- a/module/Core/src/ShortUrl/Spec/BelongsToApiKey.php +++ b/module/Core/src/ShortUrl/Spec/BelongsToApiKey.php @@ -16,8 +16,8 @@ class BelongsToApiKey extends BaseSpecification public function __construct(ApiKey $apiKey, ?string $dqlAlias = null) { - $this->dqlAlias = $dqlAlias ?? 's'; $this->apiKey = $apiKey; + $this->dqlAlias = $dqlAlias ?? 's'; parent::__construct($this->dqlAlias); } diff --git a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php index 508a0984..ca0c5806 100644 --- a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -27,6 +27,7 @@ class VisitsPaginatorAdapterTest extends TestCase $this->repo->reveal(), new ShortUrlIdentifier(''), VisitsParams::fromRawData([]), + null, ); } @@ -36,7 +37,9 @@ class VisitsPaginatorAdapterTest extends TestCase $count = 3; $limit = 1; $offset = 5; - $findVisits = $this->repo->findVisitsByShortCode('', null, new DateRange(), $limit, $offset)->willReturn([]); + $findVisits = $this->repo->findVisitsByShortCode('', null, new DateRange(), $limit, $offset, null)->willReturn( + [], + ); for ($i = 0; $i < $count; $i++) { $this->adapter->getItems($offset, $limit); @@ -49,7 +52,7 @@ class VisitsPaginatorAdapterTest extends TestCase public function repoIsCalledOnlyOnceForCount(): void { $count = 3; - $countVisits = $this->repo->countVisitsByShortCode('', null, new DateRange())->willReturn(3); + $countVisits = $this->repo->countVisitsByShortCode('', null, new DateRange(), null)->willReturn(3); for ($i = 0; $i < $count; $i++) { $this->adapter->count(); diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 1d9096e3..b5509ae3 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -63,13 +63,15 @@ class VisitsTrackerTest extends TestCase { $shortCode = '123ABC'; $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $count = $repo->shortCodeIsInUse($shortCode, null)->willReturn(true); + $count = $repo->shortCodeIsInUse($shortCode, null, null)->willReturn(true); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $list = map(range(0, 1), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())); $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0)->willReturn($list); - $repo2->countVisitsByShortCode($shortCode, null, Argument::type(DateRange::class))->willReturn(1); + $repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0, null)->willReturn( + $list, + ); + $repo2->countVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), null)->willReturn(1); $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); $paginator = $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams()); @@ -83,7 +85,7 @@ class VisitsTrackerTest extends TestCase { $shortCode = '123ABC'; $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $count = $repo->shortCodeIsInUse($shortCode, null)->willReturn(false); + $count = $repo->shortCodeIsInUse($shortCode, null, null)->willReturn(false); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $this->expectException(ShortUrlNotFoundException::class); diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index 92a7e873..4a9a95e9 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; +use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class ShortUrlVisitsAction extends AbstractRestAction { @@ -30,7 +31,9 @@ class ShortUrlVisitsAction extends AbstractRestAction public function handle(Request $request): Response { $identifier = ShortUrlIdentifier::fromApiRequest($request); - $visits = $this->visitsTracker->info($identifier, VisitsParams::fromRawData($request->getQueryParams())); + $params = VisitsParams::fromRawData($request->getQueryParams()); + $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); + $visits = $this->visitsTracker->info($identifier, $params, $apiKey); return new JsonResponse([ 'visits' => $this->serializePaginator($visits), diff --git a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php index 25e71006..0bedbd37 100644 --- a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php @@ -5,18 +5,20 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\Visit; use Cake\Chronos\Chronos; -use Laminas\Diactoros\ServerRequest; +use Laminas\Diactoros\ServerRequestFactory; use Laminas\Paginator\Adapter\ArrayAdapter; use Laminas\Paginator\Paginator; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTracker; use Shlinkio\Shlink\Rest\Action\Visit\ShortUrlVisitsAction; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class ShortUrlVisitsActionTest extends TestCase { @@ -35,11 +37,14 @@ class ShortUrlVisitsActionTest extends TestCase public function providingCorrectShortCodeReturnsVisits(): void { $shortCode = 'abc123'; - $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), Argument::type(VisitsParams::class))->willReturn( - new Paginator(new ArrayAdapter([])), - )->shouldBeCalledOnce(); + $this->visitsTracker->info( + new ShortUrlIdentifier($shortCode), + Argument::type(VisitsParams::class), + Argument::type(ApiKey::class), + )->willReturn(new Paginator(new ArrayAdapter([]))) + ->shouldBeCalledOnce(); - $response = $this->action->handle((new ServerRequest())->withAttribute('shortCode', $shortCode)); + $response = $this->action->handle($this->requestWithApiKey()->withAttribute('shortCode', $shortCode)); self::assertEquals(200, $response->getStatusCode()); } @@ -51,18 +56,23 @@ class ShortUrlVisitsActionTest extends TestCase new DateRange(null, Chronos::parse('2016-01-01 00:00:00')), 3, 10, - )) + ), Argument::type(ApiKey::class)) ->willReturn(new Paginator(new ArrayAdapter([]))) ->shouldBeCalledOnce(); $response = $this->action->handle( - (new ServerRequest())->withAttribute('shortCode', $shortCode) - ->withQueryParams([ - 'endDate' => '2016-01-01 00:00:00', - 'page' => '3', - 'itemsPerPage' => '10', - ]), + $this->requestWithApiKey()->withAttribute('shortCode', $shortCode) + ->withQueryParams([ + 'endDate' => '2016-01-01 00:00:00', + 'page' => '3', + 'itemsPerPage' => '10', + ]), ); self::assertEquals(200, $response->getStatusCode()); } + + private function requestWithApiKey(): ServerRequestInterface + { + return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, new ApiKey()); + } }