From d16fda3f1698f952cad60b83fedaecc357c3eaeb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 Oct 2021 10:53:21 +0200 Subject: [PATCH] Added option to send orphan visits to webhooks --- config/autoload/redirects.global.php | 9 +++++ config/autoload/url-shortener.global.php | 8 ---- config/autoload/webhooks.global.php | 19 +++++++++ module/Core/config/dependencies.config.php | 2 + .../Core/config/event_dispatcher.config.php | 2 +- .../EventDispatcher/NotifyVisitToWebHooks.php | 12 ++++-- module/Core/src/Options/WebhookOptions.php | 40 +++++++++++++++++++ .../NotifyVisitToWebHooksTest.php | 25 +++++++++++- 8 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 config/autoload/webhooks.global.php create mode 100644 module/Core/src/Options/WebhookOptions.php diff --git a/config/autoload/redirects.global.php b/config/autoload/redirects.global.php index 0707f1e4..339ca27d 100644 --- a/config/autoload/redirects.global.php +++ b/config/autoload/redirects.global.php @@ -4,6 +4,9 @@ declare(strict_types=1); use function Shlinkio\Shlink\Common\env; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; + return [ 'not_found_redirects' => [ @@ -12,4 +15,10 @@ return [ 'base_url' => env('BASE_URL_REDIRECT_TO'), ], + 'url_shortener' => [ + // TODO Move these options to their own config namespace. Maybe "redirects". + 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), + 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), + ], + ]; diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 432c63e1..ae6bfe84 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -4,13 +4,10 @@ declare(strict_types=1); use function Shlinkio\Shlink\Common\env; -use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; -use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; return (static function (): array { - $webhooks = env('VISITS_WEBHOOKS'); $shortCodesLength = (int) env('DEFAULT_SHORT_CODES_LENGTH', DEFAULT_SHORT_CODES_LENGTH); $shortCodesLength = $shortCodesLength < MIN_SHORT_CODES_LENGTH ? MIN_SHORT_CODES_LENGTH : $shortCodesLength; $useHttps = env('USE_HTTPS'); // Deprecated. For v3, set this to true by default, instead of null @@ -24,14 +21,9 @@ return (static function (): array { 'hostname' => env('DEFAULT_DOMAIN', env('SHORT_DOMAIN_HOST', '')), ], 'validate_url' => (bool) env('VALIDATE_URLS', false), // Deprecated - 'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks), 'default_short_codes_length' => $shortCodesLength, 'auto_resolve_titles' => (bool) env('AUTO_RESOLVE_TITLES', false), 'append_extra_path' => (bool) env('REDIRECT_APPEND_EXTRA_PATH', false), - - // TODO Move these two options to their own config namespace. Maybe "redirects". - 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), - 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), ], ]; diff --git a/config/autoload/webhooks.global.php b/config/autoload/webhooks.global.php new file mode 100644 index 00000000..585d3eb2 --- /dev/null +++ b/config/autoload/webhooks.global.php @@ -0,0 +1,19 @@ + [ + // TODO Move these options to their own config namespace + 'visits_webhooks' => $webhooks === null ? [] : explode(',', $webhooks), + 'notify_orphan_visits_to_webhooks' => (bool) env('NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS', false), + ], + + ]; +})(); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 7c3d7468..16b84819 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -26,6 +26,7 @@ return [ Options\UrlShortenerOptions::class => ConfigAbstractFactory::class, Options\TrackingOptions::class => ConfigAbstractFactory::class, Options\QrCodeOptions::class => ConfigAbstractFactory::class, + Options\WebhookOptions::class => ConfigAbstractFactory::class, Service\UrlShortener::class => ConfigAbstractFactory::class, Service\ShortUrlService::class => ConfigAbstractFactory::class, @@ -88,6 +89,7 @@ return [ Options\UrlShortenerOptions::class => ['config.url_shortener'], Options\TrackingOptions::class => ['config.tracking'], Options\QrCodeOptions::class => ['config.qr_codes'], + Options\WebhookOptions::class => ['config.url_shortener'], // TODO This config is currently under url_shortener Service\UrlShortener::class => [ ShortUrl\Helper\ShortUrlTitleResolutionHelper::class, diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index bddd59f5..5256bc92 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -58,7 +58,7 @@ return [ 'httpClient', 'em', 'Logger_Shlink', - 'config.url_shortener.visits_webhooks', + Options\WebhookOptions::class, ShortUrl\Transformer\ShortUrlDataTransformer::class, Options\AppOptions::class, ], diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index ad8381be..b5c2e501 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\Core\Options\AppOptions; +use Shlinkio\Shlink\Core\Options\WebhookOptions; use Throwable; use function Functional\map; @@ -26,8 +27,7 @@ class NotifyVisitToWebHooks private ClientInterface $httpClient, private EntityManagerInterface $em, private LoggerInterface $logger, - /** @var string[] */ - private array $webhooks, + private WebhookOptions $webhookOptions, private DataTransformerInterface $transformer, private AppOptions $appOptions, ) { @@ -35,7 +35,7 @@ class NotifyVisitToWebHooks public function __invoke(VisitLocated $shortUrlLocated): void { - if (empty($this->webhooks)) { + if (! $this->webhookOptions->hasWebhooks()) { return; } @@ -50,6 +50,10 @@ class NotifyVisitToWebHooks return; } + if ($visit->isOrphan() && ! $this->webhookOptions->notifyOrphanVisits()) { + return; + } + $requestOptions = $this->buildRequestOptions($visit); $requestPromises = $this->performRequests($requestOptions, $visitId); @@ -78,7 +82,7 @@ class NotifyVisitToWebHooks private function performRequests(array $requestOptions, string $visitId): array { return map( - $this->webhooks, + $this->webhookOptions->webhooks(), fn (string $webhook): PromiseInterface => $this->httpClient ->requestAsync(RequestMethodInterface::METHOD_POST, $webhook, $requestOptions) ->otherwise(fn (Throwable $e) => $this->logWebhookFailure($webhook, $visitId, $e)), diff --git a/module/Core/src/Options/WebhookOptions.php b/module/Core/src/Options/WebhookOptions.php new file mode 100644 index 00000000..c86789b2 --- /dev/null +++ b/module/Core/src/Options/WebhookOptions.php @@ -0,0 +1,40 @@ +visitsWebhooks; + } + + public function hasWebhooks(): bool + { + return ! empty($this->visitsWebhooks); + } + + protected function setVisitsWebhooks(array $visitsWebhooks): void + { + $this->visitsWebhooks = $visitsWebhooks; + } + + public function notifyOrphanVisits(): bool + { + return $this->notifyOrphanVisitsToWebhooks; + } + + protected function setNotifyOrphanVisitsToWebhooks(bool $notifyOrphanVisitsToWebhooks): void + { + $this->notifyOrphanVisitsToWebhooks = $notifyOrphanVisitsToWebhooks; + } +} diff --git a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php index 9e884753..99609bb4 100644 --- a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -23,6 +23,7 @@ use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\Core\EventDispatcher\NotifyVisitToWebHooks; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; +use Shlinkio\Shlink\Core\Options\WebhookOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; @@ -75,6 +76,24 @@ class NotifyVisitToWebHooksTest extends TestCase $requestAsync->shouldNotHaveBeenCalled(); } + /** @test */ + public function orphanVisitDoesNotPerformAnyRequestWhenDisabled(): void + { + $find = $this->em->find(Visit::class, '1')->willReturn(Visit::forBasePath(Visitor::emptyInstance())); + $requestAsync = $this->httpClient->requestAsync( + RequestMethodInterface::METHOD_POST, + Argument::type('string'), + Argument::type('array'), + )->willReturn(new FulfilledPromise('')); + $logWarning = $this->logger->warning(Argument::cetera()); + + $this->createListener(['foo', 'bar'], false)(new VisitLocated('1')); + + $find->shouldHaveBeenCalledOnce(); + $logWarning->shouldNotHaveBeenCalled(); + $requestAsync->shouldNotHaveBeenCalled(); + } + /** * @test * @dataProvider provideVisits @@ -136,13 +155,15 @@ class NotifyVisitToWebHooksTest extends TestCase yield 'orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), ['visit'],]; } - private function createListener(array $webhooks): NotifyVisitToWebHooks + private function createListener(array $webhooks, bool $notifyOrphanVisits = true): NotifyVisitToWebHooks { return new NotifyVisitToWebHooks( $this->httpClient->reveal(), $this->em->reveal(), $this->logger->reveal(), - $webhooks, + new WebhookOptions( + ['visits_webhooks' => $webhooks, 'notify_orphan_visits_to_webhooks' => $notifyOrphanVisits], + ), new ShortUrlDataTransformer(new ShortUrlStringifier([])), new AppOptions(['name' => 'Shlink', 'version' => '1.2.3']), );