Implemented feature to optionally return an existing short url when all provided params match an existing one

This commit is contained in:
Alejandro Celaya 2019-02-03 09:40:32 +01:00
parent 772494f46f
commit c4fd8d5120
6 changed files with 160 additions and 42 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -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('&/(');
}