Moved process of sluggifying custom slug to a filter

This commit is contained in:
Alejandro Celaya 2019-02-03 08:17:22 +01:00
parent 594e7da256
commit 772494f46f
7 changed files with 44 additions and 81 deletions

View file

@ -1,23 +0,0 @@
<?php
declare(strict_types=1);
use Cocur\Slugify\Slugify;
use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
return [
'slugify_options' => [
'lowercase' => false,
],
'dependencies' => [
'factories' => [
Slugify::class => ConfigAbstractFactory::class,
],
],
ConfigAbstractFactory::class => [
Slugify::class => ['config.slugify_options'],
],
];

View file

@ -0,0 +1,31 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Common\Validation;
use Cocur\Slugify;
use Zend\Filter\Exception;
use Zend\Filter\FilterInterface;
class SluggerFilter implements FilterInterface
{
/** @var Slugify\SlugifyInterface */
private $slugger;
public function __construct(?Slugify\SlugifyInterface $slugger = null)
{
$this->slugger = $slugger ?: new Slugify\Slugify(['lowercase' => false]);
}
/**
* Returns the result of filtering $value
*
* @param mixed $value
* @throws Exception\RuntimeException If filtering $value is impossible
* @return mixed
*/
public function filter($value)
{
return $value ? $this->slugger->slugify($value) : $value;
}
}

View file

@ -46,7 +46,7 @@ return [
Options\NotFoundShortUrlOptions::class => ['config.url_shortener.not_found_short_url'], Options\NotFoundShortUrlOptions::class => ['config.url_shortener.not_found_short_url'],
Options\UrlShortenerOptions::class => ['config.url_shortener'], Options\UrlShortenerOptions::class => ['config.url_shortener'],
Service\UrlShortener::class => ['httpClient', 'em', Options\UrlShortenerOptions::class, Slugify::class], Service\UrlShortener::class => ['httpClient', 'em', Options\UrlShortenerOptions::class],
Service\VisitsTracker::class => ['em'], Service\VisitsTracker::class => ['em'],
Service\ShortUrlService::class => ['em'], Service\ShortUrlService::class => ['em'],
Service\VisitService::class => ['em'], Service\VisitService::class => ['em'],

View file

@ -150,12 +150,4 @@ final class ShortUrlMeta
{ {
return $this->findIfExists; return $this->findIfExists;
} }
public function withCustomSlug(string $customSlug): self
{
$clone = clone $this;
$clone->customSlug = $customSlug;
return $clone;
}
} }

View file

