diff --git a/CHANGELOG.md b/CHANGELOG.md index b0cac56b..084f4435 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this When trying to delete orphan visits the result will also be `0` and no visits will actually get deleted. ### Changed -* *Nothing* +* [#1799](https://github.com/shlinkio/shlink/issues/1799) RoadRunner/openswoole jobs are not run anymore for tasks that are actually disabled. + + For example, if you did not enable RabbitMQ real-time updates, instead of triggering a job that ends immediately, the job will not even be enqueued. ### Deprecated * [#1783](https://github.com/shlinkio/shlink/issues/1783) Deprecated support for openswoole. RoadRunner is the best replacement, with the same capabilities, but much easier and convenient to install and manage. diff --git a/composer.json b/composer.json index 2f5ae894..99a71e50 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "ramsey/uuid": "^4.7", "shlinkio/shlink-common": "^5.5", "shlinkio/shlink-config": "dev-main#245bbdd as 2.5", - "shlinkio/shlink-event-dispatcher": "^3.0", + "shlinkio/shlink-event-dispatcher": "dev-main#bd3a62b as 3.1", "shlinkio/shlink-importer": "^5.1", "shlinkio/shlink-installer": "^8.4", "shlinkio/shlink-ip-geolocation": "^3.2", diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index e4bf3c0c..a7867240 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -12,7 +12,9 @@ use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper; use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitLocator; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitToLocationHelper; +use Shlinkio\Shlink\EventDispatcher\Listener\EnabledListenerCheckerInterface; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; +use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; return [ @@ -54,6 +56,12 @@ return [ EventDispatcher\RedisPubSub\NotifyVisitToRedis::class => ConfigAbstractFactory::class, EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis::class => ConfigAbstractFactory::class, EventDispatcher\UpdateGeoLiteDb::class => ConfigAbstractFactory::class, + + EventDispatcher\Helper\EnabledListenerChecker::class => ConfigAbstractFactory::class, + ], + + 'aliases' => [ + EnabledListenerCheckerInterface::class => EventDispatcher\Helper\EnabledListenerChecker::class, ], 'delegators' => [ @@ -147,6 +155,14 @@ return [ 'Logger_Shlink', EventDispatcherInterface::class, ], + + EventDispatcher\Helper\EnabledListenerChecker::class => [ + Options\RabbitMqOptions::class, + 'config.redis.pub_sub_enabled', + 'config.mercure.public_hub_url', + Options\WebhookOptions::class, + GeoLite2Options::class, + ], ], ]; diff --git a/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php b/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php new file mode 100644 index 00000000..d7e231a2 --- /dev/null +++ b/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php @@ -0,0 +1,42 @@ +<?php + +declare(strict_types=1); + +namespace Shlinkio\Shlink\Core\EventDispatcher\Helper; + +use Shlinkio\Shlink\Core\EventDispatcher; +use Shlinkio\Shlink\Core\Options\RabbitMqOptions; +use Shlinkio\Shlink\Core\Options\WebhookOptions; +use Shlinkio\Shlink\EventDispatcher\Listener\EnabledListenerCheckerInterface; +use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; + +class EnabledListenerChecker implements EnabledListenerCheckerInterface +{ + public function __construct( + private readonly RabbitMqOptions $rabbitMqOptions, + private readonly bool $redisPubSubEnabled, + private readonly ?string $publicMercureHubUrl, + private readonly WebhookOptions $webhookOptions, + private readonly GeoLite2Options $geoLiteOptions, + ) { + } + + public function shouldRegisterListener(string $event, string $listener, bool $isAsync): bool + { + if (! $isAsync) { + return true; + } + + return match ($listener) { + EventDispatcher\RabbitMq\NotifyVisitToRabbitMq::class, + EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq::class => $this->rabbitMqOptions->enabled, + EventDispatcher\RedisPubSub\NotifyVisitToRedis::class, + EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis::class => $this->redisPubSubEnabled, + EventDispatcher\Mercure\NotifyVisitToMercure::class, + EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => $this->publicMercureHubUrl !== null, + EventDispatcher\NotifyVisitToWebHooks::class => $this->webhookOptions->hasWebhooks(), + EventDispatcher\UpdateGeoLiteDb::class => $this->geoLiteOptions->hasLicenseKey(), + default => false, // Any unknown async listener should not be enabled by default + }; + } +} diff --git a/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php b/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php new file mode 100644 index 00000000..a98c1e67 --- /dev/null +++ b/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php @@ -0,0 +1,160 @@ +<?php + +declare(strict_types=1); + +namespace ShlinkioTest\Shlink\Core\EventDispatcher\Helper; + +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\EventDispatcher\Helper\EnabledListenerChecker; +use Shlinkio\Shlink\Core\EventDispatcher\Mercure\NotifyNewShortUrlToMercure; +use Shlinkio\Shlink\Core\EventDispatcher\Mercure\NotifyVisitToMercure; +use Shlinkio\Shlink\Core\EventDispatcher\NotifyVisitToWebHooks; +use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq; +use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyVisitToRabbitMq; +use Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis; +use Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub\NotifyVisitToRedis; +use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb; +use Shlinkio\Shlink\Core\Options\RabbitMqOptions; +use Shlinkio\Shlink\Core\Options\WebhookOptions; +use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; + +class EnabledListenerCheckerTest extends TestCase +{ + #[Test, DataProvider('provideListeners')] + public function syncListenersAreRegisteredByDefault(string $listener): void + { + self::assertTrue($this->checker()->shouldRegisterListener('', $listener, false)); + } + + public static function provideListeners(): iterable + { + return [ + [NotifyVisitToRabbitMq::class], + [NotifyNewShortUrlToRabbitMq::class], + [NotifyVisitToRedis::class], + [NotifyNewShortUrlToRedis::class], + [NotifyVisitToMercure::class], + [NotifyNewShortUrlToMercure::class], + [NotifyVisitToWebHooks::class], + [UpdateGeoLiteDb::class], + ]; + } + + /** + * @param array<string, boolean> $expectedResult + */ + #[Test, DataProvider('provideConfiguredCheckers')] + public function appropriateListenersAreEnabledBasedOnConfig( + EnabledListenerChecker $checker, + array $expectedResult, + ): void { + foreach ($expectedResult as $listener => $shouldBeRegistered) { + self::assertEquals($shouldBeRegistered, $checker->shouldRegisterListener('', $listener, true)); + } + } + + public static function provideConfiguredCheckers(): iterable + { + yield 'RabbitMQ' => [self::checker(rabbitMqEnabled: true), [ + NotifyVisitToRabbitMq::class => true, + NotifyNewShortUrlToRabbitMq::class => true, + NotifyVisitToRedis::class => false, + NotifyNewShortUrlToRedis::class => false, + NotifyVisitToMercure::class => false, + NotifyNewShortUrlToMercure::class => false, + NotifyVisitToWebHooks::class => false, + UpdateGeoLiteDb::class => false, + 'unknown' => false, + ]]; + yield 'Redis Pub/Sub' => [self::checker(redisPubSubEnabled: true), [ + NotifyVisitToRabbitMq::class => false, + NotifyNewShortUrlToRabbitMq::class => false, + NotifyVisitToRedis::class => true, + NotifyNewShortUrlToRedis::class => true, + NotifyVisitToMercure::class => false, + NotifyNewShortUrlToMercure::class => false, + NotifyVisitToWebHooks::class => false, + UpdateGeoLiteDb::class => false, + 'unknown' => false, + ]]; + yield 'Mercure' => [self::checker(mercureEnabled: true), [ + NotifyVisitToRabbitMq::class => false, + NotifyNewShortUrlToRabbitMq::class => false, + NotifyVisitToRedis::class => false, + NotifyNewShortUrlToRedis::class => false, + NotifyVisitToMercure::class => true, + NotifyNewShortUrlToMercure::class => true, + NotifyVisitToWebHooks::class => false, + UpdateGeoLiteDb::class => false, + 'unknown' => false, + ]]; + yield 'Webhooks' => [self::checker(webhooksEnabled: true), [ + NotifyVisitToRabbitMq::class => false, + NotifyNewShortUrlToRabbitMq::class => false, + NotifyVisitToRedis::class => false, + NotifyNewShortUrlToRedis::class => false, + NotifyVisitToMercure::class => false, + NotifyNewShortUrlToMercure::class => false, + NotifyVisitToWebHooks::class => true, + UpdateGeoLiteDb::class => false, + 'unknown' => false, + ]]; + yield 'GeoLite' => [self::checker(geoLiteEnabled: true), [ + NotifyVisitToRabbitMq::class => false, + NotifyNewShortUrlToRabbitMq::class => false, + NotifyVisitToRedis::class => false, + NotifyNewShortUrlToRedis::class => false, + NotifyVisitToMercure::class => false, + NotifyNewShortUrlToMercure::class => false, + NotifyVisitToWebHooks::class => false, + UpdateGeoLiteDb::class => true, + 'unknown' => false, + ]]; + yield 'All disabled' => [self::checker(), [ + NotifyVisitToRabbitMq::class => false, + NotifyNewShortUrlToRabbitMq::class => false, + NotifyVisitToRedis::class => false, + NotifyNewShortUrlToRedis::class => false, + NotifyVisitToMercure::class => false, + NotifyNewShortUrlToMercure::class => false, + NotifyVisitToWebHooks::class => false, + UpdateGeoLiteDb::class => false, + 'unknown' => false, + ]]; + yield 'All enabled' => [self::checker( + rabbitMqEnabled: true, + redisPubSubEnabled: true, + mercureEnabled: true, + webhooksEnabled: true, + geoLiteEnabled: true, + ), [ + NotifyVisitToRabbitMq::class => true, + NotifyNewShortUrlToRabbitMq::class => true, + NotifyVisitToRedis::class => true, + NotifyNewShortUrlToRedis::class => true, + NotifyVisitToMercure::class => true, + NotifyNewShortUrlToMercure::class => true, + NotifyVisitToWebHooks::class => true, + UpdateGeoLiteDb::class => true, + 'unknown' => false, + ]]; + } + + private static function checker( + bool $rabbitMqEnabled = false, + bool $redisPubSubEnabled = false, + bool $mercureEnabled = false, + bool $webhooksEnabled = false, + bool $geoLiteEnabled = false, + ): EnabledListenerChecker { + return new EnabledListenerChecker( + new RabbitMqOptions(enabled: $rabbitMqEnabled), + $redisPubSubEnabled, + $mercureEnabled ? 'the-url' : null, + new WebhookOptions(['webhooks' => $webhooksEnabled ? ['foo', 'bar'] : []]), + new GeoLite2Options(licenseKey: $geoLiteEnabled ? 'the-key' : null), + ); + } +}