From 0bea843e7f25551f21202c60632218691b87ab7f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 20 Jun 2020 09:50:56 +0200 Subject: [PATCH] Added test covering how redirects config works --- module/Core/src/Action/RedirectAction.php | 1 + .../Core/src/Options/UrlShortenerOptions.php | 9 ++-- .../Core/test/Action/RedirectActionTest.php | 44 ++++++++++++++++++- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 72a56096..80fef898 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -13,6 +13,7 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; + use function sprintf; class RedirectAction extends AbstractTrackingAction implements StatusCodeInterface diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 69a06f1f..92bb7d07 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -8,6 +8,7 @@ use Laminas\Stdlib\AbstractOptions; use function Functional\contains; +use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; class UrlShortenerOptions extends AbstractOptions @@ -16,7 +17,7 @@ class UrlShortenerOptions extends AbstractOptions private bool $validateUrl = true; private int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE; - private int $redirectCacheLifetime = 30; + private int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME; public function isUrlValidationEnabled(): bool { @@ -40,7 +41,7 @@ class UrlShortenerOptions extends AbstractOptions private function normalizeRedirectStatusCode(int $statusCode): int { - return contains([301, 302], $statusCode) ? $statusCode : 302; + return contains([301, 302], $statusCode) ? $statusCode : DEFAULT_REDIRECT_STATUS_CODE; } public function redirectCacheLifetime(): int @@ -50,6 +51,8 @@ class UrlShortenerOptions extends AbstractOptions protected function setRedirectCacheLifetime(int $redirectCacheLifetime): void { - $this->redirectCacheLifetime = $redirectCacheLifetime; + $this->redirectCacheLifetime = $redirectCacheLifetime > 0 + ? $redirectCacheLifetime + : DEFAULT_REDIRECT_CACHE_LIFETIME; } } diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index f4de05c5..1a6bb617 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -27,16 +27,19 @@ class RedirectActionTest extends TestCase private RedirectAction $action; private ObjectProphecy $urlResolver; private ObjectProphecy $visitTracker; + private Options\UrlShortenerOptions $shortenerOpts; public function setUp(): void { $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); $this->visitTracker = $this->prophesize(VisitsTrackerInterface::class); + $this->shortenerOpts = new Options\UrlShortenerOptions(); $this->action = new RedirectAction( $this->urlResolver->reveal(), $this->visitTracker->reveal(), new Options\AppOptions(['disableTrackParam' => 'foobar']), + $this->shortenerOpts, ); } @@ -48,8 +51,9 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing'); - $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) - ->willReturn($shortUrl); + $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl( + new ShortUrlIdentifier($shortCode, ''), + )->willReturn($shortUrl); $track = $this->visitTracker->track(Argument::cetera())->will(function (): void { }); @@ -110,4 +114,40 @@ class RedirectActionTest extends TestCase $track->shouldNotHaveBeenCalled(); } + + /** + * @test + * @dataProvider provideRedirectConfigs + */ + public function expectedStatusCodeAndCacheIsReturnedBasedOnConfig( + int $configuredStatus, + int $configuredLifetime, + int $expectedStatus, + ?string $expectedCacheControl + ): void { + $this->shortenerOpts->redirectStatusCode = $configuredStatus; + $this->shortenerOpts->redirectCacheLifetime = $configuredLifetime; + + $shortUrl = new ShortUrl('http://domain.com/foo/bar'); + $shortCode = $shortUrl->getShortCode(); + $this->urlResolver->resolveEnabledShortUrl(Argument::cetera())->willReturn($shortUrl); + + $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); + $response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); + + $this->assertInstanceOf(Response\RedirectResponse::class, $response); + $this->assertEquals($expectedStatus, $response->getStatusCode()); + $this->assertEquals($response->hasHeader('Cache-Control'), $expectedCacheControl !== null); + $this->assertEquals($response->getHeaderLine('Cache-Control'), $expectedCacheControl ?? ''); + } + + public function provideRedirectConfigs(): iterable + { + yield 'status 302' => [302, 20, 302, null]; + yield 'status over 302' => [400, 20, 302, null]; + yield 'status below 301' => [201, 20, 302, null]; + yield 'status 301 with valid expiration' => [301, 20, 301, 'private,max-age=20']; + 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']; + } }