@ -3,7 +3,6 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Service; namespace Shlinkio\Shlink\Core\Service;
use Cocur\Slugify\SlugifyInterface;
use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityManagerInterface;
use GuzzleHttp\ClientInterface; use GuzzleHttp\ClientInterface;
use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\Exception\GuzzleException;
@ -34,21 +33,14 @@ class UrlShortener implements UrlShortenerInterface
private $httpClient; private $httpClient;
/** @var EntityManagerInterface */ /** @var EntityManagerInterface */
private $em; private $em;
/** @var SlugifyInterface */
private $slugger;
/** @var UrlShortenerOptions */ /** @var UrlShortenerOptions */
private $options; private $options;
public function __construct( public function __construct(ClientInterface $httpClient, EntityManagerInterface $em, UrlShortenerOptions $options)
ClientInterface $httpClient, {
EntityManagerInterface $em,
UrlShortenerOptions $options,
SlugifyInterface $slugger
) {
$this->httpClient = $httpClient; $this->httpClient = $httpClient;
$this->em = $em; $this->em = $em;
$this->options = $options; $this->options = $options;
$this->slugger = $slugger;
} }
/** /**
@ -63,7 +55,7 @@ class UrlShortener implements UrlShortenerInterface
if ($this->options->isUrlValidationEnabled()) { if ($this->options->isUrlValidationEnabled()) {
$this->checkUrlExists($url); $this->checkUrlExists($url);
} }
$meta = $this->processCustomSlug($meta); $this->verifyCustomSlug($meta);
// Transactionally insert the short url, then generate the short code and finally update the short code // Transactionally insert the short url, then generate the short code and finally update the short code
try { try {
@ -123,15 +115,13 @@ class UrlShortener implements UrlShortenerInterface
return $chars[(int) $id] . $code; return $chars[(int) $id] . $code;
} }
private function processCustomSlug(ShortUrlMeta $meta): ?ShortUrlMeta private function verifyCustomSlug(ShortUrlMeta $meta): void
{ {
if (! $meta->hasCustomSlug()) { if (! $meta->hasCustomSlug()) {
return $meta; return;
} }
// FIXME If the slug was generated while filtering the value originally, we would not need an immutable setter $customSlug = $meta->getCustomSlug();
// in ShortUrlMeta
$customSlug = $this->slugger->slugify($meta->getCustomSlug());
/** @var ShortUrlRepository $repo */ /** @var ShortUrlRepository $repo */
$repo = $this->em->getRepository(ShortUrl::class); $repo = $this->em->getRepository(ShortUrl::class);
@ -139,8 +129,6 @@ class UrlShortener implements UrlShortenerInterface
if ($shortUrlsCount > 0) { if ($shortUrlsCount > 0) {
throw NonUniqueSlugException::fromSlug($customSlug); throw NonUniqueSlugException::fromSlug($customSlug);
} }
return $meta->withCustomSlug($customSlug);
} }
/** /**

View file

@ -4,13 +4,13 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Validation; namespace Shlinkio\Shlink\Core\Validation;
use DateTime; use DateTime;
use Shlinkio\Shlink\Common\Validation\InputFactoryTrait; use Shlinkio\Shlink\Common\Validation;
use Zend\InputFilter\InputFilter; use Zend\InputFilter\InputFilter;
use Zend\Validator; use Zend\Validator;
class ShortUrlMetaInputFilter extends InputFilter class ShortUrlMetaInputFilter extends InputFilter
{ {
use InputFactoryTrait; use Validation\InputFactoryTrait;
public const VALID_SINCE = 'validSince'; public const VALID_SINCE = 'validSince';
public const VALID_UNTIL = 'validUntil'; public const VALID_UNTIL = 'validUntil';
@ -36,7 +36,9 @@ class ShortUrlMetaInputFilter extends InputFilter
$validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM])); $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM]));
$this->add($validUntil); $this->add($validUntil);
$this->add($this->createInput(self::CUSTOM_SLUG, false)); $customSlug = $this->createInput(self::CUSTOM_SLUG, false);
$customSlug->getFilterChain()->attach(new Validation\SluggerFilter());
$this->add($customSlug);
$maxVisits = $this->createInput(self::MAX_VISITS, false); $maxVisits = $this->createInput(self::MAX_VISITS, false);
$maxVisits->getValidatorChain()->attach(new Validator\Digits()) $maxVisits->getValidatorChain()->attach(new Validator\Digits())

View file

@ -3,7 +3,6 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Service; namespace ShlinkioTest\Shlink\Core\Service;
use Cocur\Slugify\SlugifyInterface;
use Doctrine\DBAL\Connection; use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\ORMException; use Doctrine\ORM\ORMException;
@ -31,8 +30,6 @@ class UrlShortenerTest extends TestCase
private $em; private $em;
/** @var ObjectProphecy */ /** @var ObjectProphecy */
private $httpClient; private $httpClient;
/** @var ObjectProphecy */
private $slugger;
public function setUp() public function setUp()
{ {
@ -54,8 +51,6 @@ class UrlShortenerTest extends TestCase
$repo->count(Argument::any())->willReturn(0); $repo->count(Argument::any())->willReturn(0);
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$this->slugger = $this->prophesize(SlugifyInterface::class);
$this->setUrlShortener(false); $this->setUrlShortener(false);
} }
@ -64,8 +59,7 @@ class UrlShortenerTest extends TestCase
$this->urlShortener = new UrlShortener( $this->urlShortener = new UrlShortener(
$this->httpClient->reveal(), $this->httpClient->reveal(),
$this->em->reveal(), $this->em->reveal(),
new UrlShortenerOptions(['validate_url' => $urlValidationEnabled]), new UrlShortenerOptions(['validate_url' => $urlValidationEnabled])
$this->slugger->reveal()
); );
} }
@ -121,38 +115,17 @@ class UrlShortenerTest extends TestCase
); );
} }
/**
* @test
*/
public function whenCustomSlugIsProvidedItIsUsed()
{
/** @var MethodProphecy $slugify */
$slugify = $this->slugger->slugify('custom-slug')->willReturnArgument();
$this->urlShortener->urlToShortCode(
new Uri('http://foobar.com/12345/hello?foo=bar'),
[],
ShortUrlMeta::createFromRawData(['customSlug' => 'custom-slug'])
);
$slugify->shouldHaveBeenCalledOnce();
}
/** /**
* @test * @test
*/ */
public function exceptionIsThrownWhenNonUniqueSlugIsProvided() public function exceptionIsThrownWhenNonUniqueSlugIsProvided()
{ {
/** @var MethodProphecy $slugify */
$slugify = $this->slugger->slugify('custom-slug')->willReturnArgument();
$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->findOneBy(Argument::cetera())->willReturn(null);
/** @var MethodProphecy $getRepo */ /** @var MethodProphecy $getRepo */
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$slugify->shouldBeCalledOnce();
$countBySlug->shouldBeCalledOnce(); $countBySlug->shouldBeCalledOnce();
$getRepo->shouldBeCalled(); $getRepo->shouldBeCalled();
$this->expectException(NonUniqueSlugException::class); $this->expectException(NonUniqueSlugException::class);