From 5432eb7b776df51645f330a350b89e3507884bf2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Mar 2020 17:22:52 +0100 Subject: [PATCH] Added URL validation to ShortUrl edition, as long URL can now be edited --- module/Core/config/dependencies.config.php | 2 +- module/Core/src/Service/ShortUrlService.php | 16 +++++- .../src/Service/ShortUrlServiceInterface.php | 2 + .../Core/test/Service/ShortUrlServiceTest.php | 55 ++++++++++++++----- 4 files changed, 58 insertions(+), 17 deletions(-) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 6f33587a..2800cdd6 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -56,7 +56,7 @@ return [ Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class], Service\VisitsTracker::class => ['em', EventDispatcherInterface::class], - Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class], + Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], Service\VisitService::class => ['em'], Service\Tag\TagService::class => ['em'], Service\ShortUrl\DeleteShortUrlService::class => [ diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index ae0cc7a3..5cdab93d 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM; use Laminas\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; @@ -15,6 +16,7 @@ use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\TagManagerTrait; +use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; class ShortUrlService implements ShortUrlServiceInterface { @@ -22,11 +24,16 @@ class ShortUrlService implements ShortUrlServiceInterface private ORM\EntityManagerInterface $em; private ShortUrlResolverInterface $urlResolver; + private UrlValidatorInterface $urlValidator; - public function __construct(ORM\EntityManagerInterface $em, ShortUrlResolverInterface $urlResolver) - { + public function __construct( + ORM\EntityManagerInterface $em, + ShortUrlResolverInterface $urlResolver, + UrlValidatorInterface $urlValidator + ) { $this->em = $em; $this->urlResolver = $urlResolver; + $this->urlValidator = $urlValidator; } /** @@ -59,9 +66,14 @@ class ShortUrlService implements ShortUrlServiceInterface /** * @throws ShortUrlNotFoundException + * @throws InvalidUrlException */ public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl { + if ($shortUrlEdit->hasLongUrl()) { + $this->urlValidator->validateUrl($shortUrlEdit->longUrl()); + } + $shortUrl = $this->urlResolver->resolveShortUrl($identifier); $shortUrl->update($shortUrlEdit); diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index f17a7bea..3c09e7e9 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Service; use Laminas\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; @@ -26,6 +27,7 @@ interface ShortUrlServiceInterface /** * @throws ShortUrlNotFoundException + * @throws InvalidUrlException */ public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl; } diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 4f40683e..9becdf8b 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\ShortUrlService; +use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; use function count; @@ -26,6 +27,7 @@ class ShortUrlServiceTest extends TestCase private ShortUrlService $service; private ObjectProphecy $em; private ObjectProphecy $urlResolver; + private ObjectProphecy $urlValidator; public function setUp(): void { @@ -34,8 +36,13 @@ class ShortUrlServiceTest extends TestCase $this->em->flush()->willReturn(null); $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + $this->urlValidator = $this->prophesize(UrlValidatorInterface::class); - $this->service = new ShortUrlService($this->em->reveal(), $this->urlResolver->reveal()); + $this->service = new ShortUrlService( + $this->em->reveal(), + $this->urlResolver->reveal(), + $this->urlValidator->reveal(), + ); } /** @test */ @@ -74,27 +81,47 @@ class ShortUrlServiceTest extends TestCase $this->service->setTagsByShortCode(new ShortUrlIdentifier($shortCode), ['foo', 'bar']); } - /** @test */ - public function updateMetadataByShortCodeUpdatesProvidedData(): void - { - $shortUrl = new ShortUrl(''); + /** + * @test + * @dataProvider provideShortUrlEdits + */ + public function updateMetadataByShortCodeUpdatesProvidedData( + int $expectedValidateCalls, + ShortUrlEdit $shortUrlEdit + ): void { + $originalLongUrl = 'originalLongUrl'; + $shortUrl = new ShortUrl($originalLongUrl); $findShortUrl = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier('abc123'))->willReturn($shortUrl); $flush = $this->em->flush()->willReturn(null); - $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), ShortUrlEdit::fromRawData( + $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), $shortUrlEdit); + + $this->assertSame($shortUrl, $result); + $this->assertEquals($shortUrlEdit->validSince(), $shortUrl->getValidSince()); + $this->assertEquals($shortUrlEdit->validUntil(), $shortUrl->getValidUntil()); + $this->assertEquals($shortUrlEdit->maxVisits(), $shortUrl->getMaxVisits()); + $this->assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl()); + $findShortUrl->shouldHaveBeenCalled(); + $flush->shouldHaveBeenCalled(); + $this->urlValidator->validateUrl($shortUrlEdit->longUrl())->shouldHaveBeenCalledTimes($expectedValidateCalls); + } + + public function provideShortUrlEdits(): iterable + { + yield 'no long URL' => [0, ShortUrlEdit::fromRawData( [ 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(), 'maxVisits' => 5, ], - )); - - $this->assertSame($shortUrl, $result); - $this->assertEquals(Chronos::parse('2017-01-01 00:00:00'), $shortUrl->getValidSince()); - $this->assertEquals(Chronos::parse('2017-01-05 00:00:00'), $shortUrl->getValidUntil()); - $this->assertEquals(5, $shortUrl->getMaxVisits()); - $findShortUrl->shouldHaveBeenCalled(); - $flush->shouldHaveBeenCalled(); + )]; + yield 'long URL' => [1, ShortUrlEdit::fromRawData( + [ + 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), + 'maxVisits' => 10, + 'longUrl' => 'modifiedLongUrl', + ], + )]; } }