From e36c4d397cf97418bbcca2fa23da66e991d987fa Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 27 Jul 2022 18:18:36 +0200 Subject: [PATCH] Moved duplicated code in visit listeners to an abstract class --- .../Async/AbstractAsyncListener.php | 12 ++++ .../AbstractNotifyNewShortUrlListener.php | 10 +-- .../Async/AbstractNotifyVisitListener.php | 72 +++++++++++++++++++ .../Mercure/NotifyVisitToMercure.php | 61 ++-------------- .../RabbitMq/NotifyVisitToRabbitMq.php | 61 ++++++---------- .../RedisPubSub/NotifyVisitToRedis.php | 57 +++------------ .../Mercure/NotifyVisitToMercureTest.php | 7 +- .../RabbitMq/NotifyVisitToRabbitMqTest.php | 8 +-- 8 files changed, 133 insertions(+), 155 deletions(-) create mode 100644 module/Core/src/EventDispatcher/Async/AbstractAsyncListener.php create mode 100644 module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php diff --git a/module/Core/src/EventDispatcher/Async/AbstractAsyncListener.php b/module/Core/src/EventDispatcher/Async/AbstractAsyncListener.php new file mode 100644 index 00000000..e9c78306 --- /dev/null +++ b/module/Core/src/EventDispatcher/Async/AbstractAsyncListener.php @@ -0,0 +1,12 @@ +mercureHelper->publishUpdate($this->updatesGenerator->newShortUrlUpdate($shortUrl)); + $this->publishingHelper->publishUpdate($this->updatesGenerator->newShortUrlUpdate($shortUrl)); } catch (Throwable $e) { $this->logger->debug( 'Error while trying to notify {name} with new short URL. {e}', @@ -49,8 +49,4 @@ abstract class AbstractNotifyNewShortUrlListener ); } } - - abstract protected function isEnabled(): bool; - - abstract protected function getRemoteSystemName(): string; } diff --git a/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php b/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php new file mode 100644 index 00000000..a2967e64 --- /dev/null +++ b/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php @@ -0,0 +1,72 @@ +isEnabled()) { + return; + } + + $visitId = $visitLocated->visitId; + $visit = $this->em->find(Visit::class, $visitId); + $name = $this->getRemoteSystemName(); + + if ($visit === null) { + $this->logger->warning( + 'Tried to notify {name} for visit with id "{visitId}", but it does not exist.', + ['visitId' => $visitId, 'name' => $name], + ); + return; + } + + $updates = $this->determineUpdatesForVisit($visit); + + try { + each($updates, fn (Update $update) => $this->publishingHelper->publishUpdate($update)); + } catch (Throwable $e) { + $this->logger->debug( + 'Error while trying to notify {name} with new visit. {e}', + ['e' => $e, 'name' => $name], + ); + } + } + + /** + * @return Update[] + */ + protected function determineUpdatesForVisit(Visit $visit): array + { + if ($visit->isOrphan()) { + return [$this->updatesGenerator->newOrphanVisitUpdate($visit)]; + } + + return [ + $this->updatesGenerator->newShortUrlVisitUpdate($visit), + $this->updatesGenerator->newVisitUpdate($visit), + ]; + } +} diff --git a/module/Core/src/EventDispatcher/Mercure/NotifyVisitToMercure.php b/module/Core/src/EventDispatcher/Mercure/NotifyVisitToMercure.php index cd55fcb2..0ccd5fe9 100644 --- a/module/Core/src/EventDispatcher/Mercure/NotifyVisitToMercure.php +++ b/module/Core/src/EventDispatcher/Mercure/NotifyVisitToMercure.php @@ -4,64 +4,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher\Mercure; -use Doctrine\ORM\EntityManagerInterface; -use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; -use Shlinkio\Shlink\Common\UpdatePublishing\Update; -use Shlinkio\Shlink\Core\Entity\Visit; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; -use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; -use Throwable; +use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyVisitListener; -use function Functional\each; - -class NotifyVisitToMercure +class NotifyVisitToMercure extends AbstractNotifyVisitListener { - public function __construct( - private readonly PublishingHelperInterface $mercureHelper, - private readonly PublishingUpdatesGeneratorInterface $updatesGenerator, - private readonly EntityManagerInterface $em, - private readonly LoggerInterface $logger, - ) { + protected function isEnabled(): bool + { + return true; } - public function __invoke(VisitLocated $visitLocated): void + protected function getRemoteSystemName(): string { - $visitId = $visitLocated->visitId; - - /** @var Visit|null $visit */ - $visit = $this->em->find(Visit::class, $visitId); - if ($visit === null) { - $this->logger->warning('Tried to notify mercure for visit with id "{visitId}", but it does not exist.', [ - 'visitId' => $visitId, - ]); - return; - } - - try { - each( - $this->determineUpdatesForVisit($visit), - fn (Update $update) => $this->mercureHelper->publishUpdate($update), - ); - } catch (Throwable $e) { - $this->logger->debug('Error while trying to notify mercure hub with new visit. {e}', [ - 'e' => $e, - ]); - } - } - - /** - * @return Update[] - */ - private function determineUpdatesForVisit(Visit $visit): array - { - if ($visit->isOrphan()) { - return [$this->updatesGenerator->newOrphanVisitUpdate($visit)]; - } - - return [ - $this->updatesGenerator->newShortUrlVisitUpdate($visit), - $this->updatesGenerator->newVisitUpdate($visit), - ]; + return 'Mercure'; } } diff --git a/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php b/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php index 51c9f423..2bf74bb2 100644 --- a/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php +++ b/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php @@ -10,55 +10,28 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; use Shlinkio\Shlink\Core\Entity\Visit; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyVisitListener; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\EventDispatcher\Topic; use Shlinkio\Shlink\Core\Options\RabbitMqOptions; -use Throwable; -use function Functional\each; - -class NotifyVisitToRabbitMq +class NotifyVisitToRabbitMq extends AbstractNotifyVisitListener { public function __construct( - private readonly PublishingHelperInterface $rabbitMqHelper, - private readonly PublishingUpdatesGeneratorInterface $updatesGenerator, - private readonly EntityManagerInterface $em, - private readonly LoggerInterface $logger, + PublishingHelperInterface $rabbitMqHelper, + PublishingUpdatesGeneratorInterface $updatesGenerator, + EntityManagerInterface $em, + LoggerInterface $logger, private readonly DataTransformerInterface $orphanVisitTransformer, private readonly RabbitMqOptions $options, ) { - } - - public function __invoke(VisitLocated $visitLocated): void - { - if (! $this->options->isEnabled()) { - return; - } - - $visitId = $visitLocated->visitId; - $visit = $this->em->find(Visit::class, $visitId); - - if ($visit === null) { - $this->logger->warning('Tried to notify RabbitMQ for visit with id "{visitId}", but it does not exist.', [ - 'visitId' => $visitId, - ]); - return; - } - - $updates = $this->determineUpdatesForVisit($visit); - - try { - each($updates, fn (Update $update) => $this->rabbitMqHelper->publishUpdate($update)); - } catch (Throwable $e) { - $this->logger->debug('Error while trying to notify RabbitMQ with new visit. {e}', ['e' => $e]); - } + parent::__construct($rabbitMqHelper, $updatesGenerator, $em, $logger); } /** * @return Update[] */ - private function determineUpdatesForVisit(Visit $visit): array + protected function determineUpdatesForVisit(Visit $visit): array { return match (true) { // This was defined incorrectly. @@ -79,12 +52,18 @@ class NotifyVisitToRabbitMq ), ], - // Once the two deprecated cases above have been remove, replace this with a simple "if" and early return. - $visit->isOrphan() => [$this->updatesGenerator->newOrphanVisitUpdate($visit)], - default => [ - $this->updatesGenerator->newShortUrlVisitUpdate($visit), - $this->updatesGenerator->newVisitUpdate($visit), - ], + // Once the two deprecated cases above have been remove, make parent method private + default => parent::determineUpdatesForVisit($visit), }; } + + protected function isEnabled(): bool + { + return $this->options->isEnabled(); + } + + protected function getRemoteSystemName(): string + { + return 'RabbitMQ'; + } } diff --git a/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php b/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php index 820921db..8b54eff0 100644 --- a/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php +++ b/module/Core/src/EventDispatcher/RedisPubSub/NotifyVisitToRedis.php @@ -7,63 +7,28 @@ namespace Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; -use Shlinkio\Shlink\Common\UpdatePublishing\Update; -use Shlinkio\Shlink\Core\Entity\Visit; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyVisitListener; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; -use Throwable; -use function Functional\each; - -class NotifyVisitToRedis +class NotifyVisitToRedis extends AbstractNotifyVisitListener { public function __construct( - private readonly PublishingHelperInterface $redisHelper, - private readonly PublishingUpdatesGeneratorInterface $updatesGenerator, - private readonly EntityManagerInterface $em, - private readonly LoggerInterface $logger, + PublishingHelperInterface $redisHelper, + PublishingUpdatesGeneratorInterface $updatesGenerator, + EntityManagerInterface $em, + LoggerInterface $logger, private readonly bool $enabled, ) { + parent::__construct($redisHelper, $updatesGenerator, $em, $logger); } - public function __invoke(VisitLocated $visitLocated): void + protected function isEnabled(): bool { - if (! $this->enabled) { - return; - } - - $visitId = $visitLocated->visitId; - $visit = $this->em->find(Visit::class, $visitId); - - if ($visit === null) { - $this->logger->warning( - 'Tried to notify Redis pub/sub for visit with id "{visitId}", but it does not exist.', - ['visitId' => $visitId], - ); - return; - } - - $updates = $this->determineUpdatesForVisit($visit); - - try { - each($updates, fn (Update $update) => $this->redisHelper->publishUpdate($update)); - } catch (Throwable $e) { - $this->logger->debug('Error while trying to notify Redis pub/sub with new visit. {e}', ['e' => $e]); - } + return $this->enabled; } - /** - * @return Update[] - */ - private function determineUpdatesForVisit(Visit $visit): array + protected function getRemoteSystemName(): string { - if ($visit->isOrphan()) { - return [$this->updatesGenerator->newOrphanVisitUpdate($visit)]; - } - - return [ - $this->updatesGenerator->newShortUrlVisitUpdate($visit), - $this->updatesGenerator->newVisitUpdate($visit), - ]; + return 'Redis pub/sub'; } } diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php index 65049f49..1ce29d0d 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php @@ -52,8 +52,8 @@ class NotifyVisitToMercureTest extends TestCase $visitId = '123'; $findVisit = $this->em->find(Visit::class, $visitId)->willReturn(null); $logWarning = $this->logger->warning( - 'Tried to notify mercure for visit with id "{visitId}", but it does not exist.', - ['visitId' => $visitId], + 'Tried to notify {name} for visit with id "{visitId}", but it does not exist.', + ['visitId' => $visitId, 'name' => 'Mercure'], ); $logDebug = $this->logger->debug(Argument::cetera()); $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate( @@ -110,8 +110,9 @@ class NotifyVisitToMercureTest extends TestCase $findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit); $logWarning = $this->logger->warning(Argument::cetera()); - $logDebug = $this->logger->debug('Error while trying to notify mercure hub with new visit. {e}', [ + $logDebug = $this->logger->debug('Error while trying to notify {name} with new visit. {e}', [ 'e' => $e, + 'name' => 'Mercure', ]); $buildNewShortUrlVisitUpdate = $this->updatesGenerator->newShortUrlVisitUpdate($visit)->willReturn($update); $buildNewOrphanVisitUpdate = $this->updatesGenerator->newOrphanVisitUpdate($visit)->willReturn($update); diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index f2e31072..05ee7568 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -79,8 +79,8 @@ class NotifyVisitToRabbitMqTest extends TestCase $visitId = '123'; $findVisit = $this->em->find(Visit::class, $visitId)->willReturn(null); $logWarning = $this->logger->warning( - 'Tried to notify RabbitMQ for visit with id "{visitId}", but it does not exist.', - ['visitId' => $visitId], + 'Tried to notify {name} for visit with id "{visitId}", but it does not exist.', + ['visitId' => $visitId, 'name' => 'RabbitMQ'], ); ($this->listener)(new VisitLocated($visitId)); @@ -147,8 +147,8 @@ class NotifyVisitToRabbitMqTest extends TestCase ($this->listener)(new VisitLocated($visitId)); $this->logger->debug( - 'Error while trying to notify RabbitMQ with new visit. {e}', - ['e' => $e], + 'Error while trying to notify {name} with new visit. {e}', + ['e' => $e, 'name' => 'RabbitMQ'], )->shouldHaveBeenCalledOnce(); $findVisit->shouldHaveBeenCalledOnce(); $generateUpdate->shouldHaveBeenCalledOnce();