diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 665d0894..7042b19a 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -63,6 +63,7 @@ return [ Service\UrlShortener::class, Service\VisitsTracker::class, Options\AppOptions::class, + Options\NotFoundShortUrlOptions::class, 'Logger_Shlink', ], Action\PixelAction::class => [ diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 670aa908..21ab0b49 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -9,7 +9,6 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; -use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Model\Visitor; @@ -20,8 +19,6 @@ use function array_key_exists; abstract class AbstractTrackingAction implements MiddlewareInterface { - use ErrorResponseBuilderTrait; - /** * @var UrlShortenerInterface */ @@ -74,12 +71,17 @@ abstract class AbstractTrackingAction implements MiddlewareInterface $this->visitTracker->track($shortCode, Visitor::fromRequest($request)); } - return $this->createResp($url->getLongUrl()); + return $this->createSuccessResp($url->getLongUrl()); } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { $this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]); - return $this->buildErrorResponse($request, $handler); + return $this->createErrorResp($request, $handler); } } - abstract protected function createResp(string $longUrl): ResponseInterface; + abstract protected function createSuccessResp(string $longUrl): ResponseInterface; + + abstract protected function createErrorResp( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ): ResponseInterface; } diff --git a/module/Core/src/Action/PixelAction.php b/module/Core/src/Action/PixelAction.php index 5f2c797a..fff85ce3 100644 --- a/module/Core/src/Action/PixelAction.php +++ b/module/Core/src/Action/PixelAction.php @@ -4,12 +4,21 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Action; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\PixelResponse; class PixelAction extends AbstractTrackingAction { - protected function createResp(string $longUrl): ResponseInterface + protected function createSuccessResp(string $longUrl): ResponseInterface { return new PixelResponse(); } + + protected function createErrorResp( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ): ResponseInterface { + return new PixelResponse(); + } } diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 4654e1fe..83c4312c 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -4,14 +4,50 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Action; use Psr\Http\Message\ResponseInterface as Response; +use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; +use Shlinkio\Shlink\Core\Options; +use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; +use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Zend\Diactoros\Response\RedirectResponse; class RedirectAction extends AbstractTrackingAction { - protected function createResp(string $longUrl): Response + use ErrorResponseBuilderTrait; + + /** + * @var Options\NotFoundShortUrlOptions + */ + private $notFoundOptions; + + public function __construct( + UrlShortenerInterface $urlShortener, + VisitsTrackerInterface $visitTracker, + Options\AppOptions $appOptions, + Options\NotFoundShortUrlOptions $notFoundOptions, + LoggerInterface $logger = null + ) { + parent::__construct($urlShortener, $visitTracker, $appOptions, $logger); + $this->notFoundOptions = $notFoundOptions; + } + + protected function createSuccessResp(string $longUrl): Response { // Return a redirect response to the long URL. // Use a temporary redirect to make sure browsers always hit the server for analytics purposes return new RedirectResponse($longUrl); } + + protected function createErrorResp( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ): Response { + if ($this->notFoundOptions->isRedirectionEnabled()) { + return new RedirectResponse($this->notFoundOptions->getRedirectTo()); + } + + return $this->buildErrorResponse($request, $handler); + } } diff --git a/module/Core/src/Options/NotFoundShortUrlOptions.php b/module/Core/src/Options/NotFoundShortUrlOptions.php index 8e461240..871c8eff 100644 --- a/module/Core/src/Options/NotFoundShortUrlOptions.php +++ b/module/Core/src/Options/NotFoundShortUrlOptions.php @@ -21,18 +21,18 @@ class NotFoundShortUrlOptions extends AbstractOptions return $this->enableRedirection; } - protected function enableRedirection(bool $enableRedirection = true): self + protected function setEnableRedirection(bool $enableRedirection = true): self { $this->enableRedirection = $enableRedirection; return $this; } - public function getRedirectTo(): ?string + public function getRedirectTo(): string { - return $this->redirectTo; + return $this->redirectTo ?? ''; } - protected function setRedirectTo(string $redirectTo): self + protected function setRedirectTo(?string $redirectTo): self { $this->redirectTo = $redirectTo; return $this; diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 91e1b4d6..5de290f7 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -5,13 +5,12 @@ namespace ShlinkioTest\Shlink\Core\Action; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; -use Shlinkio\Shlink\Core\Options\AppOptions; +use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\VisitsTracker; use ShlinkioTest\Shlink\Common\Util\TestUtils; @@ -23,25 +22,31 @@ class RedirectActionTest extends TestCase /** * @var RedirectAction */ - protected $action; + private $action; /** * @var ObjectProphecy */ - protected $urlShortener; + private $urlShortener; /** * @var ObjectProphecy */ - protected $visitTracker; + private $visitTracker; + /** + * @var Options\NotFoundShortUrlOptions + */ + private $notFoundOptions; public function setUp() { $this->urlShortener = $this->prophesize(UrlShortener::class); $this->visitTracker = $this->prophesize(VisitsTracker::class); + $this->notFoundOptions = new Options\NotFoundShortUrlOptions(); $this->action = new RedirectAction( $this->urlShortener->reveal(), $this->visitTracker->reveal(), - new AppOptions(['disableTrackParam' => 'foobar']) + new Options\AppOptions(['disableTrackParam' => 'foobar']), + $this->notFoundOptions ); } @@ -76,14 +81,38 @@ class RedirectActionTest extends TestCase ->shouldBeCalledTimes(1); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); - $delegate = $this->prophesize(RequestHandlerInterface::class); - /** @var MethodProphecy $process */ - $process = $delegate->handle(Argument::any())->willReturn(new Response()); + $handler = $this->prophesize(RequestHandlerInterface::class); + $handle = $handler->handle(Argument::any())->willReturn(new Response()); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $this->action->process($request, $delegate->reveal()); + $this->action->process($request, $handler->reveal()); - $process->shouldHaveBeenCalledTimes(1); + $handle->shouldHaveBeenCalledTimes(1); + } + + /** + * @test + */ + public function redirectToCustomUrlIsReturnedIfConfiguredSoAndShortUrlIsNotFound() + { + $shortCode = 'abc123'; + $shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode)->willThrow( + EntityDoesNotExistException::class + ); + + $handler = $this->prophesize(RequestHandlerInterface::class); + $handle = $handler->handle(Argument::any())->willReturn(new Response()); + + $this->notFoundOptions->enableRedirection = true; + $this->notFoundOptions->redirectTo = 'https://shlink.io'; + + $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); + $resp = $this->action->process($request, $handler->reveal()); + + $this->assertEquals(302, $resp->getStatusCode()); + $this->assertEquals('https://shlink.io', $resp->getHeaderLine('Location')); + $shortCodeToUrl->shouldHaveBeenCalledTimes(1); + $handle->shouldNotHaveBeenCalled(); } /**