diff --git a/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php index e80fbcdd..3b73509a 100644 --- a/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php @@ -4,20 +4,28 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; +use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class VisitsForTagPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { private VisitRepositoryInterface $visitRepository; private string $tag; private VisitsParams $params; + private ?ApiKey $apiKey; - public function __construct(VisitRepositoryInterface $visitRepository, string $tag, VisitsParams $params) - { + public function __construct( + VisitRepositoryInterface $visitRepository, + string $tag, + VisitsParams $params, + ?ApiKey $apiKey + ) { $this->visitRepository = $visitRepository; $this->params = $params; $this->tag = $tag; + $this->apiKey = $apiKey; } public function getItems($offset, $itemCountPerPage): array // phpcs:ignore @@ -27,11 +35,21 @@ class VisitsForTagPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte $this->params->getDateRange(), $itemCountPerPage, $offset, + $this->resolveSpec(), ); } protected function doCount(): int { - return $this->visitRepository->countVisitsByTag($this->tag, $this->params->getDateRange()); + return $this->visitRepository->countVisitsByTag( + $this->tag, + $this->params->getDateRange(), + $this->resolveSpec(), + ); + } + + private function resolveSpec(): ?Specification + { + return $this->apiKey !== null ? $this->apiKey->spec(true) : null; } } diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index a2cb5bfd..dd15c292 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -5,9 +5,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; use Happyr\DoctrineSpecification\EntitySpecificationRepository; +use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\Spec\CountTagsWithName; +use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use function Functional\map; @@ -47,4 +51,14 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito fn (array $row) => new TagInfo($row['tag'], (int) $row['shortUrlsCount'], (int) $row['visitsCount']), ); } + + public function tagExists(string $tag, ?ApiKey $apiKey = null): bool + { + $result = (int) $this->matchSingleScalarResult(Spec::andX( + new CountTagsWithName($tag), + new WithApiKeySpecsEnsuringJoin($apiKey), + )); + + return $result > 0; + } } diff --git a/module/Core/src/Repository/TagRepositoryInterface.php b/module/Core/src/Repository/TagRepositoryInterface.php index a486ef55..86898ed1 100644 --- a/module/Core/src/Repository/TagRepositoryInterface.php +++ b/module/Core/src/Repository/TagRepositoryInterface.php @@ -8,6 +8,7 @@ use Doctrine\Persistence\ObjectRepository; use Happyr\DoctrineSpecification\EntitySpecificationRepositoryInterface; use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Rest\Entity\ApiKey; interface TagRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { @@ -17,4 +18,6 @@ interface TagRepositoryInterface extends ObjectRepository, EntitySpecificationRe * @return TagInfo[] */ public function findTagsWithInfo(?Specification $spec = null): array; + + public function tagExists(string $tag, ?ApiKey $apiKey = null): bool; } diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 13447372..8e750bc3 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -131,32 +131,36 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo string $tag, ?DateRange $dateRange = null, ?int $limit = null, - ?int $offset = null + ?int $offset = null, + ?Specification $spec = null ): array { - $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange); + $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange, $spec); return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); } - public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int + public function countVisitsByTag(string $tag, ?DateRange $dateRange = null, ?Specification $spec = null): int { - $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange); + $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange, $spec); $qb->select('COUNT(v.id)'); return (int) $qb->getQuery()->getSingleScalarResult(); } - private function createVisitsByTagQueryBuilder(string $tag, ?DateRange $dateRange = null): QueryBuilder - { - // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later + private function createVisitsByTagQueryBuilder( + string $tag, + ?DateRange $dateRange, + ?Specification $spec + ): QueryBuilder { + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later // Since they are not strictly provided by the caller, it's reasonably safe $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v') ->join('v.shortUrl', 's') ->join('s.tags', 't') - ->where($qb->expr()->eq('t.name', '\'' . $tag . '\'')); + ->where($qb->expr()->eq('t.name', '\'' . $tag . '\'')); // This needs to be concatenated, not bound - // Apply date range filtering $this->applyDatesInline($qb, $dateRange); + $this->applySpecification($qb, $spec, 'v'); return $qb; } diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index 804023c8..71a6c4ae 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -55,8 +55,9 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification string $tag, ?DateRange $dateRange = null, ?int $limit = null, - ?int $offset = null + ?int $offset = null, + ?Specification $spec = null ): array; - public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int; + public function countVisitsByTag(string $tag, ?DateRange $dateRange = null, ?Specification $spec = null): int; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index e12ddbec..fc35499f 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -76,18 +76,17 @@ class VisitsTracker implements VisitsTrackerInterface * @return Visit[]|Paginator * @throws TagNotFoundException */ - public function visitsForTag(string $tag, VisitsParams $params): Paginator + public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator { /** @var TagRepository $tagRepo */ $tagRepo = $this->em->getRepository(Tag::class); - $count = $tagRepo->count(['name' => $tag]); - if ($count === 0) { + if (! $tagRepo->tagExists($tag, $apiKey)) { throw TagNotFoundException::fromTag($tag); } /** @var VisitRepositoryInterface $repo */ $repo = $this->em->getRepository(Visit::class); - $paginator = new Paginator(new VisitsForTagPaginatorAdapter($repo, $tag, $params)); + $paginator = new Paginator(new VisitsForTagPaginatorAdapter($repo, $tag, $params, $apiKey)); $paginator->setItemCountPerPage($params->getItemsPerPage()) ->setCurrentPageNumber($params->getPage()); diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index 68e6c854..ecffae23 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -28,5 +28,5 @@ interface VisitsTrackerInterface * @return Visit[]|Paginator * @throws TagNotFoundException */ - public function visitsForTag(string $tag, VisitsParams $params): Paginator; + public function visitsForTag(string $tag, VisitsParams $params, ?ApiKey $apiKey = null): Paginator; } diff --git a/module/Core/src/ShortUrl/Spec/BelongsToApiKeyInlined.php b/module/Core/src/ShortUrl/Spec/BelongsToApiKeyInlined.php new file mode 100644 index 00000000..197031f3 --- /dev/null +++ b/module/Core/src/ShortUrl/Spec/BelongsToApiKeyInlined.php @@ -0,0 +1,29 @@ +apiKey = $apiKey; + } + + public function getFilter(QueryBuilder $qb, string $dqlAlias): string + { + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later + return (string) $qb->expr()->eq('s.authorApiKey', '\'' . $this->apiKey->getId() . '\''); + } + + public function modify(QueryBuilder $qb, string $dqlAlias): void + { + } +} diff --git a/module/Core/src/ShortUrl/Spec/BelongsToDomainInlined.php b/module/Core/src/ShortUrl/Spec/BelongsToDomainInlined.php new file mode 100644 index 00000000..edadf760 --- /dev/null +++ b/module/Core/src/ShortUrl/Spec/BelongsToDomainInlined.php @@ -0,0 +1,28 @@ +domainId = $domainId; + } + + public function getFilter(QueryBuilder $qb, string $dqlAlias): string + { + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later + return (string) $qb->expr()->eq('s.domain', '\'' . $this->domainId . '\''); + } + + public function modify(QueryBuilder $qb, string $dqlAlias): void + { + } +} diff --git a/module/Core/src/Tag/Spec/CountTagsWithName.php b/module/Core/src/Tag/Spec/CountTagsWithName.php new file mode 100644 index 00000000..a3f90a78 --- /dev/null +++ b/module/Core/src/Tag/Spec/CountTagsWithName.php @@ -0,0 +1,30 @@ +tagName = $tagName; + } + + protected function getSpec(): Specification + { + return Spec::countOf( + Spec::andX( + Spec::select('id'), + Spec::eq('name', $this->tagName), + ), + ); + } +} diff --git a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index b3a47749..8d577b91 100644 --- a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -22,7 +22,12 @@ class VisitsForTagPaginatorAdapterTest extends TestCase protected function setUp(): void { $this->repo = $this->prophesize(VisitRepositoryInterface::class); - $this->adapter = new VisitsForTagPaginatorAdapter($this->repo->reveal(), 'foo', VisitsParams::fromRawData([])); + $this->adapter = new VisitsForTagPaginatorAdapter( + $this->repo->reveal(), + 'foo', + VisitsParams::fromRawData([]), + null, + ); } /** @test */ @@ -31,7 +36,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $count = 3; $limit = 1; $offset = 5; - $findVisits = $this->repo->findVisitsByTag('foo', new DateRange(), $limit, $offset)->willReturn([]); + $findVisits = $this->repo->findVisitsByTag('foo', new DateRange(), $limit, $offset, null)->willReturn([]); for ($i = 0; $i < $count; $i++) { $this->adapter->getItems($offset, $limit); @@ -44,7 +49,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase public function repoIsCalledOnlyOnceForCount(): void { $count = 3; - $countVisits = $this->repo->countVisitsByTag('foo', new DateRange())->willReturn(3); + $countVisits = $this->repo->countVisitsByTag('foo', 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 b5509ae3..56478966 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -25,6 +25,7 @@ use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Shlinkio\Shlink\Core\Service\VisitsTracker; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use function Functional\map; use function range; @@ -98,15 +99,16 @@ class VisitsTrackerTest extends TestCase public function throwsExceptionWhenRequestingVisitsForInvalidTag(): void { $tag = 'foo'; + $apiKey = new ApiKey(); $repo = $this->prophesize(TagRepository::class); - $count = $repo->count(['name' => $tag])->willReturn(0); + $tagExists = $repo->tagExists($tag, $apiKey)->willReturn(false); $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); $this->expectException(TagNotFoundException::class); - $count->shouldBeCalledOnce(); + $tagExists->shouldBeCalledOnce(); $getRepo->shouldBeCalledOnce(); - $this->visitsTracker->visitsForTag($tag, new VisitsParams()); + $this->visitsTracker->visitsForTag($tag, new VisitsParams(), $apiKey); } /** @test */ @@ -114,19 +116,19 @@ class VisitsTrackerTest extends TestCase { $tag = 'foo'; $repo = $this->prophesize(TagRepository::class); - $count = $repo->count(['name' => $tag])->willReturn(1); + $tagExists = $repo->tagExists($tag, null)->willReturn(true); $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); $list = map(range(0, 1), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())); $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0)->willReturn($list); - $repo2->countVisitsByTag($tag, Argument::type(DateRange::class))->willReturn(1); + $repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0, null)->willReturn($list); + $repo2->countVisitsByTag($tag, Argument::type(DateRange::class), null)->willReturn(1); $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); $paginator = $this->visitsTracker->visitsForTag($tag, new VisitsParams()); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentItems())); - $count->shouldHaveBeenCalledOnce(); + $tagExists->shouldHaveBeenCalledOnce(); $getRepo->shouldHaveBeenCalledOnce(); } } diff --git a/module/Rest/src/Action/Visit/TagVisitsAction.php b/module/Rest/src/Action/Visit/TagVisitsAction.php index 1107ca5c..c83ee95c 100644 --- a/module/Rest/src/Action/Visit/TagVisitsAction.php +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; 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 TagVisitsAction extends AbstractRestAction { @@ -29,7 +30,9 @@ class TagVisitsAction extends AbstractRestAction public function handle(Request $request): Response { $tag = $request->getAttribute('tag', ''); - $visits = $this->visitsTracker->visitsForTag($tag, VisitsParams::fromRawData($request->getQueryParams())); + $params = VisitsParams::fromRawData($request->getQueryParams()); + $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); + $visits = $this->visitsTracker->visitsForTag($tag, $params, $apiKey); return new JsonResponse([ 'visits' => $this->serializePaginator($visits), diff --git a/module/Rest/src/ApiKey/Role.php b/module/Rest/src/ApiKey/Role.php index 1140f1ff..83a78087 100644 --- a/module/Rest/src/ApiKey/Role.php +++ b/module/Rest/src/ApiKey/Role.php @@ -7,7 +7,9 @@ namespace Shlinkio\Shlink\Rest\ApiKey; use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToApiKey; +use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToApiKeyInlined; use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToDomain; +use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToDomainInlined; use Shlinkio\Shlink\Rest\Entity\ApiKeyRole; class Role @@ -15,14 +17,15 @@ class Role public const AUTHORED_SHORT_URLS = 'AUTHORED_SHORT_URLS'; public const DOMAIN_SPECIFIC = 'DOMAIN_SPECIFIC'; - public static function toSpec(ApiKeyRole $role): Specification + public static function toSpec(ApiKeyRole $role, bool $inlined): Specification { if ($role->name() === self::AUTHORED_SHORT_URLS) { - return new BelongsToApiKey($role->apiKey()); + return $inlined ? new BelongsToApiKeyInlined($role->apiKey()) : new BelongsToApiKey($role->apiKey()); } if ($role->name() === self::DOMAIN_SPECIFIC) { - return new BelongsToDomain($role->meta()['domain_id'] ?? -1); + $domainId = $role->meta()['domain_id'] ?? -1; + return $inlined ? new BelongsToDomainInlined($domainId) : new BelongsToDomain($domainId); } return Spec::andX(); diff --git a/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php b/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php new file mode 100644 index 00000000..6bf8034d --- /dev/null +++ b/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php @@ -0,0 +1,29 @@ +apiKey = $apiKey; + } + + protected function getSpec(): Specification + { + return $this->apiKey === null || $this->apiKey->isAdmin() ? Spec::andX() : Spec::andX( + Spec::join('shortUrls', 's'), + $this->apiKey->spec(), + ); + } +} diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index 6c122494..81c22b25 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -68,9 +68,14 @@ class ApiKey extends AbstractEntity return $this->key; } - public function spec(): Specification + public function spec(bool $inlined = false): Specification { - $specs = $this->roles->map(fn (ApiKeyRole $role) => Role::toSpec($role)); + $specs = $this->roles->map(fn (ApiKeyRole $role) => Role::toSpec($role, $inlined)); return Spec::andX(...$specs); } + + public function isAdmin(): bool + { + return $this->roles->count() === 0; + } } diff --git a/module/Rest/test/Action/Visit/TagVisitsActionTest.php b/module/Rest/test/Action/Visit/TagVisitsActionTest.php index 53dbf8f2..a7598971 100644 --- a/module/Rest/test/Action/Visit/TagVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/TagVisitsActionTest.php @@ -14,6 +14,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTracker; use Shlinkio\Shlink\Rest\Action\Visit\TagVisitsAction; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class TagVisitsActionTest extends TestCase { @@ -32,11 +33,14 @@ class TagVisitsActionTest extends TestCase public function providingCorrectShortCodeReturnsVisits(): void { $tag = 'foo'; - $getVisits = $this->visitsTracker->visitsForTag($tag, Argument::type(VisitsParams::class))->willReturn( + $apiKey = new ApiKey(); + $getVisits = $this->visitsTracker->visitsForTag($tag, Argument::type(VisitsParams::class), $apiKey)->willReturn( new Paginator(new ArrayAdapter([])), ); - $response = $this->action->handle((new ServerRequest())->withAttribute('tag', $tag)); + $response = $this->action->handle( + (new ServerRequest())->withAttribute('tag', $tag)->withAttribute(ApiKey::class, $apiKey), + ); self::assertEquals(200, $response->getStatusCode()); $getVisits->shouldHaveBeenCalledOnce();