From 42af057316cef4e85e2d157b4d65f7da1076a664 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 17 Sep 2022 15:36:40 +0200 Subject: [PATCH] Migrated RedirectOptions to immutable object --- module/Core/config/dependencies.config.php | 3 +- module/Core/src/Options/RedirectOptions.php | 37 +++++-------------- .../Core/src/Util/RedirectResponseHelper.php | 4 +- .../test/Util/RedirectResponseHelperTest.php | 19 ++++------ 4 files changed, 20 insertions(+), 43 deletions(-) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 8c06001e..79192c16 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -24,7 +24,7 @@ return [ Options\AppOptions::class => [ValinorConfigFactory::class, 'config.app_options'], Options\DeleteShortUrlsOptions::class => [ValinorConfigFactory::class, 'config.delete_short_urls'], Options\NotFoundRedirectOptions::class => [ValinorConfigFactory::class, 'config.not_found_redirects'], - Options\RedirectOptions::class => ConfigAbstractFactory::class, + Options\RedirectOptions::class => [ValinorConfigFactory::class, 'config.redirects'], Options\UrlShortenerOptions::class => ConfigAbstractFactory::class, Options\TrackingOptions::class => [ValinorConfigFactory::class, 'config.tracking'], Options\QrCodeOptions::class => [ValinorConfigFactory::class, 'config.qr_codes'], @@ -86,7 +86,6 @@ return [ Domain\DomainService::class, ], - Options\RedirectOptions::class => ['config.redirects'], Options\UrlShortenerOptions::class => ['config.url_shortener'], Options\WebhookOptions::class => ['config.visits_webhooks'], diff --git a/module/Core/src/Options/RedirectOptions.php b/module/Core/src/Options/RedirectOptions.php index 5479c59b..9a1fedac 100644 --- a/module/Core/src/Options/RedirectOptions.php +++ b/module/Core/src/Options/RedirectOptions.php @@ -4,40 +4,23 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Options; -use Laminas\Stdlib\AbstractOptions; - use function Functional\contains; use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; -class RedirectOptions extends AbstractOptions +final class RedirectOptions { - private int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE; - private int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME; + public readonly int $redirectStatusCode; + public readonly int $redirectCacheLifetime; - public function redirectStatusCode(): int - { - return $this->redirectStatusCode; - } - - protected function setRedirectStatusCode(int $redirectStatusCode): void - { - $this->redirectStatusCode = $this->normalizeRedirectStatusCode($redirectStatusCode); - } - - private function normalizeRedirectStatusCode(int $statusCode): int - { - return contains([301, 302], $statusCode) ? $statusCode : DEFAULT_REDIRECT_STATUS_CODE; - } - - public function redirectCacheLifetime(): int - { - return $this->redirectCacheLifetime; - } - - protected function setRedirectCacheLifetime(int $redirectCacheLifetime): void - { + public function __construct( + int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE, + int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME, + ) { + $this->redirectStatusCode = contains([301, 302], $redirectStatusCode) + ? $redirectStatusCode + : DEFAULT_REDIRECT_STATUS_CODE; $this->redirectCacheLifetime = $redirectCacheLifetime > 0 ? $redirectCacheLifetime : DEFAULT_REDIRECT_CACHE_LIFETIME; diff --git a/module/Core/src/Util/RedirectResponseHelper.php b/module/Core/src/Util/RedirectResponseHelper.php index 312c2a95..dfc87480 100644 --- a/module/Core/src/Util/RedirectResponseHelper.php +++ b/module/Core/src/Util/RedirectResponseHelper.php @@ -19,9 +19,9 @@ class RedirectResponseHelper implements RedirectResponseHelperInterface public function buildRedirectResponse(string $location): ResponseInterface { - $statusCode = $this->options->redirectStatusCode(); + $statusCode = $this->options->redirectStatusCode; $headers = $statusCode === StatusCodeInterface::STATUS_FOUND ? [] : [ - 'Cache-Control' => sprintf('private,max-age=%s', $this->options->redirectCacheLifetime()), + 'Cache-Control' => sprintf('private,max-age=%s', $this->options->redirectCacheLifetime), ]; return new RedirectResponse($location, $statusCode, $headers); diff --git a/module/Core/test/Util/RedirectResponseHelperTest.php b/module/Core/test/Util/RedirectResponseHelperTest.php index 651d4bc7..fc2b89a2 100644 --- a/module/Core/test/Util/RedirectResponseHelperTest.php +++ b/module/Core/test/Util/RedirectResponseHelperTest.php @@ -11,15 +11,6 @@ use Shlinkio\Shlink\Core\Util\RedirectResponseHelper; class RedirectResponseHelperTest extends TestCase { - private RedirectResponseHelper $helper; - private RedirectOptions $shortenerOpts; - - protected function setUp(): void - { - $this->shortenerOpts = new RedirectOptions(); - $this->helper = new RedirectResponseHelper($this->shortenerOpts); - } - /** * @test * @dataProvider provideRedirectConfigs @@ -30,10 +21,9 @@ class RedirectResponseHelperTest extends TestCase int $expectedStatus, ?string $expectedCacheControl, ): void { - $this->shortenerOpts->redirectStatusCode = $configuredStatus; - $this->shortenerOpts->redirectCacheLifetime = $configuredLifetime; + $options = new RedirectOptions($configuredStatus, $configuredLifetime); - $response = $this->helper->buildRedirectResponse('destination'); + $response = $this->helper($options)->buildRedirectResponse('destination'); self::assertInstanceOf(RedirectResponse::class, $response); self::assertEquals($expectedStatus, $response->getStatusCode()); @@ -52,4 +42,9 @@ class RedirectResponseHelperTest extends TestCase yield 'status 301 with zero expiration' => [301, 0, 301, 'private,max-age=30']; yield 'status 301 with negative expiration' => [301, -20, 301, 'private,max-age=30']; } + + private function helper(?RedirectOptions $options = null): RedirectResponseHelper + { + return new RedirectResponseHelper($options ?? new RedirectOptions()); + } }