From 8d244c8d340da64c8c8f5870fa278ef30e84f603 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Sep 2022 15:54:43 +0200 Subject: [PATCH] Migrated UrlShortenerOptions to immutable object --- config/test/test_config.global.php | 1 - .../ShortUrl/CreateShortUrlCommand.php | 6 +- .../ShortUrl/CreateShortUrlCommandTest.php | 2 +- module/Core/config/dependencies.config.php | 3 +- .../Core/src/Options/UrlShortenerOptions.php | 70 +++---------------- .../ExtraPathRedirectMiddleware.php | 8 +-- module/Core/src/Util/UrlValidator.php | 4 +- .../ExtraPathRedirectMiddlewareTest.php | 39 ++++++----- module/Core/test/Util/UrlValidatorTest.php | 36 +++++----- .../Action/ShortUrl/CreateShortUrlAction.php | 2 +- 10 files changed, 58 insertions(+), 113 deletions(-) diff --git a/config/test/test_config.global.php b/config/test/test_config.global.php index 9b338d7a..678e1b05 100644 --- a/config/test/test_config.global.php +++ b/config/test/test_config.global.php @@ -130,7 +130,6 @@ return [ 'schema' => 'http', 'hostname' => 'doma.in', ], - 'validate_url' => true, ], 'mezzio-swoole' => [ diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 6b4cce1a..666dea5b 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -40,7 +40,7 @@ class CreateShortUrlCommand extends Command private readonly UrlShortenerOptions $options, ) { parent::__construct(); - $this->defaultDomain = $this->options->domain()['hostname'] ?? ''; + $this->defaultDomain = $this->options->domain['hostname'] ?? ''; } protected function configure(): void @@ -158,7 +158,7 @@ class CreateShortUrlCommand extends Command $tags = unique(flatten(array_map($explodeWithComma, $input->getOption('tags')))); $customSlug = $input->getOption('custom-slug'); $maxVisits = $input->getOption('max-visits'); - $shortCodeLength = $input->getOption('short-code-length') ?? $this->options->defaultShortCodesLength(); + $shortCodeLength = $input->getOption('short-code-length') ?? $this->options->defaultShortCodesLength; $doValidateUrl = $input->getOption('validate-url'); try { @@ -175,7 +175,7 @@ class CreateShortUrlCommand extends Command ShortUrlInputFilter::TAGS => $tags, ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), - EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value => $this->options->multiSegmentSlugsEnabled(), + EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value => $this->options->multiSegmentSlugsEnabled, ])); $io->writeln([ diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index e529f0ad..733f6b72 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -39,7 +39,7 @@ class CreateShortUrlCommandTest extends TestCase $command = new CreateShortUrlCommand( $this->urlShortener->reveal(), $this->stringifier->reveal(), - new UrlShortenerOptions(['defaultShortCodesLength' => 5, 'domain' => ['hostname' => self::DEFAULT_DOMAIN]]), + new UrlShortenerOptions(domain: ['hostname' => self::DEFAULT_DOMAIN], defaultShortCodesLength: 5), ); $this->commandTester = $this->testerForCommand($command); } diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 79192c16..9855e2aa 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -25,7 +25,7 @@ return [ Options\DeleteShortUrlsOptions::class => [ValinorConfigFactory::class, 'config.delete_short_urls'], Options\NotFoundRedirectOptions::class => [ValinorConfigFactory::class, 'config.not_found_redirects'], Options\RedirectOptions::class => [ValinorConfigFactory::class, 'config.redirects'], - Options\UrlShortenerOptions::class => ConfigAbstractFactory::class, + Options\UrlShortenerOptions::class => [ValinorConfigFactory::class, 'config.url_shortener'], Options\TrackingOptions::class => [ValinorConfigFactory::class, 'config.tracking'], Options\QrCodeOptions::class => [ValinorConfigFactory::class, 'config.qr_codes'], Options\RabbitMqOptions::class => [ValinorConfigFactory::class, 'config.rabbitmq'], @@ -86,7 +86,6 @@ return [ Domain\DomainService::class, ], - Options\UrlShortenerOptions::class => ['config.url_shortener'], Options\WebhookOptions::class => ['config.visits_webhooks'], Service\UrlShortener::class => [ diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 38e185c2..dd7fdc8d 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -4,69 +4,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Options; -use Laminas\Stdlib\AbstractOptions; - use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; -class UrlShortenerOptions extends AbstractOptions +final class UrlShortenerOptions { - protected $__strictMode__ = false; // phpcs:ignore - - private array $domain = []; - private int $defaultShortCodesLength = DEFAULT_SHORT_CODES_LENGTH; - private bool $autoResolveTitles = false; - private bool $appendExtraPath = false; - private bool $multiSegmentSlugsEnabled = false; - - public function domain(): array - { - return $this->domain; - } - - protected function setDomain(array $domain): self - { - $this->domain = $domain; - return $this; - } - - public function defaultShortCodesLength(): int - { - return $this->defaultShortCodesLength; - } - - protected function setDefaultShortCodesLength(int $defaultShortCodesLength): self - { - $this->defaultShortCodesLength = $defaultShortCodesLength; - return $this; - } - - public function autoResolveTitles(): bool - { - return $this->autoResolveTitles; - } - - protected function setAutoResolveTitles(bool $autoResolveTitles): void - { - $this->autoResolveTitles = $autoResolveTitles; - } - - public function appendExtraPath(): bool - { - return $this->appendExtraPath; - } - - protected function setAppendExtraPath(bool $appendExtraPath): void - { - $this->appendExtraPath = $appendExtraPath; - } - - public function multiSegmentSlugsEnabled(): bool - { - return $this->multiSegmentSlugsEnabled; - } - - protected function setMultiSegmentSlugsEnabled(bool $multiSegmentSlugsEnabled): void - { - $this->multiSegmentSlugsEnabled = $multiSegmentSlugsEnabled; + public function __construct( + /** @var array{schema: ?string, hostname: ?string} */ + public readonly array $domain = ['schema' => null, 'hostname' => null], + public readonly int $defaultShortCodesLength = DEFAULT_SHORT_CODES_LENGTH, + public readonly bool $autoResolveTitles = false, + public readonly bool $appendExtraPath = false, + public readonly bool $multiSegmentSlugsEnabled = false, + ) { } } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index bb350aa2..3fead5f2 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -49,16 +49,16 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface private function shouldApplyLogic(?NotFoundType $notFoundType): bool { - if ($notFoundType === null || ! $this->urlShortenerOptions->appendExtraPath()) { + if ($notFoundType === null || ! $this->urlShortenerOptions->appendExtraPath) { return false; } return ( // If multi-segment slugs are enabled, the appropriate not-found type is "invalid_short_url" - $this->urlShortenerOptions->multiSegmentSlugsEnabled() && $notFoundType->isInvalidShortUrl() + $this->urlShortenerOptions->multiSegmentSlugsEnabled && $notFoundType->isInvalidShortUrl() ) || ( // If multi-segment slugs are disabled, the appropriate not-found type is "regular_404" - ! $this->urlShortenerOptions->multiSegmentSlugsEnabled() && $notFoundType->isRegularNotFound() + ! $this->urlShortenerOptions->multiSegmentSlugsEnabled && $notFoundType->isRegularNotFound() ); } @@ -79,7 +79,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } catch (ShortUrlNotFoundException) { - if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled()) { + if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled) { return $handler->handle($request); } diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 2e2965b1..0057660a 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -46,11 +46,11 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface public function validateUrlWithTitle(string $url, bool $doValidate): ?string { - if (! $doValidate && ! $this->options->autoResolveTitles()) { + if (! $doValidate && ! $this->options->autoResolveTitles) { return null; } - if (! $this->options->autoResolveTitles()) { + if (! $this->options->autoResolveTitles) { $this->validateUrlAndGetResponse($url, self::METHOD_HEAD); return null; } diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 4099faea..3cd2adef 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -34,12 +34,10 @@ class ExtraPathRedirectMiddlewareTest extends TestCase { use ProphecyTrait; - private ExtraPathRedirectMiddleware $middleware; private ObjectProphecy $resolver; private ObjectProphecy $requestTracker; private ObjectProphecy $redirectionBuilder; private ObjectProphecy $redirectResponseHelper; - private UrlShortenerOptions $options; private ObjectProphecy $handler; protected function setUp(): void @@ -48,16 +46,6 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $this->requestTracker = $this->prophesize(RequestTrackerInterface::class); $this->redirectionBuilder = $this->prophesize(ShortUrlRedirectionBuilderInterface::class); $this->redirectResponseHelper = $this->prophesize(RedirectResponseHelperInterface::class); - $this->options = new UrlShortenerOptions(['append_extra_path' => true]); - - $this->middleware = new ExtraPathRedirectMiddleware( - $this->resolver->reveal(), - $this->requestTracker->reveal(), - $this->redirectionBuilder->reveal(), - $this->redirectResponseHelper->reveal(), - $this->options, - ); - $this->handler = $this->prophesize(RequestHandlerInterface::class); $this->handler->handle(Argument::cetera())->willReturn(new RedirectResponse('')); } @@ -71,10 +59,12 @@ class ExtraPathRedirectMiddlewareTest extends TestCase bool $multiSegmentEnabled, ServerRequestInterface $request, ): void { - $this->options->appendExtraPath = $appendExtraPath; - $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; + $options = new UrlShortenerOptions( + appendExtraPath: $appendExtraPath, + multiSegmentSlugsEnabled: $multiSegmentEnabled, + ); - $this->middleware->process($request, $this->handler->reveal()); + $this->middleware($options)->process($request, $this->handler->reveal()); $this->handler->handle($request)->shouldHaveBeenCalledOnce(); $this->resolver->resolveEnabledShortUrl(Argument::cetera())->shouldNotHaveBeenCalled(); @@ -123,7 +113,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase bool $multiSegmentEnabled, int $expectedResolveCalls, ): void { - $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; + $options = new UrlShortenerOptions(appendExtraPath: true, multiSegmentSlugsEnabled: $multiSegmentEnabled); $type = $this->prophesize(NotFoundType::class); $type->isRegularNotFound()->willReturn(true); @@ -135,7 +125,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase Argument::that(fn (ShortUrlIdentifier $identifier) => str_starts_with($identifier->shortCode, 'shortCode')), )->willThrow(ShortUrlNotFoundException::class); - $this->middleware->process($request, $this->handler->reveal()); + $this->middleware($options)->process($request, $this->handler->reveal()); $resolve->shouldHaveBeenCalledTimes($expectedResolveCalls); $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldNotHaveBeenCalled(); @@ -152,7 +142,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase int $expectedResolveCalls, ?string $expectedExtraPath, ): void { - $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; + $options = new UrlShortenerOptions(appendExtraPath: true, multiSegmentSlugsEnabled: $multiSegmentEnabled); $type = $this->prophesize(NotFoundType::class); $type->isRegularNotFound()->willReturn(true); @@ -181,7 +171,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase new RedirectResponse(''), ); - $this->middleware->process($request, $this->handler->reveal()); + $this->middleware($options)->process($request, $this->handler->reveal()); $resolve->shouldHaveBeenCalledTimes($expectedResolveCalls); $buildLongUrl->shouldHaveBeenCalledOnce(); @@ -194,4 +184,15 @@ class ExtraPathRedirectMiddlewareTest extends TestCase yield [false, 1, '/bar/baz']; yield [true, 3, null]; } + + private function middleware(?UrlShortenerOptions $options = null): ExtraPathRedirectMiddleware + { + return new ExtraPathRedirectMiddleware( + $this->resolver->reveal(), + $this->requestTracker->reveal(), + $this->redirectionBuilder->reveal(), + $this->redirectResponseHelper->reveal(), + $options ?? new UrlShortenerOptions(appendExtraPath: true), + ); + } } diff --git a/module/Core/test/Util/UrlValidatorTest.php b/module/Core/test/Util/UrlValidatorTest.php index de5cad23..cc13bd2c 100644 --- a/module/Core/test/Util/UrlValidatorTest.php +++ b/module/Core/test/Util/UrlValidatorTest.php @@ -23,15 +23,11 @@ class UrlValidatorTest extends TestCase { use ProphecyTrait; - private UrlValidator $urlValidator; private ObjectProphecy $httpClient; - private UrlShortenerOptions $options; protected function setUp(): void { $this->httpClient = $this->prophesize(ClientInterface::class); - $this->options = new UrlShortenerOptions(['validate_url' => true]); - $this->urlValidator = new UrlValidator($this->httpClient->reveal(), $this->options); } /** @test */ @@ -42,7 +38,7 @@ class UrlValidatorTest extends TestCase $request->shouldBeCalledOnce(); $this->expectException(InvalidUrlException::class); - $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar', true); + $this->urlValidator()->validateUrl('http://foobar.com/12345/hello?foo=bar', true); } /** @test */ @@ -65,7 +61,7 @@ class UrlValidatorTest extends TestCase }), )->willReturn(new Response()); - $this->urlValidator->validateUrl($expectedUrl, true); + $this->urlValidator()->validateUrl($expectedUrl, true); $request->shouldHaveBeenCalledOnce(); } @@ -75,7 +71,7 @@ class UrlValidatorTest extends TestCase { $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); - $this->urlValidator->validateUrl('', false); + $this->urlValidator()->validateUrl('', false); $request->shouldNotHaveBeenCalled(); } @@ -84,9 +80,8 @@ class UrlValidatorTest extends TestCase public function validateUrlWithTitleReturnsNullWhenRequestFailsAndValidationIsDisabled(): void { $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class); - $this->options->autoResolveTitles = true; - $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', false); + $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', false); self::assertNull($result); $request->shouldHaveBeenCalledOnce(); @@ -96,9 +91,8 @@ class UrlValidatorTest extends TestCase public function validateUrlWithTitleReturnsNullWhenAutoResolutionIsDisabled(): void { $request = $this->httpClient->request(Argument::cetera())->willReturn($this->respWithTitle()); - $this->options->autoResolveTitles = false; - $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', false); + $result = $this->urlValidator()->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', false); self::assertNull($result); $request->shouldNotHaveBeenCalled(); @@ -110,9 +104,8 @@ class UrlValidatorTest extends TestCase $request = $this->httpClient->request(RequestMethodInterface::METHOD_HEAD, Argument::cetera())->willReturn( $this->respWithTitle(), ); - $this->options->autoResolveTitles = false; - $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); + $result = $this->urlValidator()->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); self::assertNull($result); $request->shouldHaveBeenCalledOnce(); @@ -124,9 +117,8 @@ class UrlValidatorTest extends TestCase $request = $this->httpClient->request(RequestMethodInterface::METHOD_GET, Argument::cetera())->willReturn( $this->respWithTitle(), ); - $this->options->autoResolveTitles = true; - $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); + $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); self::assertEquals('Resolved "title"', $result); $request->shouldHaveBeenCalledOnce(); @@ -138,9 +130,8 @@ class UrlValidatorTest extends TestCase $request = $this->httpClient->request(RequestMethodInterface::METHOD_GET, Argument::cetera())->willReturn( new Response('php://memory', 200, ['Content-Type' => 'application/octet-stream']), ); - $this->options->autoResolveTitles = true; - $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); + $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); self::assertNull($result); $request->shouldHaveBeenCalledOnce(); @@ -152,9 +143,8 @@ class UrlValidatorTest extends TestCase $request = $this->httpClient->request(RequestMethodInterface::METHOD_GET, Argument::cetera())->willReturn( new Response($this->createStreamWithContent('No title'), 200, ['Content-Type' => 'text/html']), ); - $this->options->autoResolveTitles = true; - $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); + $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); self::assertNull($result); $request->shouldHaveBeenCalledOnce(); @@ -174,4 +164,12 @@ class UrlValidatorTest extends TestCase return $body; } + + public function urlValidator(bool $autoResolveTitles = false): UrlValidator + { + return new UrlValidator( + $this->httpClient->reveal(), + new UrlShortenerOptions(autoResolveTitles: $autoResolveTitles), + ); + } } diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index 376c6bec..46fff970 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -23,7 +23,7 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction { $payload = (array) $request->getParsedBody(); $payload[ShortUrlInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request); - $payload[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] = $this->urlShortenerOptions->multiSegmentSlugsEnabled(); + $payload[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] = $this->urlShortenerOptions->multiSegmentSlugsEnabled; return ShortUrlMeta::fromRawData($payload); }