diff --git a/module/Common/src/Validation/SluggerFilter.php b/module/Common/src/Validation/SluggerFilter.php index 587695f9..9387e85a 100644 --- a/module/Common/src/Validation/SluggerFilter.php +++ b/module/Common/src/Validation/SluggerFilter.php @@ -26,6 +26,6 @@ class SluggerFilter implements FilterInterface */ public function filter($value) { - return $value ? $this->slugger->slugify($value) : $value; + return ! empty($value) ? $this->slugger->slugify($value) : null; } } diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index a7a9d26b..960b74ac 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -3,7 +3,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; -use Cocur\Slugify\Slugify; use Doctrine\Common\Cache\Cache; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Response\NotFoundHandler; diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php index 6a94b548..4291f0ec 100644 --- a/module/Core/src/Exception/InvalidUrlException.php +++ b/module/Core/src/Exception/InvalidUrlException.php @@ -8,7 +8,7 @@ use function sprintf; class InvalidUrlException extends RuntimeException { - public static function fromUrl($url, Throwable $previous = null) + public static function fromUrl(string $url, Throwable $previous = null) { $code = $previous !== null ? $previous->getCode() : -1; return new static(sprintf('Provided URL "%s" is not an existing and valid URL', $url), $code, $previous); diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 84e7a1fa..2eb6ebca 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Core\Model; use Cake\Chronos\Chronos; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; -use function is_string; final class ShortUrlMeta { @@ -18,7 +17,7 @@ final class ShortUrlMeta private $customSlug; /** @var int|null */ private $maxVisits; - /** @var bool */ + /** @var bool|null */ private $findIfExists; // Force named constructors @@ -86,12 +85,11 @@ final class ShortUrlMeta $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG); $this->maxVisits = $inputFilter->getValue(ShortUrlMetaInputFilter::MAX_VISITS); $this->maxVisits = $this->maxVisits !== null ? (int) $this->maxVisits : null; - $this->findIfExists = (bool) $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS); + $this->findIfExists = $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS); } /** * @param string|Chronos|null $date - * @return Chronos|null */ private function parseDateField($date): ?Chronos { @@ -99,11 +97,7 @@ final class ShortUrlMeta return $date; } - if (is_string($date)) { - return Chronos::parse($date); - } - - return null; + return Chronos::parse($date); } public function getValidSince(): ?Chronos @@ -148,6 +142,6 @@ final class ShortUrlMeta public function findIfExists(): bool { - return $this->findIfExists; + return (bool) $this->findIfExists; } } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index e6e63b50..f224ab51 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -4,8 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM\EntityManagerInterface; +use Fig\Http\Message\RequestMethodInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\GuzzleException; +use GuzzleHttp\RequestOptions; use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; @@ -18,8 +20,12 @@ use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Throwable; +use function array_reduce; +use function count; use function floor; use function fmod; +use function Functional\contains; +use function Functional\invoke; use function preg_match; use function strlen; @@ -51,6 +57,14 @@ class UrlShortener implements UrlShortenerInterface */ public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl { + $url = (string) $url; + + // First, check if a short URL exists for all provided params + $existingShortUrl = $this->findExistingShortUrlIfExists($url, $tags, $meta); + if ($existingShortUrl !== null) { + return $existingShortUrl; + } + // If the URL validation is enabled, check that the URL actually exists if ($this->options->isUrlValidationEnabled()) { $this->checkUrlExists($url); @@ -62,7 +76,7 @@ class UrlShortener implements UrlShortenerInterface $this->em->beginTransaction(); // First, create the short URL with an empty short code - $shortUrl = new ShortUrl((string) $url, $meta); + $shortUrl = new ShortUrl($url, $meta); $this->em->persist($shortUrl); $this->em->flush(); @@ -87,17 +101,71 @@ class UrlShortener implements UrlShortenerInterface } } - private function checkUrlExists(UriInterface $url): void + private function findExistingShortUrlIfExists(string $url, array $tags, ShortUrlMeta $meta): ?ShortUrl + { + if (! $meta->findIfExists()) { + return null; + } + + $criteria = ['longUrl' => $url]; + if ($meta->hasCustomSlug()) { + $criteria['shortCode'] = $meta->getCustomSlug(); + } + /** @var ShortUrl|null $shortUrl */ + $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy($criteria); + if ($shortUrl === null) { + 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; + } + + $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 { try { - $this->httpClient->request('GET', $url, ['allow_redirects' => [ - 'max' => 15, - ]]); + $this->httpClient->request(RequestMethodInterface::METHOD_GET, $url, [ + RequestOptions::ALLOW_REDIRECTS => ['max' => 15], + ]); } catch (GuzzleException $e) { throw InvalidUrlException::fromUrl($url, $e); } } + private function verifyCustomSlug(ShortUrlMeta $meta): void + { + if (! $meta->hasCustomSlug()) { + return; + } + + $customSlug = $meta->getCustomSlug(); + + /** @var ShortUrlRepository $repo */ + $repo = $this->em->getRepository(ShortUrl::class); + $shortUrlsCount = $repo->count(['shortCode' => $customSlug]); + if ($shortUrlsCount > 0) { + throw NonUniqueSlugException::fromSlug($customSlug); + } + } + private function convertAutoincrementIdToShortCode(float $id): string { $id += self::ID_INCREMENT; // Increment the Id so that the generated shortcode is not too short @@ -115,22 +183,6 @@ class UrlShortener implements UrlShortenerInterface return $chars[(int) $id] . $code; } - private function verifyCustomSlug(ShortUrlMeta $meta): void - { - if (! $meta->hasCustomSlug()) { - return; - } - - $customSlug = $meta->getCustomSlug(); - - /** @var ShortUrlRepository $repo */ - $repo = $this->em->getRepository(ShortUrl::class); - $shortUrlsCount = $repo->count(['shortCode' => $customSlug]); - if ($shortUrlsCount > 0) { - throw NonUniqueSlugException::fromSlug($customSlug); - } - } - /** * @throws InvalidShortCodeException * @throws EntityDoesNotExistException diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 350775db..48dae28e 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Service; +use Cake\Chronos\Chronos; +use Doctrine\Common\Collections\ArrayCollection; use Doctrine\DBAL\Connection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\ORMException; @@ -14,6 +16,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; @@ -31,7 +34,7 @@ class UrlShortenerTest extends TestCase /** @var ObjectProphecy */ private $httpClient; - public function setUp() + public function setUp(): void { $this->httpClient = $this->prophesize(ClientInterface::class); @@ -54,7 +57,7 @@ class UrlShortenerTest extends TestCase $this->setUrlShortener(false); } - public function setUrlShortener(bool $urlValidationEnabled) + public function setUrlShortener(bool $urlValidationEnabled): void { $this->urlShortener = new UrlShortener( $this->httpClient->reveal(), @@ -66,7 +69,7 @@ class UrlShortenerTest extends TestCase /** * @test */ - public function urlIsProperlyShortened() + public function urlIsProperlyShortened(): void { // 10 -> 12C1c $shortUrl = $this->urlShortener->urlToShortCode( @@ -81,7 +84,7 @@ class UrlShortenerTest extends TestCase * @test * @expectedException \Shlinkio\Shlink\Core\Exception\RuntimeException */ - public function exceptionIsThrownWhenOrmThrowsException() + public function exceptionIsThrownWhenOrmThrowsException(): void { $conn = $this->prophesize(Connection::class); $conn->isTransactionActive()->willReturn(true); @@ -101,7 +104,7 @@ class UrlShortenerTest extends TestCase * @test * @expectedException \Shlinkio\Shlink\Core\Exception\InvalidUrlException */ - public function exceptionIsThrownWhenUrlDoesNotExist() + public function exceptionIsThrownWhenUrlDoesNotExist(): void { $this->setUrlShortener(true); @@ -118,7 +121,7 @@ class UrlShortenerTest extends TestCase /** * @test */ - public function exceptionIsThrownWhenNonUniqueSlugIsProvided() + public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void { $repo = $this->prophesize(ShortUrlRepository::class); $countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1); @@ -139,8 +142,78 @@ class UrlShortenerTest extends TestCase /** * @test + * @dataProvider provideExsitingShortUrls */ - public function shortCodeIsProperlyParsed() + public function existingShortUrlIsReturnedWhenRequested( + string $url, + array $tags, + ShortUrlMeta $meta, + ?ShortUrl $expected + ): void { + $repo = $this->prophesize(ShortUrlRepository::class); + $findExisting = $repo->findOneBy(Argument::any())->willReturn($expected); + $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + + $result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta); + + $this->assertSame($expected, $result); + $findExisting->shouldHaveBeenCalledOnce(); + $getRepo->shouldHaveBeenCalledOnce(); + } + + public function provideExsitingShortUrls(): array + { + $url = 'http://foo.com'; + + return [ + [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), new ShortUrl($url)], + [$url, [], ShortUrlMeta::createFromRawData( + ['findIfExists' => true, 'customSlug' => 'foo'] + ), new ShortUrl($url)], + [ + $url, + ['foo', 'bar'], + ShortUrlMeta::createFromRawData(['findIfExists' => true]), + (new ShortUrl($url))->setTags(new ArrayCollection([new Tag('bar'), new Tag('foo')])), + ], + [ + $url, + [], + ShortUrlMeta::createFromRawData(['findIfExists' => true, 'maxVisits' => 3]), + new ShortUrl($url, ShortUrlMeta::createFromRawData(['maxVisits' => 3])), + ], + [ + $url, + [], + ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validSince' => Chronos::parse('2017-01-01')]), + new ShortUrl($url, ShortUrlMeta::createFromRawData(['validSince' => Chronos::parse('2017-01-01')])), + ], + [ + $url, + [], + ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01')]), + new ShortUrl($url, ShortUrlMeta::createFromRawData(['validUntil' => Chronos::parse('2017-01-01')])), + ], + [ + $url, + ['baz', 'foo', 'bar'], + ShortUrlMeta::createFromRawData([ + 'findIfExists' => true, + 'validUntil' => Chronos::parse('2017-01-01'), + 'maxVisits' => 4, + ]), + (new ShortUrl($url, ShortUrlMeta::createFromRawData([ + 'validUntil' => Chronos::parse('2017-01-01'), + 'maxVisits' => 4, + ])))->setTags(new ArrayCollection([new Tag('foo'), new Tag('bar'), new Tag('baz')])), + ], + ]; + } + + /** + * @test + */ + public function shortCodeIsProperlyParsed(): void { // 12C1c -> 10 $shortCode = '12C1c'; @@ -159,7 +232,7 @@ class UrlShortenerTest extends TestCase * @test * @expectedException \Shlinkio\Shlink\Core\Exception\InvalidShortCodeException */ - public function invalidCharSetThrowsException() + public function invalidCharSetThrowsException(): void { $this->urlShortener->shortCodeToUrl('&/('); }