Merge pull request #1486 from acelaya-forks/feature/backwards-compatible-rabbit-mq

Feature/backwards compatible rabbit mq
This commit is contained in:
Alejandro Celaya 2022-07-25 12:55:28 +02:00 committed by GitHub
commit ceabb5ab2c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 157 additions and 33 deletions

View File

@ -16,6 +16,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
### Changed
* [#1452](https://github.com/shlinkio/shlink/issues/1452) Updated to monolog 3
* [#1485](https://github.com/shlinkio/shlink/issues/1485) Changed payload published in RabbitMQ for all visits events, in order to conform with the Async API spec.
Since this is a breaking change, also provided a new `RABBITMQ_LEGACY_VISITS_PUBLISHING=true` env var that can be provided in order to keep the old payload.
This env var is considered deprecated and will be removed in Shlink 4, when the legacy format will no longer be supported.
### Deprecated
* *Nothing*

View File

@ -13,6 +13,9 @@ return [
'user' => EnvVars::RABBITMQ_USER->loadFromEnv(),
'password' => EnvVars::RABBITMQ_PASSWORD->loadFromEnv(),
'vhost' => EnvVars::RABBITMQ_VHOST->loadFromEnv('/'),
// Deprecated
'legacy_visits_publishing' => (bool) EnvVars::RABBITMQ_LEGACY_VISITS_PUBLISHING->loadFromEnv(false),
],
];

View File

@ -27,6 +27,7 @@ return [
Options\UrlShortenerOptions::class => ConfigAbstractFactory::class,
Options\TrackingOptions::class => ConfigAbstractFactory::class,
Options\QrCodeOptions::class => ConfigAbstractFactory::class,
Options\RabbitMqOptions::class => ConfigAbstractFactory::class,
Options\WebhookOptions::class => ConfigAbstractFactory::class,
Service\UrlShortener::class => ConfigAbstractFactory::class,
@ -91,6 +92,7 @@ return [
Options\UrlShortenerOptions::class => ['config.url_shortener'],
Options\TrackingOptions::class => ['config.tracking'],
Options\QrCodeOptions::class => ['config.qr_codes'],
Options\RabbitMqOptions::class => ['config.rabbitmq'],
Options\WebhookOptions::class => ['config.visits_webhooks'],
Service\UrlShortener::class => [

View File

@ -98,14 +98,14 @@ return [
'Logger_Shlink',
Visit\Transformer\OrphanVisitDataTransformer::class,
ShortUrl\Transformer\ShortUrlDataTransformer::class,
'config.rabbitmq.enabled',
Options\RabbitMqOptions::class,
],
EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq::class => [
RabbitMqPublishingHelper::class,
'em',
'Logger_Shlink',
ShortUrl\Transformer\ShortUrlDataTransformer::class,
'config.rabbitmq.enabled',
Options\RabbitMqOptions::class,
],
EventDispatcher\UpdateGeoLiteDb::class => [GeolocationDbUpdater::class, 'Logger_Shlink'],
],

View File

@ -33,6 +33,8 @@ enum EnvVars: string
case RABBITMQ_USER = 'RABBITMQ_USER';
case RABBITMQ_PASSWORD = 'RABBITMQ_PASSWORD';
case RABBITMQ_VHOST = 'RABBITMQ_VHOST';
/** @deprecated */
case RABBITMQ_LEGACY_VISITS_PUBLISHING = 'RABBITMQ_LEGACY_VISITS_PUBLISHING';
case DEFAULT_INVALID_SHORT_URL_REDIRECT = 'DEFAULT_INVALID_SHORT_URL_REDIRECT';
case DEFAULT_REGULAR_404_REDIRECT = 'DEFAULT_REGULAR_404_REDIRECT';
case DEFAULT_BASE_URL_REDIRECT = 'DEFAULT_BASE_URL_REDIRECT';

View File

@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated;
use Shlinkio\Shlink\Core\EventDispatcher\Topic;
use Shlinkio\Shlink\Core\Options\RabbitMqOptions;
use Throwable;
class NotifyNewShortUrlToRabbitMq
@ -20,13 +21,13 @@ class NotifyNewShortUrlToRabbitMq
private readonly EntityManagerInterface $em,
private readonly LoggerInterface $logger,
private readonly DataTransformerInterface $shortUrlTransformer,
private readonly bool $isEnabled,
private readonly RabbitMqOptions $options,
) {
}
public function __invoke(ShortUrlCreated $shortUrlCreated): void
{
if (! $this->isEnabled) {
if (! $this->options->isEnabled()) {
return;
}

View File

@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated;
use Shlinkio\Shlink\Core\EventDispatcher\Topic;
use Shlinkio\Shlink\Core\Options\RabbitMqOptions;
use Throwable;
class NotifyVisitToRabbitMq
@ -20,14 +21,14 @@ class NotifyVisitToRabbitMq
private readonly EntityManagerInterface $em,
private readonly LoggerInterface $logger,
private readonly DataTransformerInterface $orphanVisitTransformer,
private readonly DataTransformerInterface $shortUrlTransformer, // @phpstan-ignore-line
private readonly bool $isEnabled,
private readonly DataTransformerInterface $shortUrlTransformer,
private readonly RabbitMqOptions $options,
) {
}
public function __invoke(VisitLocated $shortUrlLocated): void
{
if (! $this->isEnabled) {
if (! $this->options->isEnabled()) {
return;
}
@ -70,14 +71,15 @@ class NotifyVisitToRabbitMq
private function visitToPayload(Visit $visit): array
{
// FIXME This was defined incorrectly.
// According to the spec, both the visit and the short URL it belongs to, should be published.
// The shape should be ['visit' => [...], 'shortUrl' => ?[...]]
// However, this would be a breaking change, so we need a flag that determines the shape of the payload.
// This was defined incorrectly.
// According to the spec, both the visit and the short URL it belongs to, should be published.
// The shape should be ['visit' => [...], 'shortUrl' => ?[...]]
// However, this would be a breaking change, so we need a flag that determines the shape of the payload.
if ($this->options->legacyVisitsPublishing()) {
return ! $visit->isOrphan() ? $visit->jsonSerialize() : $this->orphanVisitTransformer->transform($visit);
}
return ! $visit->isOrphan() ? $visit->jsonSerialize() : $this->orphanVisitTransformer->transform($visit);
if ($visit->isOrphan()) { // @phpstan-ignore-line
if ($visit->isOrphan()) {
return ['visit' => $this->orphanVisitTransformer->transform($visit)];
}

View File

@ -0,0 +1,40 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Options;
use Laminas\Stdlib\AbstractOptions;
class RabbitMqOptions extends AbstractOptions
{
protected $__strictMode__ = false; // phpcs:ignore
private bool $enabled = false;
/** @deprecated */
private bool $legacyVisitsPublishing = false;
public function isEnabled(): bool
{
return $this->enabled;
}
protected function setEnabled(bool $enabled): self
{
$this->enabled = $enabled;
return $this;
}
/** @deprecated */
public function legacyVisitsPublishing(): bool
{
return $this->legacyVisitsPublishing;
}
/** @deprecated */
protected function setLegacyVisitsPublishing(bool $legacyVisitsPublishing): self
{
$this->legacyVisitsPublishing = $legacyVisitsPublishing;
return $this;
}
}

View File

@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated;
use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq;
use Shlinkio\Shlink\Core\EventDispatcher\Topic;
use Shlinkio\Shlink\Core\Options\RabbitMqOptions;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer;
use Throwable;
@ -30,34 +31,30 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase
private ObjectProphecy $helper;
private ObjectProphecy $em;
private ObjectProphecy $logger;
private RabbitMqOptions $options;
protected function setUp(): void
{
$this->helper = $this->prophesize(RabbitMqPublishingHelperInterface::class);
$this->em = $this->prophesize(EntityManagerInterface::class);
$this->logger = $this->prophesize(LoggerInterface::class);
$this->options = new RabbitMqOptions(['enabled' => true]);
$this->listener = new NotifyNewShortUrlToRabbitMq(
$this->helper->reveal(),
$this->em->reveal(),
$this->logger->reveal(),
new ShortUrlDataTransformer(new ShortUrlStringifier([])),
true,
$this->options,
);
}
/** @test */
public function doesNothingWhenTheFeatureIsNotEnabled(): void
{
$listener = new NotifyNewShortUrlToRabbitMq(
$this->helper->reveal(),
$this->em->reveal(),
$this->logger->reveal(),
new ShortUrlDataTransformer(new ShortUrlStringifier([])),
false,
);
$this->options->enabled = false;
$listener(new ShortUrlCreated('123'));
($this->listener)(new ShortUrlCreated('123'));
$this->em->find(Argument::cetera())->shouldNotHaveBeenCalled();
$this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();

View File

@ -7,6 +7,7 @@ namespace ShlinkioTest\Shlink\Core\EventDispatcher\RabbitMq;
use Doctrine\ORM\EntityManagerInterface;
use DomainException;
use Exception;
use PHPUnit\Framework\Assert;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
@ -20,6 +21,7 @@ use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated;
use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyVisitToRabbitMq;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Options\RabbitMqOptions;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer;
use Shlinkio\Shlink\Core\Visit\Transformer\OrphanVisitDataTransformer;
@ -36,12 +38,14 @@ class NotifyVisitToRabbitMqTest extends TestCase
private ObjectProphecy $helper;
private ObjectProphecy $em;
private ObjectProphecy $logger;
private RabbitMqOptions $options;
protected function setUp(): void
{
$this->helper = $this->prophesize(RabbitMqPublishingHelperInterface::class);
$this->em = $this->prophesize(EntityManagerInterface::class);
$this->logger = $this->prophesize(LoggerInterface::class);
$this->options = new RabbitMqOptions(['enabled' => true, 'legacy_visits_publishing' => true]);
$this->listener = new NotifyVisitToRabbitMq(
$this->helper->reveal(),
@ -49,23 +53,16 @@ class NotifyVisitToRabbitMqTest extends TestCase
$this->logger->reveal(),
new OrphanVisitDataTransformer(),
new ShortUrlDataTransformer(new ShortUrlStringifier([])),
true,
$this->options,
);
}
/** @test */
public function doesNothingWhenTheFeatureIsNotEnabled(): void
{
$listener = new NotifyVisitToRabbitMq(
$this->helper->reveal(),
$this->em->reveal(),
$this->logger->reveal(),
new OrphanVisitDataTransformer(),
new ShortUrlDataTransformer(new ShortUrlStringifier([])),
false,
);
$this->options->enabled = false;
$listener(new VisitLocated('123'));
($this->listener)(new VisitLocated('123'));
$this->em->find(Argument::cetera())->shouldNotHaveBeenCalled();
$this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
@ -156,4 +153,79 @@ class NotifyVisitToRabbitMqTest extends TestCase
yield [new Exception('Exception Error')];
yield [new DomainException('DomainException Error')];
}
/**
* @test
* @dataProvider provideLegacyPayloads
*/
public function expectedPayloadIsPublishedDependingOnConfig(
bool $legacy,
Visit $visit,
callable $assertPayload,
): void {
$this->options->legacyVisitsPublishing = $legacy;
$visitId = '123';
$findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit);
($this->listener)(new VisitLocated($visitId));
$findVisit->shouldHaveBeenCalledOnce();
$this->helper->publishPayloadInQueue(Argument::that($assertPayload), Argument::type('string'))
->shouldHaveBeenCalled();
}
public function provideLegacyPayloads(): iterable
{
yield 'non-legacy non-orphan visit' => [
true,
$visit = Visit::forValidShortUrl(ShortUrl::withLongUrl(''), Visitor::emptyInstance()),
function (array $payload) use ($visit): bool {
Assert::assertEquals($payload, $visit->jsonSerialize());
Assert::assertArrayNotHasKey('visitedUrl', $payload);
Assert::assertArrayNotHasKey('type', $payload);
Assert::assertArrayNotHasKey('visit', $payload);
Assert::assertArrayNotHasKey('shortUrl', $payload);
return true;
},
];
yield 'non-legacy orphan visit' => [
true,
Visit::forBasePath(Visitor::emptyInstance()),
function (array $payload): bool {
Assert::assertArrayHasKey('visitedUrl', $payload);
Assert::assertArrayHasKey('type', $payload);
return true;
},
];
yield 'legacy non-orphan visit' => [
false,
$visit = Visit::forValidShortUrl(ShortUrl::withLongUrl(''), Visitor::emptyInstance()),
function (array $payload) use ($visit): bool {
Assert::assertArrayHasKey('visit', $payload);
Assert::assertArrayHasKey('shortUrl', $payload);
Assert::assertIsArray($payload['visit']);
Assert::assertEquals($payload['visit'], $visit->jsonSerialize());
Assert::assertArrayNotHasKey('visitedUrl', ['visit']);
Assert::assertArrayNotHasKey('type', ['visit']);
return true;
},
];
yield 'legacy orphan visit' => [
false,
Visit::forBasePath(Visitor::emptyInstance()),
function (array $payload): bool {
Assert::assertArrayHasKey('visit', $payload);
Assert::assertArrayNotHasKey('shortUrl', $payload);
Assert::assertIsArray($payload['visit']);
Assert::assertArrayHasKey('visitedUrl', $payload['visit']);
Assert::assertArrayHasKey('type', $payload['visit']);
return true;
},
];
}
}