Fix 500 error when listing non-orphan visits with short-url-depending API key

This commit is contained in:
Alejandro Celaya
2025-02-19 19:33:44 +01:00
parent 7bbc938743
commit 449a588796
3 changed files with 23 additions and 18 deletions

View File

@@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
## [Unreleased] ## [4.4.4] - 2025-02-19
### Added ### Added
* *Nothing* * *Nothing*
@@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
### Fixed ### Fixed
* [#2366](https://github.com/shlinkio/shlink/issues/2366) Fix error "Cannot use 'SCRIPT' with redis-cluster" thrown when creating a lock while using a redis cluster. * [#2366](https://github.com/shlinkio/shlink/issues/2366) Fix error "Cannot use 'SCRIPT' with redis-cluster" thrown when creating a lock while using a redis cluster.
* [#2368](https://github.com/shlinkio/shlink/issues/2368) Fix error when listing non-orphan visits using API key with `AUTHORED_SHORT_URLS` role.
## [4.4.3] - 2025-02-15 ## [4.4.3] - 2025-02-15
### Added ### Added

View File

@@ -21,6 +21,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Spec\CountOfNonOrphanVisits; use Shlinkio\Shlink\Core\Visit\Spec\CountOfNonOrphanVisits;
use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits; use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits;
use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\ApiKey\Role;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use const PHP_INT_MAX; use const PHP_INT_MAX;
@@ -177,7 +178,12 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo
$qb = $this->createAllVisitsQueryBuilder($filtering); $qb = $this->createAllVisitsQueryBuilder($filtering);
$qb->andWhere($qb->expr()->isNotNull('v.shortUrl')); $qb->andWhere($qb->expr()->isNotNull('v.shortUrl'));
$this->applySpecification($qb, $filtering->apiKey?->inlinedSpec()); $apiKey = $filtering->apiKey;
if (ApiKey::isShortUrlRestricted($apiKey)) {
$qb->join('v.shortUrl', 's');
}
$this->applySpecification($qb, $apiKey?->inlinedSpec(), 'v');
return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset);
} }

View File

@@ -470,22 +470,18 @@ class VisitRepositoryTest extends DatabaseTestCase
#[Test] #[Test]
public function findNonOrphanVisitsReturnsExpectedResult(): void public function findNonOrphanVisitsReturnsExpectedResult(): void
{ {
$shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://1'])); $authoredApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()));
$this->getEntityManager()->persist($shortUrl); $this->getEntityManager()->persist($authoredApiKey);
$this->createVisitsForShortUrl($shortUrl, 7);
$shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://2'])); $this->createShortUrlsAndVisits(withDomain: false, visitsAmount: 7);
$this->getEntityManager()->persist($shortUrl2); $this->createShortUrlsAndVisits(withDomain: false, apiKey: $authoredApiKey, visitsAmount: 4);
$this->createVisitsForShortUrl($shortUrl2, 4); $this->createShortUrlsAndVisits(withDomain: false, visitsAmount: 10);
$shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://3']));
$this->getEntityManager()->persist($shortUrl3);
$this->createVisitsForShortUrl($shortUrl3, 10);
$this->getEntityManager()->flush(); $this->getEntityManager()->flush();
self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering())); self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering()));
self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::allTime()))); self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::allTime())));
self::assertCount(4, $this->repo->findNonOrphanVisits(new VisitsListFiltering(apiKey: $authoredApiKey)));
self::assertCount(7, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::since( self::assertCount(7, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::since(
Chronos::parse('2016-01-05')->endOfDay(), Chronos::parse('2016-01-05')->endOfDay(),
)))); ))));
@@ -503,11 +499,11 @@ class VisitRepositoryTest extends DatabaseTestCase
self::assertCount(3, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( self::assertCount(3, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between(
Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-03')->startOfDay(),
Chronos::parse('2016-01-08')->endOfDay(), Chronos::parse('2016-01-08')->endOfDay(),
), false, null, 10, 10))); ), limit: 10, offset: 10)));
self::assertCount(15, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, true))); self::assertCount(15, $this->repo->findNonOrphanVisits(new VisitsListFiltering(excludeBots: true)));
self::assertCount(10, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, false, null, 10))); self::assertCount(10, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 10)));
self::assertCount(1, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, false, null, 10, 20))); self::assertCount(1, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 10, offset: 20)));
self::assertCount(5, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, false, null, 5, 5))); self::assertCount(5, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 5, offset: 5)));
} }
#[Test] #[Test]
@@ -535,6 +531,7 @@ class VisitRepositoryTest extends DatabaseTestCase
bool|string $withDomain = true, bool|string $withDomain = true,
array $tags = [], array $tags = [],
ApiKey|null $apiKey = null, ApiKey|null $apiKey = null,
int $visitsAmount = 6,
): array { ): array {
$shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([
ShortUrlInputFilter::LONG_URL => 'https://longUrl', ShortUrlInputFilter::LONG_URL => 'https://longUrl',
@@ -545,7 +542,7 @@ class VisitRepositoryTest extends DatabaseTestCase
$shortCode = $shortUrl->getShortCode(); $shortCode = $shortUrl->getShortCode();
$this->getEntityManager()->persist($shortUrl); $this->getEntityManager()->persist($shortUrl);
$this->createVisitsForShortUrl($shortUrl); $this->createVisitsForShortUrl($shortUrl, $visitsAmount);
if ($withDomain !== false) { if ($withDomain !== false) {
$shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData([ $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData([