From 460ca032d28302a13eaf810bcf0d3ab92a95e596 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 23 Sep 2020 00:22:29 +0200 Subject: [PATCH 1/4] Drastically improved performance when creating new short URLs with findIfExists by moving logic to DB query --- module/Core/src/Entity/ShortUrl.php | 26 -------- .../src/Repository/ShortUrlRepository.php | 60 +++++++++++++++++++ .../ShortUrlRepositoryInterface.php | 3 + module/Core/src/Service/UrlShortener.php | 24 ++------ .../Repository/ShortUrlRepositoryTest.php | 30 ++++++++++ module/Core/test/Entity/ShortUrlTest.php | 34 ----------- module/Core/test/Service/UrlShortenerTest.php | 31 +--------- 7 files changed, 98 insertions(+), 110 deletions(-) diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 0f2811fb..ba10a44a 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -15,10 +15,7 @@ use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; -use function array_reduce; use function count; -use function Functional\contains; -use function Functional\invoke; use function Shlinkio\Shlink\Core\generateRandomShortCode; class ShortUrl extends AbstractEntity @@ -195,27 +192,4 @@ class ShortUrl extends AbstractEntity return $this->domain->getAuthority(); } - - public function matchesCriteria(ShortUrlMeta $meta, array $tags): bool - { - if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $this->maxVisits) { - return false; - } - if ($meta->hasDomain() && $meta->getDomain() !== $this->resolveDomain()) { - return false; - } - if ($meta->hasValidSince() && ($this->validSince === null || ! $meta->getValidSince()->eq($this->validSince))) { - return false; - } - if ($meta->hasValidUntil() && ($this->validUntil === null || ! $meta->getValidUntil()->eq($this->validUntil))) { - return false; - } - - $shortUrlTags = invoke($this->getTags(), '__toString'); - return count($shortUrlTags) === count($tags) && array_reduce( - $tags, - fn (bool $hasAllTags, string $tag) => $hasAllTags && contains($shortUrlTags, $tag), - true, - ); - } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 31fe1385..4bcbe059 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -5,13 +5,16 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; use Doctrine\ORM\EntityRepository; +use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\QueryBuilder; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use function array_column; use function array_key_exists; +use function count; use function Functional\contains; class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryInterface @@ -196,4 +199,61 @@ DQL; return $qb; } + + public function findOneMatching(string $url, array $tags, ShortUrlMeta $meta): ?ShortUrl + { + $qb = $this->getEntityManager()->createQueryBuilder(); + + $qb->select('s') + ->from(ShortUrl::class, 's') + ->where($qb->expr()->eq('s.longUrl', ':longUrl')) + ->setParameter('longUrl', $url) + ->setMaxResults(1); + + if ($meta->hasCustomSlug()) { + $qb->andWhere($qb->expr()->eq('s.shortCode', ':slug')) + ->setParameter('slug', $meta->getCustomSlug()); + } + if ($meta->hasMaxVisits()) { + $qb->andWhere($qb->expr()->eq('s.maxVisits', ':maxVisits')) + ->setParameter('maxVisits', $meta->getMaxVisits()); + } + if ($meta->hasValidSince()) { + $qb->andWhere($qb->expr()->eq('s.validSince', ':validSince')) + ->setParameter('validSince', $meta->getValidSince()); + } + if ($meta->hasValidUntil()) { + $qb->andWhere($qb->expr()->eq('s.validUntil', ':validUntil')) + ->setParameter('validUntil', $meta->getValidUntil()); + } + + if ($meta->hasDomain()) { + $qb->join('s.domain', 'd') + ->andWhere($qb->expr()->eq('d.authority', ':domain')) + ->setParameter('domain', $meta->getDomain()); + } + + $tagsAmount = count($tags); + if ($tagsAmount === 0) { + return $qb->getQuery()->getOneOrNullResult(); + } + + foreach ($tags as $index => $tag) { + $alias = 't_' . $index; + $qb->join('s.tags', $alias, Join::WITH, $qb->expr()->eq($alias . '.name', ':tag' . $index)) + ->setParameter('tag' . $index, $tag); + } + + // If tags where provided, we need an extra join to see the amount of tags that every short URL has, so that we + // can discard those that also have more tags, making sure only those fully matching are included. + $qb->addSelect('COUNT(t.id) as tagsAmount') + ->join('s.tags', 't') + ->groupBy('s') + ->having($qb->expr()->eq('tagsAmount', ':tagsAmount')) + ->setParameter('tagsAmount', $tagsAmount); + + $result = $qb->getQuery()->getOneOrNullResult() ?? []; + + return $result[0] ?? null; + } } diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index 065198b4..65278a85 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Persistence\ObjectRepository; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; interface ShortUrlRepositoryInterface extends ObjectRepository @@ -27,4 +28,6 @@ interface ShortUrlRepositoryInterface extends ObjectRepository public function findOne(string $shortCode, ?string $domain = null): ?ShortUrl; public function shortCodeIsInUse(string $slug, ?string $domain): bool; + + public function findOneMatching(string $url, array $tags, ShortUrlMeta $meta): ?ShortUrl; } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 7892f959..cf17d0bd 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -11,12 +11,11 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; +use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; use Throwable; -use function array_reduce; - class UrlShortener implements UrlShortenerInterface { use TagManagerTrait; @@ -77,24 +76,9 @@ class UrlShortener implements UrlShortenerInterface return null; } - $criteria = ['longUrl' => $url]; - if ($meta->hasCustomSlug()) { - $criteria['shortCode'] = $meta->getCustomSlug(); - } - /** @var ShortUrl[] $shortUrls */ - $shortUrls = $this->em->getRepository(ShortUrl::class)->findBy($criteria); - if (empty($shortUrls)) { - return null; - } - - // Iterate short URLs until one that matches is found, or return null otherwise - return array_reduce($shortUrls, function (?ShortUrl $found, ShortUrl $shortUrl) use ($tags, $meta) { - if ($found !== null) { - return $found; - } - - return $shortUrl->matchesCriteria($meta, $tags) ? $shortUrl : null; - }); + /** @var ShortUrlRepositoryInterface $repo */ + $repo = $this->em->getRepository(ShortUrl::class); + return $repo->findOneMatching($url, $tags, $meta); } private function verifyShortCodeUniqueness(ShortUrlMeta $meta, ShortUrl $shortUrlToBeCreated): void diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 4829fada..85df47de 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -210,4 +210,34 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertNull($this->repo->findOne('another-slug', 'example.com')); $this->assertNotNull($this->repo->findOne('another-slug', 'doma.in')); } + +// public function findOneMatchingAppliesProperConditions(): void +// { +// $matches = $this->provideCriteriaToMatch(); +// } +// +// private function provideCriteriaToMatch(): iterable +// { +// $start = Chronos::parse('2020-03-05 20:18:30'); +// $end = Chronos::parse('2021-03-05 20:18:30'); +// +// yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start]), false]; +// yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validUntil' => $end]), false]; +// yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start, 'validUntil' => $end]), false]; +// yield [ +// new ShortUrl('foo', ShortUrlMeta::fromRawData(['validSince' => $start])), +// ShortUrlMeta::fromRawData(['validSince' => $start]), +// true, +// ]; +// yield [ +// new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end])), +// ShortUrlMeta::fromRawData(['validUntil' => $end]), +// true, +// ]; +// yield [ +// new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start])), +// ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start]), +// true, +// ]; +// } } diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php index 07308580..c5933e39 100644 --- a/module/Core/test/Entity/ShortUrlTest.php +++ b/module/Core/test/Entity/ShortUrlTest.php @@ -75,38 +75,4 @@ class ShortUrlTest extends TestCase yield [null, DEFAULT_SHORT_CODES_LENGTH]; yield from map(range(4, 10), fn (int $value) => [$value, $value]); } - - /** - * @test - * @dataProvider provideCriteriaToMatch - */ - public function criteriaIsMatchedWhenDatesMatch(ShortUrl $shortUrl, ShortUrlMeta $meta, bool $expected): void - { - $this->assertEquals($expected, $shortUrl->matchesCriteria($meta, [])); - } - - public function provideCriteriaToMatch(): iterable - { - $start = Chronos::parse('2020-03-05 20:18:30'); - $end = Chronos::parse('2021-03-05 20:18:30'); - - yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start]), false]; - yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validUntil' => $end]), false]; - yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start, 'validUntil' => $end]), false]; - yield [ - new ShortUrl('foo', ShortUrlMeta::fromRawData(['validSince' => $start])), - ShortUrlMeta::fromRawData(['validSince' => $start]), - true, - ]; - yield [ - new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end])), - ShortUrlMeta::fromRawData(['validUntil' => $end]), - true, - ]; - yield [ - new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start])), - ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start]), - true, - ]; - } } diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index f1ef88af..9e33af6b 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -147,7 +147,7 @@ class UrlShortenerTest extends TestCase ShortUrl $expected ): void { $repo = $this->prophesize(ShortUrlRepository::class); - $findExisting = $repo->findBy(Argument::any())->willReturn([$expected]); + $findExisting = $repo->findOneMatching(Argument::cetera())->willReturn($expected); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $result = $this->urlShortener->urlToShortCode($url, $tags, $meta); @@ -211,33 +211,4 @@ class UrlShortenerTest extends TestCase ])))->setTags(new ArrayCollection([new Tag('foo'), new Tag('bar'), new Tag('baz')])), ]; } - - /** @test */ - public function properExistingShortUrlIsReturnedWhenMultipleMatch(): void - { - $url = 'http://foo.com'; - $tags = ['baz', 'foo', 'bar']; - $meta = ShortUrlMeta::fromRawData([ - 'findIfExists' => true, - 'validUntil' => Chronos::parse('2017-01-01'), - 'maxVisits' => 4, - ]); - $tagsCollection = new ArrayCollection(array_map(fn (string $tag) => new Tag($tag), $tags)); - $expected = (new ShortUrl($url, $meta))->setTags($tagsCollection); - - $repo = $this->prophesize(ShortUrlRepository::class); - $findExisting = $repo->findBy(Argument::any())->willReturn([ - new ShortUrl($url), - new ShortUrl($url, $meta), - $expected, - (new ShortUrl($url))->setTags($tagsCollection), - ]); - $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - - $result = $this->urlShortener->urlToShortCode($url, $tags, $meta); - - $this->assertSame($expected, $result); - $findExisting->shouldHaveBeenCalledOnce(); - $getRepo->shouldHaveBeenCalledOnce(); - } } From 4e94f070501550e00fb69ea3342d006b86b493d3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 23 Sep 2020 07:34:36 +0200 Subject: [PATCH 2/4] Added tests for new ShortUrlRepository::findOneMatching method --- .../src/Repository/ShortUrlRepository.php | 12 +- .../Repository/ShortUrlRepositoryTest.php | 138 ++++++++++++++---- 2 files changed, 114 insertions(+), 36 deletions(-) diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 4bcbe059..6147298e 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -208,7 +208,8 @@ DQL; ->from(ShortUrl::class, 's') ->where($qb->expr()->eq('s.longUrl', ':longUrl')) ->setParameter('longUrl', $url) - ->setMaxResults(1); + ->setMaxResults(1) + ->orderBy('s.id'); if ($meta->hasCustomSlug()) { $qb->andWhere($qb->expr()->eq('s.shortCode', ':slug')) @@ -246,14 +247,11 @@ DQL; // If tags where provided, we need an extra join to see the amount of tags that every short URL has, so that we // can discard those that also have more tags, making sure only those fully matching are included. - $qb->addSelect('COUNT(t.id) as tagsAmount') - ->join('s.tags', 't') + $qb->join('s.tags', 't') ->groupBy('s') - ->having($qb->expr()->eq('tagsAmount', ':tagsAmount')) + ->having($qb->expr()->eq('COUNT(t.id)', ':tagsAmount')) ->setParameter('tagsAmount', $tagsAmount); - $result = $qb->getQuery()->getOneOrNullResult() ?? []; - - return $result[0] ?? null; + return $qb->getQuery()->getOneOrNullResult(); } } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 85df47de..57a174c7 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -16,12 +16,15 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; +use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; use function count; class ShortUrlRepositoryTest extends DatabaseTestCase { + use TagManagerTrait; + protected const ENTITIES_TO_EMPTY = [ Tag::class, Visit::class, @@ -211,33 +214,110 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertNotNull($this->repo->findOne('another-slug', 'doma.in')); } -// public function findOneMatchingAppliesProperConditions(): void -// { -// $matches = $this->provideCriteriaToMatch(); -// } -// -// private function provideCriteriaToMatch(): iterable -// { -// $start = Chronos::parse('2020-03-05 20:18:30'); -// $end = Chronos::parse('2021-03-05 20:18:30'); -// -// yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start]), false]; -// yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validUntil' => $end]), false]; -// yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start, 'validUntil' => $end]), false]; -// yield [ -// new ShortUrl('foo', ShortUrlMeta::fromRawData(['validSince' => $start])), -// ShortUrlMeta::fromRawData(['validSince' => $start]), -// true, -// ]; -// yield [ -// new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end])), -// ShortUrlMeta::fromRawData(['validUntil' => $end]), -// true, -// ]; -// yield [ -// new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start])), -// ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start]), -// true, -// ]; -// } + /** @test */ + public function findOneMatchingReturnsNullForNonExistingShortUrls(): void + { + $this->assertNull($this->repo->findOneMatching('', [], ShortUrlMeta::createEmpty())); + $this->assertNull($this->repo->findOneMatching('foobar', [], ShortUrlMeta::createEmpty())); + $this->assertNull($this->repo->findOneMatching('foobar', ['foo', 'bar'], ShortUrlMeta::createEmpty())); + $this->assertNull($this->repo->findOneMatching('foobar', ['foo', 'bar'], ShortUrlMeta::fromRawData([ + 'validSince' => Chronos::parse('2020-03-05 20:18:30'), + 'customSlug' => 'this_slug_does_not_exist', + ]))); + } + + /** @test */ + public function findOneMatchingAppliesProperConditions(): void + { + $start = Chronos::parse('2020-03-05 20:18:30'); + $end = Chronos::parse('2021-03-05 20:18:30'); + + $shortUrl = new ShortUrl('foo', ShortUrlMeta::fromRawData(['validSince' => $start])); + $shortUrl->setTags($this->tagNamesToEntities($this->getEntityManager(), ['foo', 'bar'])); + $this->getEntityManager()->persist($shortUrl); + + $shortUrl2 = new ShortUrl('bar', ShortUrlMeta::fromRawData(['validUntil' => $end])); + $this->getEntityManager()->persist($shortUrl2); + + $shortUrl3 = new ShortUrl('baz', ShortUrlMeta::fromRawData(['validSince' => $start, 'validUntil' => $end])); + $this->getEntityManager()->persist($shortUrl3); + + $shortUrl4 = new ShortUrl('foo', ShortUrlMeta::fromRawData(['customSlug' => 'custom', 'validUntil' => $end])); + $this->getEntityManager()->persist($shortUrl4); + + $shortUrl5 = new ShortUrl('foo', ShortUrlMeta::fromRawData(['maxVisits' => 3])); + $this->getEntityManager()->persist($shortUrl5); + + $shortUrl6 = new ShortUrl('foo', ShortUrlMeta::fromRawData(['domain' => 'doma.in'])); + $this->getEntityManager()->persist($shortUrl6); + + $this->getEntityManager()->flush(); + + $this->assertSame( + $shortUrl, + $this->repo->findOneMatching('foo', ['foo', 'bar'], ShortUrlMeta::fromRawData(['validSince' => $start])), + ); + $this->assertSame( + $shortUrl2, + $this->repo->findOneMatching('bar', [], ShortUrlMeta::fromRawData(['validUntil' => $end])), + ); + $this->assertSame( + $shortUrl3, + $this->repo->findOneMatching('baz', [], ShortUrlMeta::fromRawData([ + 'validSince' => $start, + 'validUntil' => $end, + ])), + ); + $this->assertSame( + $shortUrl4, + $this->repo->findOneMatching('foo', [], ShortUrlMeta::fromRawData([ + 'customSlug' => 'custom', + 'validUntil' => $end, + ])), + ); + $this->assertSame( + $shortUrl5, + $this->repo->findOneMatching('foo', [], ShortUrlMeta::fromRawData(['maxVisits' => 3])), + ); + $this->assertSame( + $shortUrl6, + $this->repo->findOneMatching('foo', [], ShortUrlMeta::fromRawData(['domain' => 'doma.in'])), + ); + } + + /** @test */ + public function findOneMatchingReturnsOldestOneWhenThereAreMultipleMatches(): void + { + $start = Chronos::parse('2020-03-05 20:18:30'); + $meta = ['validSince' => $start, 'maxVisits' => 50]; + $tags = ['foo', 'bar']; + $tagEntities = $this->tagNamesToEntities($this->getEntityManager(), $tags); + + $shortUrl1 = new ShortUrl('foo', ShortUrlMeta::fromRawData($meta)); + $shortUrl1->setTags($tagEntities); + $this->getEntityManager()->persist($shortUrl1); + + $shortUrl2 = new ShortUrl('foo', ShortUrlMeta::fromRawData($meta)); + $shortUrl2->setTags($tagEntities); + $this->getEntityManager()->persist($shortUrl2); + + $shortUrl3 = new ShortUrl('foo', ShortUrlMeta::fromRawData($meta)); + $shortUrl3->setTags($tagEntities); + $this->getEntityManager()->persist($shortUrl3); + + $this->getEntityManager()->flush(); + + $this->assertSame( + $shortUrl1, + $this->repo->findOneMatching('foo', $tags, ShortUrlMeta::fromRawData($meta)), + ); + $this->assertNotSame( + $shortUrl2, + $this->repo->findOneMatching('foo', $tags, ShortUrlMeta::fromRawData($meta)), + ); + $this->assertNotSame( + $shortUrl3, + $this->repo->findOneMatching('foo', $tags, ShortUrlMeta::fromRawData($meta)), + ); + } } From 641f35ae0516334d0db4a16a48a939ea953ee9b7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 23 Sep 2020 07:46:25 +0200 Subject: [PATCH 3/4] Updated changelog --- CHANGELOG.md | 2 +- module/Core/src/Repository/ShortUrlRepository.php | 2 +- module/Core/test/Entity/ShortUrlTest.php | 1 - module/Core/test/Service/UrlShortenerTest.php | 2 -- 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62ad683b..dfd3ef05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Fixed -* *Nothing* +* [#837](https://github.com/shlinkio/shlink/issues/837) Drastically improved performance when creating a new shortUrl and providing `findIfExists = true`. ## 2.3.0 - 2020-08-09 diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 6147298e..e2cf578f 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -241,7 +241,7 @@ DQL; foreach ($tags as $index => $tag) { $alias = 't_' . $index; - $qb->join('s.tags', $alias, Join::WITH, $qb->expr()->eq($alias . '.name', ':tag' . $index)) + $qb->join('s.tags', $alias, Join::WITH, $alias . '.name = :tag' . $index) ->setParameter('tag' . $index, $tag); } diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php index c5933e39..e410dedb 100644 --- a/module/Core/test/Entity/ShortUrlTest.php +++ b/module/Core/test/Entity/ShortUrlTest.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Entity; -use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 9e33af6b..40ed6718 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -21,8 +21,6 @@ use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; -use function array_map; - class UrlShortenerTest extends TestCase { private UrlShortener $urlShortener; From aa0124f4e9940ea3ad7a2543424b17e2f5f6e6c0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 23 Sep 2020 07:49:59 +0200 Subject: [PATCH 4/4] Moved API tests back to composer ci command --- .travis.yml | 2 +- composer.json | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 29395d33..cd132c43 100644 --- a/.travis.yml +++ b/.travis.yml @@ -56,7 +56,7 @@ before_script: - export DOCKERFILE_CHANGED=$(git diff ${TRAVIS_COMMIT_RANGE:-origin/main} --name-only | grep Dockerfile) script: - - bin/test/run-api-tests.sh --coverage-php build/coverage-api.cov && composer ci + - composer ci - if [[ ! -z "${DOCKERFILE_CHANGED}" && "${TRAVIS_PHP_VERSION}" == "7.4" ]]; then docker build -t shlink-docker-image:temp . ; fi after_success: diff --git a/composer.json b/composer.json index f1a072fe..c37f378f 100644 --- a/composer.json +++ b/composer.json @@ -112,7 +112,8 @@ ], "test:ci": [ "@test:unit:ci", - "@test:db" + "@test:db", + "@test:api:ci" ], "test:unit": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-php build/coverage-unit.cov --testdox", "test:unit:ci": "@test:unit --coverage-clover=build/clover.xml --coverage-xml=build/coverage-unit/coverage-xml --log-junit=build/coverage-unit/junit.xml", @@ -130,6 +131,7 @@ "test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite", "test:db:ms": "DB_DRIVER=mssql composer test:db:sqlite", "test:api": "bin/test/run-api-tests.sh", + "test:api:ci": "bin/test/run-api-tests.sh --coverage-php build/coverage-api.cov", "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage", "infect": "infection --threads=4 --min-msi=80 --log-verbosity=default --only-covered", "infect:ci:base": "@infect --skip-initial-tests",