Used ShorturlIdentifier model whenever possible

This commit is contained in:
Alejandro Celaya 2021-05-23 08:41:42 +02:00
parent cd19876419
commit 5e6d2881bc
11 changed files with 112 additions and 80 deletions

View file

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

View file

@ -174,9 +174,9 @@ 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();
@ -194,7 +194,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
private function doShortCodeIsInUse(ShortUrlIdentifier $identifier, ?Specification $spec, ?int $lockMode): bool
{
$qb = $this->createFindOneQueryBuilder($identifier->shortCode(), $identifier->domain(), $spec);
$qb = $this->createFindOneQueryBuilder($identifier, $spec);
$qb->select('s.id');
$query = $qb->getQuery()->setLockMode($lockMode);
@ -202,16 +202,16 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
return $query->getOneOrNullResult() !== null;
}
private function createFindOneQueryBuilder(string $slug, ?string $domain, ?Specification $spec): QueryBuilder
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');

View file

@ -35,7 +35,7 @@ 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(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool;

View file

@ -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

View file

@ -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[]

View file

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

View file

@ -210,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 */

View file

@ -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(
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($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(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 */

View file

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

View file

@ -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 */

View file

@ -79,20 +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(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), $spec)->willReturn(
$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();
@ -102,8 +104,10 @@ class VisitsStatsHelperTest extends TestCase
public function throwsExceptionWhenRequestingVisitsForInvalidShortCode(): void
{
$shortCode = '123ABC';
$identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode);
$repo = $this->prophesize(ShortUrlRepositoryInterface::class);
$count = $repo->shortCodeIsInUse(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), null)->willReturn(
$count = $repo->shortCodeIsInUse($identifier, null)->willReturn(
false,
);
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
@ -111,7 +115,7 @@ class VisitsStatsHelperTest extends TestCase
$this->expectException(ShortUrlNotFoundException::class);
$count->shouldBeCalledOnce();
$this->helper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), new VisitsParams());
$this->helper->visitsForShortUrl($identifier, new VisitsParams());
}
/** @test */