From 3fdba53995f4e7700172f75db2c316fae87b372c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 27 Dec 2019 09:25:07 +0100 Subject: [PATCH 01/12] Added basic implementation for new webhook events --- config/autoload/url-shortener.global.php | 1 + .../Core/config/event_dispatcher.config.php | 10 +++ .../EventDispatcher/NotifyVisitToWebHooks.php | 67 +++++++++++++++++++ .../src/EventDispatcher/ShortUrlLocated.php | 28 ++++++++ 4 files changed, 106 insertions(+) create mode 100644 module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php create mode 100644 module/Core/src/EventDispatcher/ShortUrlLocated.php diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 46e12593..58bc3faa 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -10,6 +10,7 @@ return [ 'hostname' => '', ], 'validate_url' => true, + 'visits_webhooks' => [], ], ]; diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index b5d86b09..8cee1f5c 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; +use GuzzleHttp\ClientInterface; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; @@ -16,6 +17,9 @@ return [ EventDispatcher\ShortUrlVisited::class => [ EventDispatcher\LocateShortUrlVisit::class, ], + EventDispatcher\ShortUrlLocated::class => [ + EventDispatcher\NotifyVisitToWebHooks::class, + ], ], ], @@ -32,6 +36,12 @@ return [ 'Logger_Shlink', GeolocationDbUpdater::class, ], + EventDispatcher\NotifyVisitToWebHooks::class => [ + 'httpClient', + 'em', + 'Logger_Shlink', + 'config.url_shortener.visits_webhooks', + ], ], ]; diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php new file mode 100644 index 00000000..995323e1 --- /dev/null +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -0,0 +1,67 @@ +httpClient = $httpClient; + $this->em = $em; + $this->logger = $logger; + $this->webhooks = $webhooks; + } + + public function __invoke(ShortUrlLocated $shortUrlLocated): void + { + $visitId = $shortUrlLocated->visitId(); + + /** @var Visit|null $visit */ + $visit = $this->em->find(Visit::class, $visitId); + if ($visit === null) { + $this->logger->warning('Tried to notify webhooks for visit with id "{visitId}", but it does not exist.', [ + 'visitId' => $visitId, + ]); + return; + } + + $requestOptions = [ + RequestOptions::TIMEOUT => 10, + RequestOptions::JSON => $visit->jsonSerialize(), + ]; + $requestPromises = map($this->webhooks, function (string $webhook) use ($requestOptions, $visitId) { + $promise = $this->httpClient->requestAsync(RequestMethodInterface::METHOD_POST, $webhook, $requestOptions); + return $promise->otherwise(function () use ($webhook, $visitId) { + // Log failures + $this->logger->warning('Failed to notify visit with id "{visitId}" to "{webhook}" webhook', [ + 'visitId' => $visitId, + 'webhook' => $webhook, + ]); + }); + }); + } +} diff --git a/module/Core/src/EventDispatcher/ShortUrlLocated.php b/module/Core/src/EventDispatcher/ShortUrlLocated.php new file mode 100644 index 00000000..63390ebf --- /dev/null +++ b/module/Core/src/EventDispatcher/ShortUrlLocated.php @@ -0,0 +1,28 @@ +visitId = $visitId; + } + + public function visitId(): string + { + return $this->visitId; + } + + public function jsonSerialize(): array + { + return ['visitId' => $this->visitId]; + } +} From 25243a10ecb00a5f1e2a56a7cad529892fd69fc3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 27 Dec 2019 14:02:43 +0100 Subject: [PATCH 02/12] Moved common bootstrapping code to run.php script --- bin/cli | 9 +++------ config/container.php | 15 ++++++++++----- config/run.php | 15 +++++++++++++++ module/Core/config/event_dispatcher.config.php | 1 - public/index.php | 10 ++-------- 5 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 config/run.php diff --git a/bin/cli b/bin/cli index 7f512eb0..c185efd3 100755 --- a/bin/cli +++ b/bin/cli @@ -1,10 +1,7 @@ #!/usr/bin/env php get(CliApp::class)->run(); +$run = require __DIR__ . '/../config/run.php'; +$run(true); diff --git a/config/container.php b/config/container.php index 29bc3e28..2ea9dc06 100644 --- a/config/container.php +++ b/config/container.php @@ -10,10 +10,15 @@ chdir(dirname(__DIR__)); require 'vendor/autoload.php'; // This class alias tricks the ConfigAbstractFactory to return Lock\Factory instances even with a different service name -class_alias(Lock\LockFactory::class, 'Shlinkio\Shlink\LocalLockFactory'); +if (! class_exists('Shlinkio\Shlink\LocalLockFactory')) { + class_alias(Lock\LockFactory::class, 'Shlinkio\Shlink\LocalLockFactory'); +} // Build container -$config = require __DIR__ . '/config.php'; -$container = new ServiceManager($config['dependencies']); -$container->setService('config', $config); -return $container; +return (function () { + $config = require __DIR__ . '/config.php'; + $container = new ServiceManager($config['dependencies']); + $container->setService('config', $config); + + return $container; +})(); diff --git a/config/run.php b/config/run.php new file mode 100644 index 00000000..4ea61775 --- /dev/null +++ b/config/run.php @@ -0,0 +1,15 @@ +get($isCli ? CliApp::class : Application::class); + + $app->run(); +}; diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 8cee1f5c..60869d04 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; -use GuzzleHttp\ClientInterface; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; diff --git a/public/index.php b/public/index.php index 336f5cc2..78bb412a 100644 --- a/public/index.php +++ b/public/index.php @@ -2,11 +2,5 @@ declare(strict_types=1); -use Psr\Container\ContainerInterface; -use Zend\Expressive\Application; - -(function () { - /** @var ContainerInterface $container */ - $container = include __DIR__ . '/../config/container.php'; - $container->get(Application::class)->run(); -})(); +$run = require __DIR__ . '/../config/run.php'; +$run(); From 562b0a0868c70cd380da36fa67bfc78755c522a2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 27 Dec 2019 16:15:14 +0100 Subject: [PATCH 03/12] Used PSR3 logger preprocessor format instead of sprintf when possible --- .../src/EventDispatcher/LocateShortUrlVisit.php | 15 +++++++-------- .../EventDispatcher/LocateShortUrlVisitTest.php | 14 ++++++++------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php index 1facba39..3c40d0a1 100644 --- a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php +++ b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php @@ -46,7 +46,9 @@ class LocateShortUrlVisit /** @var Visit|null $visit */ $visit = $this->em->find(Visit::class, $visitId); if ($visit === null) { - $this->logger->warning(sprintf('Tried to locate visit with id "%s", but it does not exist.', $visitId)); + $this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [ + 'visitId' => $visitId, + ]); return; } @@ -57,11 +59,8 @@ class LocateShortUrlVisit } catch (GeolocationDbUpdateFailedException $e) { if (! $e->olderDbExists()) { $this->logger->error( - sprintf( - 'GeoLite2 database download failed. It is not possible to locate visit with id %s. {e}', - $visitId - ), - ['e' => $e] + 'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}', + ['e' => $e, 'visitId' => $visitId] ); return; } @@ -75,8 +74,8 @@ class LocateShortUrlVisit : Location::emptyInstance(); } catch (WrongIpException $e) { $this->logger->warning( - sprintf('Tried to locate visit with id "%s", but its address seems to be wrong. {e}', $visitId), - ['e' => $e] + 'Tried to locate visit with id "{visitId}", but its address seems to be wrong. {e}', + ['e' => $e, 'visitId' => $visitId] ); return; } diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php index 0b557dd8..88b7d6c0 100644 --- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php @@ -56,7 +56,9 @@ class LocateShortUrlVisitTest extends TestCase { $event = new ShortUrlVisited('123'); $findVisit = $this->em->find(Visit::class, '123')->willReturn(null); - $logWarning = $this->logger->warning('Tried to locate visit with id "123", but it does not exist.'); + $logWarning = $this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [ + 'visitId' => 123, + ]); ($this->locateVisit)($event); @@ -77,7 +79,7 @@ class LocateShortUrlVisitTest extends TestCase WrongIpException::class ); $logWarning = $this->logger->warning( - Argument::containingString('Tried to locate visit with id "123", but its address seems to be wrong.'), + Argument::containingString('Tried to locate visit with id "{visitId}", but its address seems to be wrong.'), Argument::type('array') ); @@ -142,7 +144,7 @@ class LocateShortUrlVisitTest extends TestCase } /** @test */ - public function errorWhenUpdatingGeoliteWithExistingCopyLogsWarning(): void + public function errorWhenUpdatingGeoLiteWithExistingCopyLogsWarning(): void { $e = GeolocationDbUpdateFailedException::create(true); $ipAddr = '1.2.3.0'; @@ -170,7 +172,7 @@ class LocateShortUrlVisitTest extends TestCase } /** @test */ - public function errorWhenDownloadingGeoliteCancelsLocation(): void + public function errorWhenDownloadingGeoLiteCancelsLocation(): void { $e = GeolocationDbUpdateFailedException::create(false); $ipAddr = '1.2.3.0'; @@ -184,8 +186,8 @@ class LocateShortUrlVisitTest extends TestCase $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location); $checkUpdateDb = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e); $logError = $this->logger->error( - 'GeoLite2 database download failed. It is not possible to locate visit with id 123. {e}', - ['e' => $e] + 'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}', + ['e' => $e, 'visitId' => 123] ); ($this->locateVisit)($event); From 21a3d4b66bd476eb1a3dbec5495d0ecceb9b01c7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 27 Dec 2019 17:07:20 +0100 Subject: [PATCH 04/12] Updated NotifyVisitToWebHooks so that it waits for all requests to finish --- .../Core/config/event_dispatcher.config.php | 1 + module/Core/src/Entity/Visit.php | 5 +++ .../EventDispatcher/NotifyVisitToWebHooks.php | 36 +++++++++++++------ .../Transformer/ShortUrlDataTransformer.php | 13 ++++--- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 60869d04..ae2ca64a 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -40,6 +40,7 @@ return [ 'em', 'Logger_Shlink', 'config.url_shortener.visits_webhooks', + 'config.url_shortener.domain', ], ], diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index ee1b5394..b2ce3640 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -61,6 +61,11 @@ class Visit extends AbstractEntity implements JsonSerializable return ! empty($this->remoteAddr); } + public function getShortUrl(): ShortUrl + { + return $this->shortUrl; + } + public function getVisitLocation(): VisitLocationInterface { return $this->visitLocation ?? new UnknownVisitLocation(); diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index 995323e1..fea69ccd 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -10,8 +10,12 @@ use GuzzleHttp\ClientInterface; use GuzzleHttp\RequestOptions; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; +use Throwable; use function Functional\map; +use function Functional\partial_left; +use function GuzzleHttp\Promise\settle; class NotifyVisitToWebHooks { @@ -23,17 +27,21 @@ class NotifyVisitToWebHooks private $logger; /** @var array */ private $webhooks; + /** @var ShortUrlDataTransformer */ + private $transformer; public function __construct( ClientInterface $httpClient, EntityManagerInterface $em, LoggerInterface $logger, - array $webhooks + array $webhooks, + array $domainConfig ) { $this->httpClient = $httpClient; $this->em = $em; $this->logger = $logger; $this->webhooks = $webhooks; + $this->transformer = new ShortUrlDataTransformer($domainConfig); } public function __invoke(ShortUrlLocated $shortUrlLocated): void @@ -51,17 +59,25 @@ class NotifyVisitToWebHooks $requestOptions = [ RequestOptions::TIMEOUT => 10, - RequestOptions::JSON => $visit->jsonSerialize(), + RequestOptions::JSON => [ + 'shortUrl' => $this->transformer->transform($visit->getShortUrl(), false), + 'visit' => $visit->jsonSerialize(), + ], ]; - $requestPromises = map($this->webhooks, function (string $webhook) use ($requestOptions, $visitId) { + $logWebhookWarning = function (string $webhook, Throwable $e) use ($visitId): void { + $this->logger->warning('Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', [ + 'visitId' => $visitId, + 'webhook' => $webhook, + 'e' => $e, + ]); + }; + + $requestPromises = map($this->webhooks, function (string $webhook) use ($requestOptions, $logWebhookWarning) { $promise = $this->httpClient->requestAsync(RequestMethodInterface::METHOD_POST, $webhook, $requestOptions); - return $promise->otherwise(function () use ($webhook, $visitId) { - // Log failures - $this->logger->warning('Failed to notify visit with id "{visitId}" to "{webhook}" webhook', [ - 'visitId' => $visitId, - 'webhook' => $webhook, - ]); - }); + return $promise->otherwise(partial_left($logWebhookWarning, $webhook)); }); + + // Wait for all the promises to finish, ignoring rejections, as in those cases we only want to log the error. + settle($requestPromises)->wait(); } } diff --git a/module/Core/src/Transformer/ShortUrlDataTransformer.php b/module/Core/src/Transformer/ShortUrlDataTransformer.php index 4562782f..348ff0d5 100644 --- a/module/Core/src/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/Transformer/ShortUrlDataTransformer.php @@ -23,11 +23,11 @@ class ShortUrlDataTransformer implements DataTransformerInterface /** * @param ShortUrl $shortUrl */ - public function transform($shortUrl): array + public function transform($shortUrl, bool $includeDeprecated = true): array { $longUrl = $shortUrl->getLongUrl(); - return [ + $rawData = [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => $shortUrl->toString($this->domainConfig), 'longUrl' => $longUrl, @@ -35,10 +35,13 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'visitsCount' => $shortUrl->getVisitsCount(), 'tags' => invoke($shortUrl->getTags(), '__toString'), 'meta' => $this->buildMeta($shortUrl), - - // Deprecated - 'originalUrl' => $longUrl, ]; + + if ($includeDeprecated) { + $rawData['originalUrl'] = $longUrl; + } + + return $rawData; } private function buildMeta(ShortUrl $shortUrl): array From 79cd3ba91299b33637605a9fc416163190e851a1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 27 Dec 2019 20:12:13 +0100 Subject: [PATCH 05/12] Created NotifyVisitToWebhooksTest --- .../EventDispatcher/NotifyVisitToWebHooks.php | 56 +++++++-- .../NotifyVisitToWebHooksTest.php | 114 ++++++++++++++++++ 2 files changed, 157 insertions(+), 13 deletions(-) create mode 100644 module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index fea69ccd..21fd4c1f 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -4,12 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher; +use Closure; use Doctrine\ORM\EntityManagerInterface; use Fig\Http\Message\RequestMethodInterface; use GuzzleHttp\ClientInterface; +use GuzzleHttp\Promise\Promise; use GuzzleHttp\RequestOptions; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Throwable; @@ -29,23 +32,31 @@ class NotifyVisitToWebHooks private $webhooks; /** @var ShortUrlDataTransformer */ private $transformer; + /** @var AppOptions */ + private $appOptions; public function __construct( ClientInterface $httpClient, EntityManagerInterface $em, LoggerInterface $logger, array $webhooks, - array $domainConfig + array $domainConfig, + AppOptions $appOptions ) { $this->httpClient = $httpClient; $this->em = $em; $this->logger = $logger; $this->webhooks = $webhooks; $this->transformer = new ShortUrlDataTransformer($domainConfig); + $this->appOptions = $appOptions; } public function __invoke(ShortUrlLocated $shortUrlLocated): void { + if (empty($this->webhooks)) { + return; + } + $visitId = $shortUrlLocated->visitId(); /** @var Visit|null $visit */ @@ -57,27 +68,46 @@ class NotifyVisitToWebHooks return; } - $requestOptions = [ + $requestOptions = $this->buildRequestOptions($visit); + $requestPromises = $this->performRequests($requestOptions, $visitId); + + // Wait for all the promises to finish, ignoring rejections, as in those cases we only want to log the error. + settle($requestPromises)->wait(); + } + + private function buildRequestOptions(Visit $visit): array + { + return [ RequestOptions::TIMEOUT => 10, + RequestOptions::HEADERS => [ + 'User-Agent' => (string) $this->appOptions, + ], RequestOptions::JSON => [ 'shortUrl' => $this->transformer->transform($visit->getShortUrl(), false), 'visit' => $visit->jsonSerialize(), ], ]; - $logWebhookWarning = function (string $webhook, Throwable $e) use ($visitId): void { - $this->logger->warning('Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', [ - 'visitId' => $visitId, - 'webhook' => $webhook, - 'e' => $e, - ]); - }; + } - $requestPromises = map($this->webhooks, function (string $webhook) use ($requestOptions, $logWebhookWarning) { + /** + * @param Promise[] $requestOptions + */ + private function performRequests(array $requestOptions, string $visitId): array + { + return map($this->webhooks, function (string $webhook) use ($requestOptions, $visitId) { $promise = $this->httpClient->requestAsync(RequestMethodInterface::METHOD_POST, $webhook, $requestOptions); - return $promise->otherwise(partial_left($logWebhookWarning, $webhook)); + return $promise->otherwise( + partial_left(Closure::fromCallable([$this, 'logWebhookFailure']), $webhook, $visitId) + ); }); + } - // Wait for all the promises to finish, ignoring rejections, as in those cases we only want to log the error. - settle($requestPromises)->wait(); + private function logWebhookFailure(string $webhook, string $visitId, Throwable $e): void + { + $this->logger->warning('Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', [ + 'visitId' => $visitId, + 'webhook' => $webhook, + 'e' => $e, + ]); } } diff --git a/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php new file mode 100644 index 00000000..33737d01 --- /dev/null +++ b/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -0,0 +1,114 @@ +httpClient = $this->prophesize(ClientInterface::class); + $this->em = $this->prophesize(EntityManagerInterface::class); + $this->logger = $this->prophesize(LoggerInterface::class); + } + + /** @test */ + public function emptyWebhooksMakeNoFurtherActions(): void + { + $find = $this->em->find(Visit::class, '1')->willReturn(null); + + $this->createListener([])(new ShortUrlLocated('1')); + + $find->shouldNotHaveBeenCalled(); + } + + /** @test */ + public function invalidVisitDoesNotPerformAnyRequest(): void + { + $find = $this->em->find(Visit::class, '1')->willReturn(null); + $requestAsync = $this->httpClient->requestAsync( + RequestMethodInterface::METHOD_POST, + Argument::type('string'), + Argument::type('array') + )->willReturn(new FulfilledPromise('')); + $logWarning = $this->logger->warning( + 'Tried to notify webhooks for visit with id "{visitId}", but it does not exist.', + ['visitId' => '1'] + ); + + $this->createListener(['foo', 'bar'])(new ShortUrlLocated('1')); + + $find->shouldHaveBeenCalledOnce(); + $logWarning->shouldHaveBeenCalledOnce(); + $requestAsync->shouldNotHaveBeenCalled(); + } + + /** @test */ + public function expectedRequestsArePerformedToWebhooks(): void + { + $webhooks = ['foo', 'invalid', 'bar', 'baz']; + $invalidWebhooks = ['invalid', 'baz']; + + $find = $this->em->find(Visit::class, '1')->willReturn(new Visit(new ShortUrl(''), Visitor::emptyInstance())); + $requestAsync = $this->httpClient->requestAsync( + RequestMethodInterface::METHOD_POST, + Argument::type('string'), + Argument::type('array') + )->will(function (array $args) use ($invalidWebhooks) { + [, $webhook] = $args; + $e = new Exception(''); + + return contains($invalidWebhooks, $webhook) ? new RejectedPromise($e) : new FulfilledPromise(''); + }); + $logWarning = $this->logger->warning( + 'Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', + Argument::type('array') + ); + + $this->createListener($webhooks)(new ShortUrlLocated('1')); + + $find->shouldHaveBeenCalledOnce(); + $requestAsync->shouldHaveBeenCalledTimes(count($webhooks)); + $logWarning->shouldHaveBeenCalledTimes(count($invalidWebhooks)); + } + + private function createListener(array $webhooks): NotifyVisitToWebHooks + { + return new NotifyVisitToWebHooks( + $this->httpClient->reveal(), + $this->em->reveal(), + $this->logger->reveal(), + $webhooks, + [], + new AppOptions() + ); + } +} From 4886825564411f92a258bd0c60372ef98c77b975 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 28 Dec 2019 10:43:13 +0100 Subject: [PATCH 06/12] Improved NotifyVisitToWebHooksTest to kill more mutants --- .../NotifyVisitToWebHooksTest.php | 24 ++++++++++++++++--- phpunit-api.xml | 2 +- phpunit-db.xml | 2 +- phpunit.xml.dist | 2 +- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php index 33737d01..bb594e7d 100644 --- a/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -10,6 +10,8 @@ use Fig\Http\Message\RequestMethodInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Promise\FulfilledPromise; use GuzzleHttp\Promise\RejectedPromise; +use GuzzleHttp\RequestOptions; +use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; @@ -81,7 +83,17 @@ class NotifyVisitToWebHooksTest extends TestCase $requestAsync = $this->httpClient->requestAsync( RequestMethodInterface::METHOD_POST, Argument::type('string'), - Argument::type('array') + Argument::that(function (array $requestOptions) { + Assert::assertArrayHasKey(RequestOptions::HEADERS, $requestOptions); + Assert::assertArrayHasKey(RequestOptions::JSON, $requestOptions); + Assert::assertArrayHasKey(RequestOptions::TIMEOUT, $requestOptions); + Assert::assertEquals($requestOptions[RequestOptions::TIMEOUT], 10); + Assert::assertEquals($requestOptions[RequestOptions::HEADERS], ['User-Agent' => 'Shlink:v1.2.3']); + Assert::assertArrayHasKey('shortUrl', $requestOptions[RequestOptions::JSON]); + Assert::assertArrayHasKey('visit', $requestOptions[RequestOptions::JSON]); + + return $requestOptions; + }) )->will(function (array $args) use ($invalidWebhooks) { [, $webhook] = $args; $e = new Exception(''); @@ -90,7 +102,13 @@ class NotifyVisitToWebHooksTest extends TestCase }); $logWarning = $this->logger->warning( 'Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', - Argument::type('array') + Argument::that(function (array $extra) { + Assert::assertArrayHasKey('webhook', $extra); + Assert::assertArrayHasKey('visitId', $extra); + Assert::assertArrayHasKey('e', $extra); + + return $extra; + }) ); $this->createListener($webhooks)(new ShortUrlLocated('1')); @@ -108,7 +126,7 @@ class NotifyVisitToWebHooksTest extends TestCase $this->logger->reveal(), $webhooks, [], - new AppOptions() + new AppOptions(['name' => 'Shlink', 'version' => '1.2.3']) ); } } diff --git a/phpunit-api.xml b/phpunit-api.xml index 69132097..6e481fe5 100644 --- a/phpunit-api.xml +++ b/phpunit-api.xml @@ -1,7 +1,7 @@ diff --git a/phpunit-db.xml b/phpunit-db.xml index eab4be28..86cdbbc6 100644 --- a/phpunit-db.xml +++ b/phpunit-db.xml @@ -1,7 +1,7 @@ diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 1ae25124..67d08507 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,7 +1,7 @@ From b17bcb6c936891c1e8306193a543731abaef3820 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 28 Dec 2019 13:07:11 +0100 Subject: [PATCH 07/12] Updated LocateShortUrlVisit so that it dispatches a VisitLocated event --- .../Core/config/event_dispatcher.config.php | 4 ++- .../EventDispatcher/LocateShortUrlVisit.php | 30 +++++++++++++++---- .../EventDispatcher/NotifyVisitToWebHooks.php | 2 +- .../{ShortUrlLocated.php => VisitLocated.php} | 2 +- .../LocateShortUrlVisitTest.php | 26 +++++++++++++++- .../NotifyVisitToWebHooksTest.php | 8 ++--- 6 files changed, 58 insertions(+), 14 deletions(-) rename module/Core/src/EventDispatcher/{ShortUrlLocated.php => VisitLocated.php} (88%) diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index ae2ca64a..996669c3 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; +use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; @@ -16,7 +17,7 @@ return [ EventDispatcher\ShortUrlVisited::class => [ EventDispatcher\LocateShortUrlVisit::class, ], - EventDispatcher\ShortUrlLocated::class => [ + EventDispatcher\VisitLocated::class => [ EventDispatcher\NotifyVisitToWebHooks::class, ], ], @@ -34,6 +35,7 @@ return [ 'em', 'Logger_Shlink', GeolocationDbUpdater::class, + EventDispatcherInterface::class, ], EventDispatcher\NotifyVisitToWebHooks::class => [ 'httpClient', diff --git a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php index 3c40d0a1..4d767272 100644 --- a/module/Core/src/EventDispatcher/LocateShortUrlVisit.php +++ b/module/Core/src/EventDispatcher/LocateShortUrlVisit.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher; use Doctrine\ORM\EntityManagerInterface; +use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; @@ -26,17 +27,21 @@ class LocateShortUrlVisit private $logger; /** @var GeolocationDbUpdaterInterface */ private $dbUpdater; + /** @var EventDispatcherInterface */ + private $eventDispatcher; public function __construct( IpLocationResolverInterface $ipLocationResolver, EntityManagerInterface $em, LoggerInterface $logger, - GeolocationDbUpdaterInterface $dbUpdater + GeolocationDbUpdaterInterface $dbUpdater, + EventDispatcherInterface $eventDispatcher ) { $this->ipLocationResolver = $ipLocationResolver; $this->em = $em; $this->logger = $logger; $this->dbUpdater = $dbUpdater; + $this->eventDispatcher = $eventDispatcher; } public function __invoke(ShortUrlVisited $shortUrlVisited): void @@ -52,6 +57,15 @@ class LocateShortUrlVisit return; } + if ($this->downloadOrUpdateGeoLiteDb($visitId)) { + $this->locateVisit($visitId, $visit); + } + + $this->eventDispatcher->dispatch(new VisitLocated($visitId)); + } + + private function downloadOrUpdateGeoLiteDb(string $visitId): bool + { try { $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) { $this->logger->notice(sprintf('%s GeoLite2 database...', $olderDbExists ? 'Updating' : 'Downloading')); @@ -62,25 +76,29 @@ class LocateShortUrlVisit 'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}', ['e' => $e, 'visitId' => $visitId] ); - return; + return false; } $this->logger->warning('GeoLite2 database update failed. Proceeding with old version. {e}', ['e' => $e]); } + return true; + } + + private function locateVisit(string $visitId, Visit $visit): void + { try { $location = $visit->isLocatable() ? $this->ipLocationResolver->resolveIpLocation($visit->getRemoteAddr()) : Location::emptyInstance(); + + $visit->locate(new VisitLocation($location)); + $this->em->flush(); } catch (WrongIpException $e) { $this->logger->warning( 'Tried to locate visit with id "{visitId}", but its address seems to be wrong. {e}', ['e' => $e, 'visitId' => $visitId] ); - return; } - - $visit->locate(new VisitLocation($location)); - $this->em->flush(); } } diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php index 21fd4c1f..d99defb5 100644 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php @@ -51,7 +51,7 @@ class NotifyVisitToWebHooks $this->appOptions = $appOptions; } - public function __invoke(ShortUrlLocated $shortUrlLocated): void + public function __invoke(VisitLocated $shortUrlLocated): void { if (empty($this->webhooks)) { return; diff --git a/module/Core/src/EventDispatcher/ShortUrlLocated.php b/module/Core/src/EventDispatcher/VisitLocated.php similarity index 88% rename from module/Core/src/EventDispatcher/ShortUrlLocated.php rename to module/Core/src/EventDispatcher/VisitLocated.php index 63390ebf..4873ffa7 100644 --- a/module/Core/src/EventDispatcher/ShortUrlLocated.php +++ b/module/Core/src/EventDispatcher/VisitLocated.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\EventDispatcher; use JsonSerializable; -final class ShortUrlLocated implements JsonSerializable +final class VisitLocated implements JsonSerializable { /** @var string */ private $visitId; diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php index 88b7d6c0..a451ddea 100644 --- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php @@ -8,6 +8,7 @@ use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; +use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface; @@ -17,6 +18,7 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; +use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation; use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; @@ -35,6 +37,8 @@ class LocateShortUrlVisitTest extends TestCase private $logger; /** @var ObjectProphecy */ private $dbUpdater; + /** @var ObjectProphecy */ + private $eventDispatcher; public function setUp(): void { @@ -42,12 +46,14 @@ class LocateShortUrlVisitTest extends TestCase $this->em = $this->prophesize(EntityManagerInterface::class); $this->logger = $this->prophesize(LoggerInterface::class); $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class); + $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); $this->locateVisit = new LocateShortUrlVisit( $this->ipLocationResolver->reveal(), $this->em->reveal(), $this->logger->reveal(), - $this->dbUpdater->reveal() + $this->dbUpdater->reveal(), + $this->eventDispatcher->reveal() ); } @@ -59,6 +65,8 @@ class LocateShortUrlVisitTest extends TestCase $logWarning = $this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [ 'visitId' => 123, ]); + $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () { + }); ($this->locateVisit)($event); @@ -66,6 +74,7 @@ class LocateShortUrlVisitTest extends TestCase $this->em->flush()->shouldNotHaveBeenCalled(); $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->shouldNotHaveBeenCalled(); $logWarning->shouldHaveBeenCalled(); + $dispatch->shouldNotHaveBeenCalled(); } /** @test */ @@ -82,6 +91,8 @@ class LocateShortUrlVisitTest extends TestCase Argument::containingString('Tried to locate visit with id "{visitId}", but its address seems to be wrong.'), Argument::type('array') ); + $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () { + }); ($this->locateVisit)($event); @@ -89,6 +100,7 @@ class LocateShortUrlVisitTest extends TestCase $resolveLocation->shouldHaveBeenCalledOnce(); $logWarning->shouldHaveBeenCalled(); $this->em->flush()->shouldNotHaveBeenCalled(); + $dispatch->shouldHaveBeenCalledOnce(); } /** @@ -102,6 +114,8 @@ class LocateShortUrlVisitTest extends TestCase $flush = $this->em->flush()->will(function () { }); $resolveIp = $this->ipLocationResolver->resolveIpLocation(Argument::any()); + $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () { + }); ($this->locateVisit)($event); @@ -110,6 +124,7 @@ class LocateShortUrlVisitTest extends TestCase $flush->shouldHaveBeenCalledOnce(); $resolveIp->shouldNotHaveBeenCalled(); $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled(); + $dispatch->shouldHaveBeenCalledOnce(); } public function provideNonLocatableVisits(): iterable @@ -133,6 +148,8 @@ class LocateShortUrlVisitTest extends TestCase $flush = $this->em->flush()->will(function () { }); $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location); + $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () { + }); ($this->locateVisit)($event); @@ -141,6 +158,7 @@ class LocateShortUrlVisitTest extends TestCase $flush->shouldHaveBeenCalledOnce(); $resolveIp->shouldHaveBeenCalledOnce(); $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled(); + $dispatch->shouldHaveBeenCalledOnce(); } /** @test */ @@ -157,6 +175,8 @@ class LocateShortUrlVisitTest extends TestCase }); $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location); $checkUpdateDb = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e); + $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () { + }); ($this->locateVisit)($event); @@ -169,6 +189,7 @@ class LocateShortUrlVisitTest extends TestCase 'GeoLite2 database update failed. Proceeding with old version. {e}', ['e' => $e] )->shouldHaveBeenCalledOnce(); + $dispatch->shouldHaveBeenCalledOnce(); } /** @test */ @@ -189,6 +210,8 @@ class LocateShortUrlVisitTest extends TestCase 'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}', ['e' => $e, 'visitId' => 123] ); + $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () { + }); ($this->locateVisit)($event); @@ -198,5 +221,6 @@ class LocateShortUrlVisitTest extends TestCase $resolveIp->shouldNotHaveBeenCalled(); $checkUpdateDb->shouldHaveBeenCalledOnce(); $logError->shouldHaveBeenCalledOnce(); + $dispatch->shouldHaveBeenCalledOnce(); } } diff --git a/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php index bb594e7d..305f2e23 100644 --- a/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -19,7 +19,7 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\NotifyVisitToWebHooks; -use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlLocated; +use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; @@ -47,7 +47,7 @@ class NotifyVisitToWebHooksTest extends TestCase { $find = $this->em->find(Visit::class, '1')->willReturn(null); - $this->createListener([])(new ShortUrlLocated('1')); + $this->createListener([])(new VisitLocated('1')); $find->shouldNotHaveBeenCalled(); } @@ -66,7 +66,7 @@ class NotifyVisitToWebHooksTest extends TestCase ['visitId' => '1'] ); - $this->createListener(['foo', 'bar'])(new ShortUrlLocated('1')); + $this->createListener(['foo', 'bar'])(new VisitLocated('1')); $find->shouldHaveBeenCalledOnce(); $logWarning->shouldHaveBeenCalledOnce(); @@ -111,7 +111,7 @@ class NotifyVisitToWebHooksTest extends TestCase }) ); - $this->createListener($webhooks)(new ShortUrlLocated('1')); + $this->createListener($webhooks)(new VisitLocated('1')); $find->shouldHaveBeenCalledOnce(); $requestAsync->shouldHaveBeenCalledTimes(count($webhooks)); From 583985e7cedefd3e44b8e88b768d46df35d865bd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 28 Dec 2019 13:50:41 +0100 Subject: [PATCH 08/12] Moved VisitLocated as a regular event, since async tasks cannot trigger other async tasks --- module/Core/config/event_dispatcher.config.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 996669c3..aead1447 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -12,20 +12,22 @@ use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; return [ 'events' => [ - 'regular' => [], + 'regular' => [ + EventDispatcher\VisitLocated::class => [ + EventDispatcher\NotifyVisitToWebHooks::class, + ], + ], 'async' => [ EventDispatcher\ShortUrlVisited::class => [ EventDispatcher\LocateShortUrlVisit::class, ], - EventDispatcher\VisitLocated::class => [ - EventDispatcher\NotifyVisitToWebHooks::class, - ], ], ], 'dependencies' => [ 'factories' => [ EventDispatcher\LocateShortUrlVisit::class => ConfigAbstractFactory::class, + EventDispatcher\NotifyVisitToWebHooks::class => ConfigAbstractFactory::class, ], ], @@ -43,6 +45,7 @@ return [ 'Logger_Shlink', 'config.url_shortener.visits_webhooks', 'config.url_shortener.domain', + Options\AppOptions::class, ], ], From 3c9da809627a43ea185fd2ed00357ca2691a1d96 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 28 Dec 2019 16:35:16 +0100 Subject: [PATCH 09/12] Documented how to provide visits webhooks to docker image via env vars --- docker/README.md | 6 ++++++ docker/config/shlink_in_docker.local.php | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/docker/README.md b/docker/README.md index 66e8e743..b600c268 100644 --- a/docker/README.md +++ b/docker/README.md @@ -110,6 +110,7 @@ This is the complete list of supported env vars: * `BASE_PATH`: The base path from which you plan to serve shlink, in case you don't want to serve it from the root of the domain. Defaults to `''`. * `WEB_WORKER_NUM`: The amount of concurrent http requests this shlink instance will be able to server. Defaults to 16. * `TASK_WORKER_NUM`: The amount of concurrent background tasks this shlink instance will be able to execute. Defaults to 16. +* `VISITS_WEBHOOKS`: A comma-separated list of URLs that will receive a `POST` request when a short URL receives a visit. * `REDIS_SERVERS`: A comma-separated list of redis servers where Shlink locks are stored (locks are used to prevent some operations to be run more than once in parallel). This is important when running more than one Shlink instance ([Multi instance considerations](#multi-instance-considerations)). If not provided, Shlink stores locks on every instance separately. @@ -145,6 +146,7 @@ docker run \ -e "BASE_PATH=/my-campaign" \ -e WEB_WORKER_NUM=64 \ -e TASK_WORKER_NUM=32 \ + -e "VISITS_WEBHOOKS=http://my-api.com/api/v2.3/notify,https://third-party.io/foo" \ shlinkio/shlink:stable ``` @@ -173,6 +175,10 @@ The whole configuration should have this format, but it can be split into multip "tcp://172.20.0.1:6379", "tcp://172.20.0.2:6379" ], + "visits_webhooks": [ + "http://my-api.com/api/v2.3/notify", + "https://third-party.io/foo" + ], "db_config": { "driver": "pdo_mysql", "dbname": "shlink", diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index 5da6761d..6d3367ac 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -99,6 +99,12 @@ $helper = new class { 'base_url' => env('BASE_URL_REDIRECT_TO'), ]; } + + public function getVisitsWebhooks(): array + { + $webhooks = env('VISITS_WEBHOOKS'); + return $webhooks === null ? [] : explode(',', $webhooks); + } }; return [ @@ -125,6 +131,7 @@ return [ 'hostname' => env('SHORT_DOMAIN_HOST', ''), ], 'validate_url' => (bool) env('VALIDATE_URLS', true), + 'visits_webhooks' => $helper->getVisitsWebhooks(), ], 'not_found_redirects' => $helper->getNotFoundRedirectsConfig(), From 664569a52b92b60b48cbb1d0885c987220eff82a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 28 Dec 2019 16:42:21 +0100 Subject: [PATCH 10/12] Added visits_webhooks option to SimplifiedConfigParser --- module/Core/src/Config/SimplifiedConfigParser.php | 1 + module/Core/test/Config/SimplifiedConfigParserTest.php | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php index 95c777e8..5ee912b0 100644 --- a/module/Core/src/Config/SimplifiedConfigParser.php +++ b/module/Core/src/Config/SimplifiedConfigParser.php @@ -32,6 +32,7 @@ class SimplifiedConfigParser 'base_path' => ['router', 'base_path'], 'web_worker_num' => ['zend-expressive-swoole', 'swoole-http-server', 'options', 'worker_num'], 'task_worker_num' => ['zend-expressive-swoole', 'swoole-http-server', 'options', 'task_worker_num'], + 'visits_webhooks' => ['url_shortener', 'visits_webhooks'], ]; private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [ 'delete_short_url_threshold' => [ diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index 20afab7f..76ba9b1b 100644 --- a/module/Core/test/Config/SimplifiedConfigParserTest.php +++ b/module/Core/test/Config/SimplifiedConfigParserTest.php @@ -53,6 +53,10 @@ class SimplifiedConfigParserTest extends TestCase ], 'base_path' => '/foo/bar', 'task_worker_num' => 50, + 'visits_webhooks' => [ + 'http://my-api.com/api/v2.3/notify', + 'https://third-party.io/foo', + ], ]; $expected = [ 'app_options' => [ @@ -76,6 +80,10 @@ class SimplifiedConfigParserTest extends TestCase 'hostname' => 'doma.in', ], 'validate_url' => false, + 'visits_webhooks' => [ + 'http://my-api.com/api/v2.3/notify', + 'https://third-party.io/foo', + ], ], 'delete_short_urls' => [ From 8667544b3a1a7187f0a790305ce57ab96313e6e3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 29 Dec 2019 14:09:51 +0100 Subject: [PATCH 11/12] Updated to installer v3.3 --- composer.json | 10 +++++----- config/autoload/installer.global.php | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 32105e23..7782f61d 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,7 @@ "pugx/shortid-php": "^0.5", "shlinkio/shlink-common": "^2.4", "shlinkio/shlink-event-dispatcher": "^1.1", - "shlinkio/shlink-installer": "^3.2", + "shlinkio/shlink-installer": "^3.3", "shlinkio/shlink-ip-geolocation": "^1.2", "symfony/console": "^5.0", "symfony/filesystem": "^5.0", @@ -111,7 +111,7 @@ "@test:api" ], "test:unit": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-php build/coverage-unit.cov --testdox", - "test:unit:ci": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-php build/coverage-unit.cov --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/junit.xml --testdox", + "test:unit:ci": "@test:unit --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/junit.xml", "test:db": [ "@test:db:sqlite", "@test:db:mysql", @@ -123,15 +123,15 @@ "@test:db:mysql", "@test:db:postgres" ], - "test:db:sqlite": "APP_ENV=test phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always -c phpunit-db.xml --coverage-php build/coverage-db.cov --testdox", + "test:db:sqlite": "APP_ENV=test phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-php build/coverage-db.cov --testdox -c phpunit-db.xml", "test:db:mysql": "DB_DRIVER=mysql composer test:db:sqlite", "test:db:maria": "DB_DRIVER=maria composer test:db:sqlite", "test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite", "test:api": "bin/test/run-api-tests.sh", "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage", "infect": "infection --threads=4 --min-msi=80 --log-verbosity=default --only-covered", - "infect:ci": "infection --threads=4 --min-msi=80 --log-verbosity=default --only-covered --coverage=build", - "infect:show": "infection --threads=4 --min-msi=80 --log-verbosity=default --only-covered --show-mutations", + "infect:ci": "@infect --coverage=build", + "infect:show": "@infect --show-mutations", "infect:test": [ "@test:unit:ci", "@infect:ci" diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 91ebdab7..402e6bb3 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -11,6 +11,8 @@ return [ Plugin\UrlShortenerConfigCustomizer::SCHEMA, Plugin\UrlShortenerConfigCustomizer::HOSTNAME, Plugin\UrlShortenerConfigCustomizer::VALIDATE_URL, + Plugin\UrlShortenerConfigCustomizer::NOTIFY_VISITS_WEBHOOKS, + Plugin\UrlShortenerConfigCustomizer::VISITS_WEBHOOKS, ], Plugin\ApplicationConfigCustomizer::class => [ From b4e3dd7b4e74990acc0b421c51f17c9e43606b78 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 29 Dec 2019 14:15:50 +0100 Subject: [PATCH 12/12] Updated changelog with v1.21.0 --- CHANGELOG.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8fe8298..bb001cb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## 1.21.0 - 2019-12-29 #### Added @@ -22,6 +22,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * The `GET /short-urls` endpoint now accepts the `startDate` and `endDate` query params. * The `short-urls:list` command now allows `--startDate` and `--endDate` flags to be optionally provided. +* [#338](https://github.com/shlinkio/shlink/issues/338) Added support to asynchronously notify external services via webhook, only when shlink is served with swoole. + + Configured webhooks will receive a POST request every time a URL receives a visit, including information about the short URL and the visit. + + The payload will look like this: + + ```json + { + "shortUrl": {}, + "visit": {} + } + ``` + + > The `shortUrl` and `visit` props have the same shape as it is defined in the [API spec](https://api-spec.shlink.io). + #### Changed * [#492](https://github.com/shlinkio/shlink/issues/492) Updated to monolog 2, together with other dependencies, like Symfony 5 and infection-php.