Applied API role specs to tag visits

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;
     }
 }

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;
+    }
 }

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;
 }

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;
     }

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;
 }

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());
 

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;
 }

module/Core/src/ShortUrl/Spec/BelongsToApiKeyInlined.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Spec;
+
+use Doctrine\ORM\QueryBuilder;
+use Happyr\DoctrineSpecification\Specification\Specification;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
+
+class BelongsToApiKeyInlined implements Specification
+{
+    private ApiKey $apiKey;
+
+    public function __construct(ApiKey $apiKey)
+    {
+        $this->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
+    {
+    }
+}

module/Core/src/ShortUrl/Spec/BelongsToDomainInlined.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Spec;
+
+use Doctrine\ORM\QueryBuilder;
+use Happyr\DoctrineSpecification\Specification\Specification;
+
+class BelongsToDomainInlined implements Specification
+{
+    private int $domainId;
+
+    public function __construct(int $domainId)
+    {
+        $this->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
+    {
+    }
+}

module/Core/src/Tag/Spec/CountTagsWithName.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Tag\Spec;
+
+use Happyr\DoctrineSpecification\BaseSpecification;
+use Happyr\DoctrineSpecification\Spec;
+use Happyr\DoctrineSpecification\Specification\Specification;
+
+class CountTagsWithName extends BaseSpecification
+{
+    private string $tagName;
+
+    public function __construct(string $tagName)
+    {
+        parent::__construct();
+        $this->tagName = $tagName;
+    }
+
+    protected function getSpec(): Specification
+    {
+        return Spec::countOf(
+            Spec::andX(
+                Spec::select('id'),
+                Spec::eq('name', $this->tagName),
+            ),
+        );
+    }
+}

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();

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();
     }
 }

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),

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();

module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Rest\ApiKey\Spec;
+
+use Happyr\DoctrineSpecification\BaseSpecification;
+use Happyr\DoctrineSpecification\Spec;
+use Happyr\DoctrineSpecification\Specification\Specification;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
+
+class WithApiKeySpecsEnsuringJoin extends BaseSpecification
+{
+    private ?ApiKey $apiKey;
+
+    public function __construct(?ApiKey $apiKey)
+    {
+        parent::__construct();
+        $this->apiKey = $apiKey;
+    }
+
+    protected function getSpec(): Specification
+    {
+        return $this->apiKey === null || $this->apiKey->isAdmin() ? Spec::andX() : Spec::andX(
+            Spec::join('shortUrls', 's'),
+            $this->apiKey->spec(),
+        );
+    }
+}

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;
+    }
 }

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();