diff --git a/CHANGELOG.md b/CHANGELOG.md index 22176f7e..b7df04c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). -## [Unreleased] +## [2.7.0] - 2021-05-23 ### Added * [#1044](https://github.com/shlinkio/shlink/issues/1044) Added ability to set names on API keys, which helps to identify them when the list grows. * [#819](https://github.com/shlinkio/shlink/issues/819) Visits are now always located in real time, even when not using swoole. diff --git a/module/Core/src/Model/ShortUrlIdentifier.php b/module/Core/src/Model/ShortUrlIdentifier.php index 4a74ba07..a277782c 100644 --- a/module/Core/src/Model/ShortUrlIdentifier.php +++ b/module/Core/src/Model/ShortUrlIdentifier.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Model; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Symfony\Component\Console\Input\InputInterface; final class ShortUrlIdentifier @@ -42,6 +43,19 @@ final class ShortUrlIdentifier return new self($shortCode, $domain); } + public static function fromShortUrl(ShortUrl $shortUrl): self + { + $domain = $shortUrl->getDomain(); + $domainAuthority = $domain !== null ? $domain->getAuthority() : null; + + return new self($shortUrl->getShortCode(), $domainAuthority); + } + + public static function fromShortCodeAndDomain(string $shortCode, ?string $domain = null): self + { + return new self($shortCode, $domain); + } + public function shortCode(): string { return $this->shortCode; diff --git a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php index 5369bd05..d651b1b5 100644 --- a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php @@ -33,8 +33,7 @@ class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter public function getSlice($offset, $length): array // phpcs:ignore { return $this->visitRepository->findVisitsByShortCode( - $this->identifier->shortCode(), - $this->identifier->domain(), + $this->identifier, new VisitsListFiltering( $this->params->getDateRange(), $this->params->excludeBots(), @@ -48,8 +47,7 @@ class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter protected function doCount(): int { return $this->visitRepository->countVisitsByShortCode( - $this->identifier->shortCode(), - $this->identifier->domain(), + $this->identifier, new VisitsCountFiltering( $this->params->getDateRange(), $this->params->excludeBots(), diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index eacf293b..c658d478 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; +use Doctrine\DBAL\LockMode; use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; @@ -11,6 +12,7 @@ use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; @@ -172,32 +174,44 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU return $query->getOneOrNullResult(); } - public function findOne(string $shortCode, ?string $domain = null, ?Specification $spec = null): ?ShortUrl + public function findOne(ShortUrlIdentifier $identifier, ?Specification $spec = null): ?ShortUrl { - $qb = $this->createFindOneQueryBuilder($shortCode, $domain, $spec); + $qb = $this->createFindOneQueryBuilder($identifier, $spec); $qb->select('s'); return $qb->getQuery()->getOneOrNullResult(); } - public function shortCodeIsInUse(string $slug, ?string $domain = null, ?Specification $spec = null): bool + public function shortCodeIsInUse(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool { - $qb = $this->createFindOneQueryBuilder($slug, $domain, $spec); - $qb->select('COUNT(DISTINCT s.id)'); - - return ((int) $qb->getQuery()->getSingleScalarResult()) > 0; + return $this->doShortCodeIsInUse($identifier, $spec, null); } - private function createFindOneQueryBuilder(string $slug, ?string $domain, ?Specification $spec): QueryBuilder + public function shortCodeIsInUseWithLock(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool + { + return $this->doShortCodeIsInUse($identifier, $spec, LockMode::PESSIMISTIC_WRITE); + } + + private function doShortCodeIsInUse(ShortUrlIdentifier $identifier, ?Specification $spec, ?int $lockMode): bool + { + $qb = $this->createFindOneQueryBuilder($identifier, $spec); + $qb->select('s.id'); + + $query = $qb->getQuery()->setLockMode($lockMode); + + return $query->getOneOrNullResult() !== null; + } + + private function createFindOneQueryBuilder(ShortUrlIdentifier $identifier, ?Specification $spec): QueryBuilder { $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(ShortUrl::class, 's') ->where($qb->expr()->isNotNull('s.shortCode')) ->andWhere($qb->expr()->eq('s.shortCode', ':slug')) - ->setParameter('slug', $slug) + ->setParameter('slug', $identifier->shortCode()) ->setMaxResults(1); - $this->whereDomainIs($qb, $domain); + $this->whereDomainIs($qb, $identifier->domain()); $this->applySpecification($qb, $spec, 's'); diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index 5d8fa924..7489f2a0 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -9,6 +9,7 @@ use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterfa use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; @@ -34,9 +35,11 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat public function findOneWithDomainFallback(string $shortCode, ?string $domain = null): ?ShortUrl; - public function findOne(string $shortCode, ?string $domain = null, ?Specification $spec = null): ?ShortUrl; + public function findOne(ShortUrlIdentifier $identifier, ?Specification $spec = null): ?ShortUrl; - public function shortCodeIsInUse(string $slug, ?string $domain, ?Specification $spec = null): bool; + public function shortCodeIsInUse(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool; + + public function shortCodeIsInUseWithLock(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool; public function findOneMatching(ShortUrlMeta $meta): ?ShortUrl; diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index bc3f958b..61cd108e 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits; @@ -84,28 +85,27 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo /** * @return Visit[] */ - public function findVisitsByShortCode(string $shortCode, ?string $domain, VisitsListFiltering $filtering): array + public function findVisitsByShortCode(ShortUrlIdentifier $identifier, VisitsListFiltering $filtering): array { - $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $filtering); + $qb = $this->createVisitsByShortCodeQueryBuilder($identifier, $filtering); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit(), $filtering->offset()); } - public function countVisitsByShortCode(string $shortCode, ?string $domain, VisitsCountFiltering $filtering): int + public function countVisitsByShortCode(ShortUrlIdentifier $identifier, VisitsCountFiltering $filtering): int { - $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $filtering); + $qb = $this->createVisitsByShortCodeQueryBuilder($identifier, $filtering); $qb->select('COUNT(v.id)'); return (int) $qb->getQuery()->getSingleScalarResult(); } private function createVisitsByShortCodeQueryBuilder( - string $shortCode, - ?string $domain, + ShortUrlIdentifier $identifier, VisitsCountFiltering $filtering ): QueryBuilder { /** @var ShortUrlRepositoryInterface $shortUrlRepo */ $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOne($shortCode, $domain, $filtering->spec()); + $shortUrl = $shortUrlRepo->findOne($identifier, $filtering->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 05a59720..28f1e9a8 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Persistence\ObjectRepository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -33,9 +34,9 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification /** * @return Visit[] */ - public function findVisitsByShortCode(string $shortCode, ?string $domain, VisitsListFiltering $filtering): array; + public function findVisitsByShortCode(ShortUrlIdentifier $identifier, VisitsListFiltering $filtering): array; - public function countVisitsByShortCode(string $shortCode, ?string $domain, VisitsCountFiltering $filtering): int; + public function countVisitsByShortCode(ShortUrlIdentifier $identifier, VisitsCountFiltering $filtering): int; /** * @return Visit[] diff --git a/module/Core/src/Service/ShortUrl/ShortCodeHelper.php b/module/Core/src/Service/ShortUrl/ShortCodeHelper.php index 3df4c016..83c3397e 100644 --- a/module/Core/src/Service/ShortUrl/ShortCodeHelper.php +++ b/module/Core/src/Service/ShortUrl/ShortCodeHelper.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; class ShortCodeHelper implements ShortCodeHelperInterface // TODO Rename to ShortCodeUniquenessHelper @@ -19,13 +20,9 @@ class ShortCodeHelper implements ShortCodeHelperInterface // TODO Rename to Shor public function ensureShortCodeUniqueness(ShortUrl $shortUrlToBeCreated, bool $hasCustomSlug): bool { - $shortCode = $shortUrlToBeCreated->getShortCode(); - $domain = $shortUrlToBeCreated->getDomain(); - $domainAuthority = $domain !== null ? $domain->getAuthority() : null; - /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); - $otherShortUrlsExist = $repo->shortCodeIsInUse($shortCode, $domainAuthority); + $otherShortUrlsExist = $repo->shortCodeIsInUseWithLock(ShortUrlIdentifier::fromShortUrl($shortUrlToBeCreated)); if (! $otherShortUrlsExist) { return true; diff --git a/module/Core/src/Service/ShortUrl/ShortUrlResolver.php b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php index 6e03114c..1394e1ab 100644 --- a/module/Core/src/Service/ShortUrl/ShortUrlResolver.php +++ b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php @@ -27,11 +27,7 @@ class ShortUrlResolver implements ShortUrlResolverInterface { /** @var ShortUrlRepository $shortUrlRepo */ $shortUrlRepo = $this->em->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOne( - $identifier->shortCode(), - $identifier->domain(), - $apiKey !== null ? $apiKey->spec() : null, - ); + $shortUrl = $shortUrlRepo->findOne($identifier, $apiKey !== null ? $apiKey->spec() : null); if ($shortUrl === null) { throw ShortUrlNotFoundException::fromNotFound($identifier); } diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index eb7fddad..8601a045 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -62,7 +62,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt return new Collections\ArrayCollection(map($tags, function (string $tagName) use ($repo): Tag { // Memoize only new tags, and let doctrine handle objects hydrated from persistence - $tag = $repo->findOneBy(['name' => $tagName]) ?? $this->memoizeNewTag($tagName); + $tag = $repo->findOneBy(['name' => $tagName]) ?? $this->memoizeNewTag($tagName); $this->em->persist($tag); return $tag; diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 1aa03c46..dfa00a4c 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -58,7 +58,7 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface /** @var ShortUrlRepositoryInterface $repo */ $repo = $this->em->getRepository(ShortUrl::class); - if (! $repo->shortCodeIsInUse($identifier->shortCode(), $identifier->domain(), $spec)) { + if (! $repo->shortCodeIsInUse($identifier, $spec)) { throw ShortUrlNotFoundException::fromNotFound($identifier); } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index bd5b22d4..867ff3f2 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use Shlinkio\Shlink\Core\Model\Visitor; @@ -180,12 +181,18 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertTrue($this->repo->shortCodeIsInUse('my-cool-slug')); - self::assertFalse($this->repo->shortCodeIsInUse('my-cool-slug', 'doma.in')); - self::assertFalse($this->repo->shortCodeIsInUse('slug-not-in-use')); - self::assertFalse($this->repo->shortCodeIsInUse('another-slug')); - self::assertFalse($this->repo->shortCodeIsInUse('another-slug', 'example.com')); - self::assertTrue($this->repo->shortCodeIsInUse('another-slug', 'doma.in')); + self::assertTrue($this->repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain('my-cool-slug'))); + self::assertFalse($this->repo->shortCodeIsInUse( + ShortUrlIdentifier::fromShortCodeAndDomain('my-cool-slug', 'doma.in'), + )); + self::assertFalse($this->repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain('slug-not-in-use'))); + self::assertFalse($this->repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain('another-slug'))); + self::assertFalse($this->repo->shortCodeIsInUse( + ShortUrlIdentifier::fromShortCodeAndDomain('another-slug', 'example.com'), + )); + self::assertTrue($this->repo->shortCodeIsInUse( + ShortUrlIdentifier::fromShortCodeAndDomain('another-slug', 'doma.in'), + )); } /** @test */ @@ -203,12 +210,16 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertNotNull($this->repo->findOne('my-cool-slug')); - self::assertNull($this->repo->findOne('my-cool-slug', 'doma.in')); - self::assertNull($this->repo->findOne('slug-not-in-use')); - self::assertNull($this->repo->findOne('another-slug')); - self::assertNull($this->repo->findOne('another-slug', 'example.com')); - self::assertNotNull($this->repo->findOne('another-slug', 'doma.in')); + self::assertNotNull($this->repo->findOne(ShortUrlIdentifier::fromShortCodeAndDomain('my-cool-slug'))); + self::assertNull($this->repo->findOne(ShortUrlIdentifier::fromShortCodeAndDomain('my-cool-slug', 'doma.in'))); + self::assertNull($this->repo->findOne(ShortUrlIdentifier::fromShortCodeAndDomain('slug-not-in-use'))); + self::assertNull($this->repo->findOne(ShortUrlIdentifier::fromShortCodeAndDomain('another-slug'))); + self::assertNull($this->repo->findOne( + ShortUrlIdentifier::fromShortCodeAndDomain('another-slug', 'example.com'), + )); + self::assertNotNull($this->repo->findOne( + ShortUrlIdentifier::fromShortCodeAndDomain('another-slug', 'doma.in'), + )); } /** @test */ diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index f77d7fd0..15fe34f4 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\VisitRepository; @@ -89,32 +90,46 @@ class VisitRepositoryTest extends DatabaseTestCase { [$shortCode, $domain] = $this->createShortUrlsAndVisits(); - self::assertCount(0, $this->repo->findVisitsByShortCode('invalid', null, new VisitsListFiltering())); - self::assertCount(6, $this->repo->findVisitsByShortCode($shortCode, null, new VisitsListFiltering())); - self::assertCount(4, $this->repo->findVisitsByShortCode($shortCode, null, new VisitsListFiltering(null, true))); - self::assertCount(3, $this->repo->findVisitsByShortCode($shortCode, $domain, new VisitsListFiltering())); - self::assertCount(2, $this->repo->findVisitsByShortCode($shortCode, null, new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), - ))); - self::assertCount(4, $this->repo->findVisitsByShortCode($shortCode, null, new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), - ))); - self::assertCount(1, $this->repo->findVisitsByShortCode($shortCode, $domain, new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), - ))); + self::assertCount(0, $this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain('invalid'), + new VisitsListFiltering(), + )); + self::assertCount(6, $this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsListFiltering(), + )); + self::assertCount(4, $this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsListFiltering(null, true), + )); self::assertCount(3, $this->repo->findVisitsByShortCode( - $shortCode, - null, + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, $domain), + new VisitsListFiltering(), + )); + self::assertCount(2, $this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsListFiltering( + DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + ), + )); + self::assertCount(4, $this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsListFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + )); + self::assertCount(1, $this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, $domain), + new VisitsListFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + )); + self::assertCount(3, $this->repo->findVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), new VisitsListFiltering(null, false, null, 3, 2), )); self::assertCount(2, $this->repo->findVisitsByShortCode( - $shortCode, - null, + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), new VisitsListFiltering(null, false, null, 5, 4), )); self::assertCount(1, $this->repo->findVisitsByShortCode( - $shortCode, - $domain, + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, $domain), new VisitsListFiltering(null, false, null, 3, 2), )); } @@ -124,22 +139,36 @@ class VisitRepositoryTest extends DatabaseTestCase { [$shortCode, $domain] = $this->createShortUrlsAndVisits(); - self::assertEquals(0, $this->repo->countVisitsByShortCode('invalid', null, new VisitsCountFiltering())); - self::assertEquals(6, $this->repo->countVisitsByShortCode($shortCode, null, new VisitsCountFiltering())); - self::assertEquals(4, $this->repo->countVisitsByShortCode($shortCode, null, new VisitsCountFiltering( - null, - true, - ))); - self::assertEquals(3, $this->repo->countVisitsByShortCode($shortCode, $domain, new VisitsCountFiltering())); - self::assertEquals(2, $this->repo->countVisitsByShortCode($shortCode, null, new VisitsCountFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), - ))); - self::assertEquals(4, $this->repo->countVisitsByShortCode($shortCode, null, new VisitsCountFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), - ))); - self::assertEquals(1, $this->repo->countVisitsByShortCode($shortCode, $domain, new VisitsCountFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), - ))); + self::assertEquals(0, $this->repo->countVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain('invalid'), + new VisitsCountFiltering(), + )); + self::assertEquals(6, $this->repo->countVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsCountFiltering(), + )); + self::assertEquals(4, $this->repo->countVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsCountFiltering(null, true), + )); + self::assertEquals(3, $this->repo->countVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, $domain), + new VisitsCountFiltering(), + )); + self::assertEquals(2, $this->repo->countVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsCountFiltering( + DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + ), + )); + self::assertEquals(4, $this->repo->countVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + new VisitsCountFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + )); + self::assertEquals(1, $this->repo->countVisitsByShortCode( + ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, $domain), + new VisitsCountFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + )); } /** @test */ diff --git a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php index d7b454b0..2a9e5fc4 100644 --- a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -35,8 +35,7 @@ class VisitsPaginatorAdapterTest extends TestCase $offset = 5; $adapter = $this->createAdapter(null); $findVisits = $this->repo->findVisitsByShortCode( - '', - null, + ShortUrlIdentifier::fromShortCodeAndDomain(''), new VisitsListFiltering(new DateRange(), false, null, $limit, $offset), )->willReturn([]); @@ -54,8 +53,7 @@ class VisitsPaginatorAdapterTest extends TestCase $apiKey = ApiKey::create(); $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByShortCode( - '', - null, + ShortUrlIdentifier::fromShortCodeAndDomain(''), new VisitsCountFiltering(new DateRange(), false, $apiKey->spec()), )->willReturn(3); diff --git a/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php b/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php index 047dbc96..ca3b463f 100644 --- a/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php +++ b/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php @@ -10,6 +10,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelper; @@ -39,12 +40,12 @@ class ShortCodeHelperTest extends TestCase $callIndex = 0; $expectedCalls = 3; $repo = $this->prophesize(ShortUrlRepository::class); - $shortCodeIsInUse = $repo->shortCodeIsInUse('abc123', $expectedAuthority)->will( - function () use (&$callIndex, $expectedCalls) { - $callIndex++; - return $callIndex < $expectedCalls; - }, - ); + $shortCodeIsInUse = $repo->shortCodeIsInUseWithLock( + ShortUrlIdentifier::fromShortCodeAndDomain('abc123', $expectedAuthority), + )->will(function () use (&$callIndex, $expectedCalls) { + $callIndex++; + return $callIndex < $expectedCalls; + }); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->shortUrl->getDomain()->willReturn($domain); @@ -66,7 +67,9 @@ class ShortCodeHelperTest extends TestCase public function inUseSlugReturnsError(): void { $repo = $this->prophesize(ShortUrlRepository::class); - $shortCodeIsInUse = $repo->shortCodeIsInUse('abc123', null)->willReturn(true); + $shortCodeIsInUse = $repo->shortCodeIsInUseWithLock( + ShortUrlIdentifier::fromShortCodeAndDomain('abc123'), + )->willReturn(true); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->shortUrl->getDomain()->willReturn(null); diff --git a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php index cf2330b3..73823729 100644 --- a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php @@ -46,12 +46,13 @@ class ShortUrlResolverTest extends TestCase { $shortUrl = ShortUrl::withLongUrl('expected_url'); $shortCode = $shortUrl->getShortCode(); + $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOne = $repo->findOne($shortCode, null, $apiKey !== null ? $apiKey->spec() : null)->willReturn($shortUrl); + $findOne = $repo->findOne($identifier, $apiKey !== null ? $apiKey->spec() : null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $result = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey); + $result = $this->urlResolver->resolveShortUrl($identifier, $apiKey); self::assertSame($shortUrl, $result); $findOne->shouldHaveBeenCalledOnce(); @@ -65,16 +66,17 @@ class ShortUrlResolverTest extends TestCase public function exceptionIsThrownIfShortcodeIsNotFound(?ApiKey $apiKey): void { $shortCode = 'abc123'; + $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOne = $repo->findOne($shortCode, null, $apiKey !== null ? $apiKey->spec() : null)->willReturn(null); + $findOne = $repo->findOne($identifier, $apiKey !== null ? $apiKey->spec() : null)->willReturn(null); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal(), $apiKey); $this->expectException(ShortUrlNotFoundException::class); $findOne->shouldBeCalledOnce(); $getRepo->shouldBeCalledOnce(); - $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode), $apiKey); + $this->urlResolver->resolveShortUrl($identifier, $apiKey); } /** @test */ diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 7e319314..6bac432e 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -46,7 +46,6 @@ class UrlShortenerTest extends TestCase return $callback(); }); $repo = $this->prophesize(ShortUrlRepository::class); - $repo->shortCodeIsInUse(Argument::cetera())->willReturn(false); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->shortCodeHelper = $this->prophesize(ShortCodeHelperInterface::class); diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 8660099c..aeef3f47 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -6,11 +6,11 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Resolver; use Doctrine\Common\EventManager; use Doctrine\ORM\EntityManagerInterface; -use Doctrine\Persistence\ObjectRepository; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; @@ -48,7 +48,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase */ public function findsOrCreatesDomainWhenValueIsProvided(?Domain $foundDomain, string $authority): void { - $repo = $this->prophesize(ObjectRepository::class); + $repo = $this->prophesize(DomainRepositoryInterface::class); $findDomain = $repo->findOneBy(['authority' => $authority])->willReturn($foundDomain); $getRepository = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); @@ -121,7 +121,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase /** @test */ public function newDomainsAreMemoizedUntilStateIsCleared(): void { - $repo = $this->prophesize(ObjectRepository::class); + $repo = $this->prophesize(DomainRepositoryInterface::class); $repo->findOneBy(Argument::type('array'))->willReturn(null); $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index e6f067da..cae3fbb1 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -79,18 +79,22 @@ class VisitsStatsHelperTest extends TestCase public function infoReturnsVisitsForCertainShortCode(?ApiKey $apiKey): void { $shortCode = '123ABC'; + $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); $spec = $apiKey === null ? null : $apiKey->spec(); + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $count = $repo->shortCodeIsInUse($shortCode, null, $spec)->willReturn(true); + $count = $repo->shortCodeIsInUse($identifier, $spec)->willReturn( + true, + ); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByShortCode($shortCode, null, Argument::type(VisitsListFiltering::class))->willReturn($list); - $repo2->countVisitsByShortCode($shortCode, null, Argument::type(VisitsCountFiltering::class))->willReturn(1); + $repo2->findVisitsByShortCode($identifier, Argument::type(VisitsListFiltering::class))->willReturn($list); + $repo2->countVisitsByShortCode($identifier, Argument::type(VisitsCountFiltering::class))->willReturn(1); $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); - $paginator = $this->helper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), new VisitsParams(), $apiKey); + $paginator = $this->helper->visitsForShortUrl($identifier, new VisitsParams(), $apiKey); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); $count->shouldHaveBeenCalledOnce(); @@ -100,14 +104,18 @@ class VisitsStatsHelperTest extends TestCase public function throwsExceptionWhenRequestingVisitsForInvalidShortCode(): void { $shortCode = '123ABC'; + $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $count = $repo->shortCodeIsInUse($shortCode, null, null)->willReturn(false); + $count = $repo->shortCodeIsInUse($identifier, null)->willReturn( + false, + ); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $this->expectException(ShortUrlNotFoundException::class); $count->shouldBeCalledOnce(); - $this->helper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), new VisitsParams()); + $this->helper->visitsForShortUrl($identifier, new VisitsParams()); } /** @test */