Merge pull request #1801 from acelaya-forks/feature/no-run-disabled-tasks

Feature/no run disabled tasks
This commit is contained in:
Alejandro Celaya 2023-06-03 19:19:08 +02:00 committed by GitHub
commit 7c649e7497
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 222 additions and 2 deletions

View file

@ -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.

View file

@ -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",

View file

@ -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,
],
],
];

View file

@ -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
};
}
}

View file

@ -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),
);
}
}