From 2906d42f978854a980acb3e95f0f9768dbe1eb31 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 30 Mar 2019 07:36:57 +0100 Subject: [PATCH] Updated how existing short URLs are checked, so that not only the first one matching the slug or url is checked --- module/Core/src/Service/UrlShortener.php | 49 +++++++++++-------- module/Core/test/Service/UrlShortenerTest.php | 4 +- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 687a21fd..fe041900 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -112,32 +112,39 @@ class UrlShortener implements UrlShortenerInterface if ($meta->hasCustomSlug()) { $criteria['shortCode'] = $meta->getCustomSlug(); } - /** @var ShortUrl|null $shortUrl */ - $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy($criteria); - if ($shortUrl === null) { + /** @var ShortUrl[] $shortUrls */ + $shortUrls = $this->em->getRepository(ShortUrl::class)->findBy($criteria); + if (empty($shortUrls)) { return null; } - if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) { - return null; - } - if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) { - return null; - } - if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) { - 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) { + return $found; + } - $shortUrlTags = invoke($shortUrl->getTags(), '__toString'); - $hasAllTags = count($shortUrlTags) === count($tags) && array_reduce( - $tags, - function (bool $hasAllTags, string $tag) use ($shortUrlTags) { - return $hasAllTags && contains($shortUrlTags, $tag); - }, - true - ); + if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) { + return null; + } + if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) { + return null; + } + if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) { + return null; + } - return $hasAllTags ? $shortUrl : null; + $shortUrlTags = invoke($shortUrl->getTags(), '__toString'); + $hasAllTags = count($shortUrlTags) === count($tags) && array_reduce( + $tags, + function (bool $hasAllTags, string $tag) use ($shortUrlTags) { + return $hasAllTags && contains($shortUrlTags, $tag); + }, + true + ); + + return $hasAllTags ? $shortUrl : null; + }); } private function checkUrlExists(string $url): void diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index bc91dcb8..967e19b9 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -121,7 +121,7 @@ class UrlShortenerTest extends TestCase { $repo = $this->prophesize(ShortUrlRepository::class); $countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1); - $repo->findOneBy(Argument::cetera())->willReturn(null); + $repo->findBy(Argument::cetera())->willReturn([]); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $countBySlug->shouldBeCalledOnce(); @@ -146,7 +146,7 @@ class UrlShortenerTest extends TestCase ?ShortUrl $expected ): void { $repo = $this->prophesize(ShortUrlRepository::class); - $findExisting = $repo->findOneBy(Argument::any())->willReturn($expected); + $findExisting = $repo->findBy(Argument::any())->willReturn([$expected]); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta);