From f078d95588d0db3ae98e395f7e23b546b1923b4d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 12 Apr 2023 09:15:36 +0200 Subject: [PATCH] Capture error on real-time update when creating short URL --- .../ShortUrl/CreateShortUrlCommand.php | 4 +- .../ShortUrl/CreateShortUrlCommandTest.php | 11 ++++-- .../ShortUrl/Model/UrlShorteningResult.php | 37 +++++++++++++++++++ module/Core/src/ShortUrl/UrlShortener.php | 18 +++++++-- .../src/ShortUrl/UrlShortenerInterface.php | 4 +- .../Core/test/ShortUrl/UrlShortenerTest.php | 6 +-- .../ShortUrl/AbstractCreateShortUrlAction.php | 4 +- .../ShortUrl/CreateShortUrlActionTest.php | 3 +- .../SingleStepCreateShortUrlActionTest.php | 3 +- 9 files changed, 71 insertions(+), 19 deletions(-) create mode 100644 module/Core/src/ShortUrl/Model/UrlShorteningResult.php diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 6fb1001b..fb5453cf 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -161,7 +161,7 @@ class CreateShortUrlCommand extends Command $doValidateUrl = $input->getOption('validate-url'); try { - $shortUrl = $this->urlShortener->shorten(ShortUrlCreation::fromRawData([ + $result = $this->urlShortener->shorten(ShortUrlCreation::fromRawData([ ShortUrlInputFilter::LONG_URL => $longUrl, ShortUrlInputFilter::VALID_SINCE => $input->getOption('valid-since'), ShortUrlInputFilter::VALID_UNTIL => $input->getOption('valid-until'), @@ -178,7 +178,7 @@ class CreateShortUrlCommand extends Command $io->writeln([ sprintf('Processed long URL: %s', $longUrl), - sprintf('Generated short URL: %s', $this->stringifier->stringify($shortUrl)), + sprintf('Generated short URL: %s', $this->stringifier->stringify($result->shortUrl)), ]); return ExitCodes::EXIT_SUCCESS; } catch (InvalidUrlException | NonUniqueSlugException $e) { diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index 48f740dc..f8b0e2d3 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Tester\CommandTester; @@ -51,7 +52,9 @@ class CreateShortUrlCommandTest extends TestCase public function properShortCodeIsCreatedIfLongUrlIsCorrect(): void { $shortUrl = ShortUrl::createFake(); - $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn($shortUrl); + $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn( + UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl), + ); $this->stringifier->expects($this->once())->method('stringify')->with($shortUrl)->willReturn( 'stringified_short_url', ); @@ -106,7 +109,7 @@ class CreateShortUrlCommandTest extends TestCase Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $creation->tags); return true; }), - )->willReturn($shortUrl); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl)); $this->stringifier->expects($this->once())->method('stringify')->with($shortUrl)->willReturn( 'stringified_short_url', ); @@ -129,7 +132,7 @@ class CreateShortUrlCommandTest extends TestCase Assert::assertEquals($expectedDomain, $meta->domain); return true; }), - )->willReturn(ShortUrl::createFake()); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching(ShortUrl::createFake())); $this->stringifier->method('stringify')->with($this->isInstanceOf(ShortUrl::class))->willReturn(''); $input['longUrl'] = 'http://domain.com/foo/bar'; @@ -155,7 +158,7 @@ class CreateShortUrlCommandTest extends TestCase Assert::assertEquals($expectedValidateUrl, $meta->doValidateUrl()); return true; }), - )->willReturn($shortUrl); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl)); $this->stringifier->method('stringify')->with($this->isInstanceOf(ShortUrl::class))->willReturn(''); $options['longUrl'] = 'http://domain.com/foo/bar'; diff --git a/module/Core/src/ShortUrl/Model/UrlShorteningResult.php b/module/Core/src/ShortUrl/Model/UrlShorteningResult.php new file mode 100644 index 00000000..c6a95739 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/UrlShorteningResult.php @@ -0,0 +1,37 @@ +errorOnEventDispatching !== null) { + $handler($this->errorOnEventDispatching); + } + } + + public static function withoutErrorOnEventDispatching(ShortUrl $shortUrl): self + { + return new self($shortUrl, null); + } + + public static function withErrorOnEventDispatching(ShortUrl $shortUrl, Throwable $errorOnEventDispatching): self + { + return new self($shortUrl, $errorOnEventDispatching); + } +} diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index 7477052f..7bb74ba6 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; use Doctrine\ORM\EntityManagerInterface; +use Psr\Container\ContainerExceptionInterface; use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; @@ -13,6 +14,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; @@ -31,12 +33,12 @@ class UrlShortener implements UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $creation): ShortUrl + public function shorten(ShortUrlCreation $creation): UrlShorteningResult { // First, check if a short URL exists for all provided params $existingShortUrl = $this->findExistingShortUrlIfExists($creation); if ($existingShortUrl !== null) { - return $existingShortUrl; + return UrlShorteningResult::withoutErrorOnEventDispatching($existingShortUrl); } $creation = $this->titleResolutionHelper->processTitleAndValidateUrl($creation); @@ -51,9 +53,17 @@ class UrlShortener implements UrlShortenerInterface return $shortUrl; }); - $this->eventDispatcher->dispatch(new ShortUrlCreated($newShortUrl->getId())); + try { + $this->eventDispatcher->dispatch(new ShortUrlCreated($newShortUrl->getId())); + } catch (ContainerExceptionInterface $e) { + // Ignore container errors when dispatching the event. + // When using openswoole, this event will try to enqueue a task, which cannot be done outside an HTTP + // request. + // If the short URL is created from CLI, the event dispatching will fail. + return UrlShorteningResult::withErrorOnEventDispatching($newShortUrl, $e); + } - return $newShortUrl; + return UrlShorteningResult::withoutErrorOnEventDispatching($newShortUrl); } private function findExistingShortUrlIfExists(ShortUrlCreation $creation): ?ShortUrl diff --git a/module/Core/src/ShortUrl/UrlShortenerInterface.php b/module/Core/src/ShortUrl/UrlShortenerInterface.php index d7ae45f0..70896ec1 100644 --- a/module/Core/src/ShortUrl/UrlShortenerInterface.php +++ b/module/Core/src/ShortUrl/UrlShortenerInterface.php @@ -6,8 +6,8 @@ namespace Shlinkio\Shlink\Core\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; interface UrlShortenerInterface { @@ -15,5 +15,5 @@ interface UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $creation): ShortUrl; + public function shorten(ShortUrlCreation $creation): UrlShorteningResult; } diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php index 713fee95..247e6185 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -58,9 +58,9 @@ class UrlShortenerTest extends TestCase )->willReturnArgument(0); $this->shortCodeHelper->method('ensureShortCodeUniqueness')->willReturn(true); - $shortUrl = $this->urlShortener->shorten($meta); + $result = $this->urlShortener->shorten($meta); - self::assertEquals($longUrl, $shortUrl->getLongUrl()); + self::assertEquals($longUrl, $result->shortUrl->getLongUrl()); } #[Test] @@ -91,7 +91,7 @@ class UrlShortenerTest extends TestCase $result = $this->urlShortener->shorten($meta); - self::assertSame($expected, $result); + self::assertSame($expected, $result->shortUrl); } public static function provideExistingShortUrls(): iterable diff --git a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php index 288033ed..914efde5 100644 --- a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php @@ -26,9 +26,9 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction public function handle(Request $request): Response { $shortUrlMeta = $this->buildShortUrlData($request); - $shortUrl = $this->urlShortener->shorten($shortUrlMeta); + $result = $this->urlShortener->shorten($shortUrlMeta); - return new JsonResponse($this->transformer->transform($shortUrl)); + return new JsonResponse($this->transformer->transform($result->shortUrl)); } /** diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index acaf8df0..c79eab86 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -53,7 +54,7 @@ class CreateShortUrlActionTest extends TestCase $this->urlShortener->expects($this->once())->method('shorten')->with( ShortUrlCreation::fromRawData($expectedMeta), - )->willReturn($shortUrl); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl)); $this->transformer->expects($this->once())->method('transform')->with($shortUrl)->willReturn( ['shortUrl' => 'stringified_short_url'], ); diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index d343a3cd..c1ac6544 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -44,7 +45,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase ])->withAttribute(ApiKey::class, $apiKey); $this->urlShortener->expects($this->once())->method('shorten')->with( ShortUrlCreation::fromRawData(['apiKey' => $apiKey, 'longUrl' => 'http://foobar.com']), - )->willReturn(ShortUrl::createFake()); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching(ShortUrl::createFake())); $resp = $this->action->handle($request);