From 772494f46f4718a804e721ca8a7b96cef1ffc597 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 3 Feb 2019 08:17:22 +0100 Subject: [PATCH] Moved process of sluggifying custom slug to a filter --- config/autoload/slugify.global.php | 23 -------------- .../Common/src/Validation/SluggerFilter.php | 31 +++++++++++++++++++ module/Core/config/dependencies.config.php | 2 +- module/Core/src/Model/ShortUrlMeta.php | 8 ----- module/Core/src/Service/UrlShortener.php | 24 ++++---------- .../Validation/ShortUrlMetaInputFilter.php | 8 +++-- module/Core/test/Service/UrlShortenerTest.php | 29 +---------------- 7 files changed, 44 insertions(+), 81 deletions(-) delete mode 100644 config/autoload/slugify.global.php create mode 100644 module/Common/src/Validation/SluggerFilter.php diff --git a/config/autoload/slugify.global.php b/config/autoload/slugify.global.php deleted file mode 100644 index 1c3a3f96..00000000 --- a/config/autoload/slugify.global.php +++ /dev/null @@ -1,23 +0,0 @@ - [ - 'lowercase' => false, - ], - - 'dependencies' => [ - 'factories' => [ - Slugify::class => ConfigAbstractFactory::class, - ], - ], - - ConfigAbstractFactory::class => [ - Slugify::class => ['config.slugify_options'], - ], - -]; diff --git a/module/Common/src/Validation/SluggerFilter.php b/module/Common/src/Validation/SluggerFilter.php new file mode 100644 index 00000000..587695f9 --- /dev/null +++ b/module/Common/src/Validation/SluggerFilter.php @@ -0,0 +1,31 @@ +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; + } +} diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index bb068e65..a7a9d26b 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -46,7 +46,7 @@ return [ Options\NotFoundShortUrlOptions::class => ['config.url_shortener.not_found_short_url'], 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\ShortUrlService::class => ['em'], Service\VisitService::class => ['em'], diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 2e7c0208..84e7a1fa 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -150,12 +150,4 @@ final class ShortUrlMeta { return $this->findIfExists; } - - public function withCustomSlug(string $customSlug): self - { - $clone = clone $this; - $clone->customSlug = $customSlug; - - return $clone; - } } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index 96355c58..e6e63b50 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -3,7 +3,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; -use Cocur\Slugify\SlugifyInterface; use Doctrine\ORM\EntityManagerInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\GuzzleException; @@ -34,21 +33,14 @@ class UrlShortener implements UrlShortenerInterface private $httpClient; /** @var EntityManagerInterface */ private $em; - /** @var SlugifyInterface */ - private $slugger; /** @var UrlShortenerOptions */ private $options; - public function __construct( - ClientInterface $httpClient, - EntityManagerInterface $em, - UrlShortenerOptions $options, - SlugifyInterface $slugger - ) { + public function __construct(ClientInterface $httpClient, EntityManagerInterface $em, UrlShortenerOptions $options) + { $this->httpClient = $httpClient; $this->em = $em; $this->options = $options; - $this->slugger = $slugger; } /** @@ -63,7 +55,7 @@ class UrlShortener implements UrlShortenerInterface if ($this->options->isUrlValidationEnabled()) { $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 try { @@ -123,15 +115,13 @@ class UrlShortener implements UrlShortenerInterface return $chars[(int) $id] . $code; } - private function processCustomSlug(ShortUrlMeta $meta): ?ShortUrlMeta + private function verifyCustomSlug(ShortUrlMeta $meta): void { if (! $meta->hasCustomSlug()) { - return $meta; + return; } - // FIXME If the slug was generated while filtering the value originally, we would not need an immutable setter - // in ShortUrlMeta - $customSlug = $this->slugger->slugify($meta->getCustomSlug()); + $customSlug = $meta->getCustomSlug(); /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); @@ -139,8 +129,6 @@ class UrlShortener implements UrlShortenerInterface if ($shortUrlsCount > 0) { throw NonUniqueSlugException::fromSlug($customSlug); } - - return $meta->withCustomSlug($customSlug); } /** diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php index 34632d49..fad63f6c 100644 --- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php +++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php @@ -4,13 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Validation; use DateTime; -use Shlinkio\Shlink\Common\Validation\InputFactoryTrait; +use Shlinkio\Shlink\Common\Validation; use Zend\InputFilter\InputFilter; use Zend\Validator; class ShortUrlMetaInputFilter extends InputFilter { - use InputFactoryTrait; + use Validation\InputFactoryTrait; public const VALID_SINCE = 'validSince'; public const VALID_UNTIL = 'validUntil'; @@ -36,7 +36,9 @@ class ShortUrlMetaInputFilter extends InputFilter $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM])); $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->getValidatorChain()->attach(new Validator\Digits()) diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index fb625202..350775db 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -3,7 +3,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Service; -use Cocur\Slugify\SlugifyInterface; use Doctrine\DBAL\Connection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\ORMException; @@ -31,8 +30,6 @@ class UrlShortenerTest extends TestCase private $em; /** @var ObjectProphecy */ private $httpClient; - /** @var ObjectProphecy */ - private $slugger; public function setUp() { @@ -54,8 +51,6 @@ class UrlShortenerTest extends TestCase $repo->count(Argument::any())->willReturn(0); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->slugger = $this->prophesize(SlugifyInterface::class); - $this->setUrlShortener(false); } @@ -64,8 +59,7 @@ class UrlShortenerTest extends TestCase $this->urlShortener = new UrlShortener( $this->httpClient->reveal(), $this->em->reveal(), - new UrlShortenerOptions(['validate_url' => $urlValidationEnabled]), - $this->slugger->reveal() + new UrlShortenerOptions(['validate_url' => $urlValidationEnabled]) ); } @@ -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 */ public function exceptionIsThrownWhenNonUniqueSlugIsProvided() { - /** @var MethodProphecy $slugify */ - $slugify = $this->slugger->slugify('custom-slug')->willReturnArgument(); - $repo = $this->prophesize(ShortUrlRepository::class); $countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1); $repo->findOneBy(Argument::cetera())->willReturn(null); /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $slugify->shouldBeCalledOnce(); $countBySlug->shouldBeCalledOnce(); $getRepo->shouldBeCalled(); $this->expectException(NonUniqueSlugException::class);