Updated how existing short URLs are checked, so that not only the first one matching the slug or url is checked

This commit is contained in:
Alejandro Celaya 2019-03-30 07:36:57 +01:00
parent 0135f205df
commit 2906d42f97
2 changed files with 30 additions and 23 deletions

View File

@ -112,32 +112,39 @@ class UrlShortener implements UrlShortenerInterface
if ($meta->hasCustomSlug()) { if ($meta->hasCustomSlug()) {
$criteria['shortCode'] = $meta->getCustomSlug(); $criteria['shortCode'] = $meta->getCustomSlug();
} }
/** @var ShortUrl|null $shortUrl */ /** @var ShortUrl[] $shortUrls */
$shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy($criteria); $shortUrls = $this->em->getRepository(ShortUrl::class)->findBy($criteria);
if ($shortUrl === null) { if (empty($shortUrls)) {
return null; return null;
} }
if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) { // Iterate short URLs until one that matches is found, or return null otherwise
return null; return array_reduce($shortUrls, function (?ShortUrl $found, ShortUrl $shortUrl) use ($tags, $meta) {
} if ($found) {
if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) { return $found;
return null; }
}
if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) {
return null;
}
$shortUrlTags = invoke($shortUrl->getTags(), '__toString'); if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) {
$hasAllTags = count($shortUrlTags) === count($tags) && array_reduce( return null;
$tags, }
function (bool $hasAllTags, string $tag) use ($shortUrlTags) { if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) {
return $hasAllTags && contains($shortUrlTags, $tag); return null;
}, }
true 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 private function checkUrlExists(string $url): void

View File

@ -121,7 +121,7 @@ class UrlShortenerTest extends TestCase
{ {
$repo = $this->prophesize(ShortUrlRepository::class); $repo = $this->prophesize(ShortUrlRepository::class);
$countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1); $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()); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$countBySlug->shouldBeCalledOnce(); $countBySlug->shouldBeCalledOnce();
@ -146,7 +146,7 @@ class UrlShortenerTest extends TestCase
?ShortUrl $expected ?ShortUrl $expected
): void { ): void {
$repo = $this->prophesize(ShortUrlRepository::class); $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()); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta); $result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta);