Merge pull request #1962 from acelaya-forks/feature/disabled-qr-codes

Allow QR codes to be generated for disabled short URLs
This commit is contained in:
Alejandro Celaya 2023-12-24 16:55:52 +01:00 committed by GitHub
commit 625eba76c7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 151 additions and 21 deletions

View file

@ -4,6 +4,23 @@ 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]
### Added
* *Nothing*
### Changed
* *Nothing*
### Deprecated
* *Nothing*
### Removed
* *Nothing*
### Fixed
* [#1960](https://github.com/shlinkio/shlink/issues/1960) Allow QR codes to be optionally resolved even when corresponding short URL is not enabled.
## [3.7.1] - 2023-12-17
### Added
* *Nothing*

View file

@ -49,7 +49,7 @@
"shlinkio/shlink-config": "^2.5",
"shlinkio/shlink-event-dispatcher": "^3.1",
"shlinkio/shlink-importer": "^5.2.1",
"shlinkio/shlink-installer": "^8.6.1",
"shlinkio/shlink-installer": "dev-develop#62aae8d as 8.7",
"shlinkio/shlink-ip-geolocation": "^3.4",
"shlinkio/shlink-json": "^1.1",
"spiral/roadrunner": "^2023.2",

View file

@ -62,6 +62,7 @@ return [
Option\QrCode\DefaultFormatConfigOption::class,
Option\QrCode\DefaultErrorCorrectionConfigOption::class,
Option\QrCode\DefaultRoundBlockSizeConfigOption::class,
Option\QrCode\EnabledForDisabledShortUrlsConfigOption::class,
Option\RabbitMq\RabbitMqEnabledConfigOption::class,
Option\RabbitMq\RabbitMqHostConfigOption::class,
Option\RabbitMq\RabbitMqUseSslConfigOption::class,

View file

@ -4,6 +4,7 @@ declare(strict_types=1);
use Shlinkio\Shlink\Core\Config\EnvVars;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_ERROR_CORRECTION;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_FORMAT;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_MARGIN;
@ -22,6 +23,9 @@ return [
'round_block_size' => (bool) EnvVars::DEFAULT_QR_CODE_ROUND_BLOCK_SIZE->loadFromEnv(
DEFAULT_QR_CODE_ROUND_BLOCK_SIZE,
),
'enabled_for_disabled_short_urls' => (bool) EnvVars::QR_CODE_FOR_DISABLED_SHORT_URLS->loadFromEnv(
DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS,
),
],
];

View file

@ -19,4 +19,6 @@ const DEFAULT_QR_CODE_MARGIN = 0;
const DEFAULT_QR_CODE_FORMAT = 'png';
const DEFAULT_QR_CODE_ERROR_CORRECTION = 'l';
const DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = true;
// Deprecated. Shlink 4.0.0 should change default value to `true`
const DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS = false;
const MIN_TASK_WORKERS = 4;

View file

@ -35,7 +35,7 @@ logs:
http:
mode: 'off' # Disable logging as Shlink handles it internally
server:
level: debug
level: info
metrics:
level: debug
jobs:

View file

@ -18,13 +18,13 @@ use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface;
class QrCodeAction implements MiddlewareInterface
readonly class QrCodeAction implements MiddlewareInterface
{
public function __construct(
private ShortUrlResolverInterface $urlResolver,
private ShortUrlStringifierInterface $stringifier,
private LoggerInterface $logger,
private QrCodeOptions $defaultOptions,
private QrCodeOptions $options,
) {
}
@ -33,13 +33,15 @@ class QrCodeAction implements MiddlewareInterface
$identifier = ShortUrlIdentifier::fromRedirectRequest($request);
try {
$shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier);
$shortUrl = $this->options->enabledForDisabledShortUrls
? $this->urlResolver->resolvePublicShortUrl($identifier)
: $this->urlResolver->resolveEnabledShortUrl($identifier);
} catch (ShortUrlNotFoundException $e) {
$this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]);
return $handler->handle($request);
}
$params = QrCodeParams::fromRequest($request, $this->defaultOptions);
$params = QrCodeParams::fromRequest($request, $this->options);
$qrCodeBuilder = Builder::create()
->data($this->stringifier->stringify($shortUrl))
->size($params->size)

View file

@ -43,6 +43,7 @@ enum EnvVars: string
case DEFAULT_QR_CODE_FORMAT = 'DEFAULT_QR_CODE_FORMAT';
case DEFAULT_QR_CODE_ERROR_CORRECTION = 'DEFAULT_QR_CODE_ERROR_CORRECTION';
case DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = 'DEFAULT_QR_CODE_ROUND_BLOCK_SIZE';
case QR_CODE_FOR_DISABLED_SHORT_URLS = 'QR_CODE_FOR_DISABLED_SHORT_URLS';
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

@ -4,20 +4,22 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Options;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_ERROR_CORRECTION;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_FORMAT;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_MARGIN;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_ROUND_BLOCK_SIZE;
use const Shlinkio\Shlink\DEFAULT_QR_CODE_SIZE;
final class QrCodeOptions
readonly final class QrCodeOptions
{
public function __construct(
public readonly int $size = DEFAULT_QR_CODE_SIZE,
public readonly int $margin = DEFAULT_QR_CODE_MARGIN,
public readonly string $format = DEFAULT_QR_CODE_FORMAT,
public readonly string $errorCorrection = DEFAULT_QR_CODE_ERROR_CORRECTION,
public readonly bool $roundBlockSize = DEFAULT_QR_CODE_ROUND_BLOCK_SIZE,
public int $size = DEFAULT_QR_CODE_SIZE,
public int $margin = DEFAULT_QR_CODE_MARGIN,
public string $format = DEFAULT_QR_CODE_FORMAT,
public string $errorCorrection = DEFAULT_QR_CODE_ERROR_CORRECTION,
public bool $roundBlockSize = DEFAULT_QR_CODE_ROUND_BLOCK_SIZE,
public bool $enabledForDisabledShortUrls = DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS,
) {
}
}

View file

@ -10,9 +10,9 @@ use Symfony\Component\Console\Input\InputInterface;
use function sprintf;
final class ShortUrlIdentifier
final readonly class ShortUrlIdentifier
{
private function __construct(public readonly string $shortCode, public readonly ?string $domain = null)
private function __construct(public string $shortCode, public ?string $domain = null)
{
}

View file

@ -12,11 +12,11 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
class ShortUrlResolver implements ShortUrlResolverInterface
readonly class ShortUrlResolver implements ShortUrlResolverInterface
{
public function __construct(
private readonly EntityManagerInterface $em,
private readonly UrlShortenerOptions $urlShortenerOptions,
private EntityManagerInterface $em,
private UrlShortenerOptions $urlShortenerOptions,
) {
}
@ -39,11 +39,21 @@ class ShortUrlResolver implements ShortUrlResolverInterface
* @throws ShortUrlNotFoundException
*/
public function resolveEnabledShortUrl(ShortUrlIdentifier $identifier): ShortUrl
{
$shortUrl = $this->resolvePublicShortUrl($identifier);
if (! $shortUrl->isEnabled()) {
throw ShortUrlNotFoundException::fromNotFound($identifier);
}
return $shortUrl;
}
public function resolvePublicShortUrl(ShortUrlIdentifier $identifier): ShortUrl
{
/** @var ShortUrlRepository $shortUrlRepo */
$shortUrlRepo = $this->em->getRepository(ShortUrl::class);
$shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier, $this->urlShortenerOptions->mode);
if (! $shortUrl?->isEnabled()) {
if ($shortUrl === null) {
throw ShortUrlNotFoundException::fromNotFound($identifier);
}

View file

@ -17,6 +17,19 @@ interface ShortUrlResolverInterface
public function resolveShortUrl(ShortUrlIdentifier $identifier, ?ApiKey $apiKey = null): ShortUrl;
/**
* Resolves a public short URL matching provided identifier.
* When trying to match public short URLs, if provided domain is default one, it gets ignored.
* If provided domain is not default, but the short code is found in default domain, we fall back to that short URL.
*
* @throws ShortUrlNotFoundException
*/
public function resolvePublicShortUrl(ShortUrlIdentifier $identifier): ShortUrl;
/**
* Resolves a public short URL matching provided identifier, only if it's not disabled.
* Disabled short URLs are those which received the max amount of visits, have a `validSince` in the future or have
* a `validUntil` in the past.
*
* @throws ShortUrlNotFoundException
*/
public function resolveEnabledShortUrl(ShortUrlIdentifier $identifier): ShortUrl;

View file

@ -0,0 +1,27 @@
<?php
declare(strict_types=1);
namespace ShlinkioApiTest\Shlink\Core\Action;
use PHPUnit\Framework\Attributes\Test;
use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
class QrCodeTest extends ApiTestCase
{
#[Test]
public function returnsNotFoundWhenShortUrlIsNotEnabled(): void
{
// The QR code successfully resolves at first
$response = $this->callShortUrl('custom/qr-code');
self::assertEquals(200, $response->getStatusCode());
// This short URL allow max 2 visits
$this->callShortUrl('custom');
$this->callShortUrl('custom');
// After 2 visits, the QR code should return a 404
$response = $this->callShortUrl('custom/qr-code');
self::assertEquals(404, $response->getStatusCode());
}
}

View file

@ -230,6 +230,35 @@ class QrCodeActionTest extends TestCase
];
}
#[Test, DataProvider('provideEnabled')]
public function qrCodeIsResolvedBasedOnOptions(bool $enabledForDisabledShortUrls): void
{
if ($enabledForDisabledShortUrls) {
$this->urlResolver->expects($this->once())->method('resolvePublicShortUrl')->willThrowException(
ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortCodeAndDomain('')),
);
$this->urlResolver->expects($this->never())->method('resolveEnabledShortUrl');
} else {
$this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->willThrowException(
ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortCodeAndDomain('')),
);
$this->urlResolver->expects($this->never())->method('resolvePublicShortUrl');
}
$options = new QrCodeOptions(enabledForDisabledShortUrls: $enabledForDisabledShortUrls);
$this->action($options)->process(
ServerRequestFactory::fromGlobals(),
$this->createMock(RequestHandlerInterface::class),
);
}
public static function provideEnabled(): iterable
{
yield 'always enabled' => [true];
yield 'only enabled short URLs' => [false];
}
public function action(?QrCodeOptions $options = null): QrCodeAction
{
return new QrCodeAction(

View file

@ -59,7 +59,7 @@ class ShortUrlResolverTest extends TestCase
}
#[Test, DataProviderExternal(ApiKeyDataProviders::class, 'adminApiKeysProvider')]
public function exceptionIsThrownIfShortcodeIsNotFound(?ApiKey $apiKey): void
public function exceptionIsThrownIfShortCodeIsNotFound(?ApiKey $apiKey): void
{
$shortCode = 'abc123';
$identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode);
@ -73,7 +73,7 @@ class ShortUrlResolverTest extends TestCase
}
#[Test]
public function shortCodeToEnabledShortUrlProperlyParsesShortCode(): void
public function resolveEnabledShortUrlProperlyParsesShortCode(): void
{
$shortUrl = ShortUrl::withLongUrl('https://expected_url');
$shortCode = $shortUrl->getShortCode();
@ -89,8 +89,30 @@ class ShortUrlResolverTest extends TestCase
self::assertSame($shortUrl, $result);
}
#[Test, DataProvider('provideResolutionMethods')]
public function resolutionThrowsExceptionIfUrlIsNotEnabled(string $method): void
{
$shortCode = 'abc123';
$this->repo->expects($this->once())->method('findOneWithDomainFallback')->with(
ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
ShortUrlMode::STRICT,
)->willReturn(null);
$this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo);
$this->expectException(ShortUrlNotFoundException::class);
$this->urlResolver->{$method}(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode));
}
public static function provideResolutionMethods(): iterable
{
yield 'resolveEnabledShortUrl' => ['resolveEnabledShortUrl'];
yield 'resolvePublicShortUrl' => ['resolvePublicShortUrl'];
}
#[Test, DataProvider('provideDisabledShortUrls')]
public function shortCodeToEnabledShortUrlThrowsExceptionIfUrlIsNotEnabled(ShortUrl $shortUrl): void
public function resolveEnabledShortUrlThrowsExceptionIfUrlIsNotEnabled(ShortUrl $shortUrl): void
{
$shortCode = $shortUrl->getShortCode();