Updated RedirectAction so that it makes use of the not found short url options

This commit is contained in:
Alejandro Celaya 2018-11-03 12:10:02 +01:00
parent 358b2b661e
commit 313927827d
6 changed files with 100 additions and 23 deletions

View file

@ -63,6 +63,7 @@ return [
Service\UrlShortener::class, Service\UrlShortener::class,
Service\VisitsTracker::class, Service\VisitsTracker::class,
Options\AppOptions::class, Options\AppOptions::class,
Options\NotFoundShortUrlOptions::class,
'Logger_Shlink', 'Logger_Shlink',
], ],
Action\PixelAction::class => [ Action\PixelAction::class => [

View file

@ -9,7 +9,6 @@ use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface; use Psr\Http\Server\RequestHandlerInterface;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger; use Psr\Log\NullLogger;
use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\Visitor;
@ -20,8 +19,6 @@ use function array_key_exists;
abstract class AbstractTrackingAction implements MiddlewareInterface abstract class AbstractTrackingAction implements MiddlewareInterface
{ {
use ErrorResponseBuilderTrait;
/** /**
* @var UrlShortenerInterface * @var UrlShortenerInterface
*/ */
@ -74,12 +71,17 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
$this->visitTracker->track($shortCode, Visitor::fromRequest($request)); $this->visitTracker->track($shortCode, Visitor::fromRequest($request));
} }
return $this->createResp($url->getLongUrl()); return $this->createSuccessResp($url->getLongUrl());
} catch (InvalidShortCodeException | EntityDoesNotExistException $e) { } catch (InvalidShortCodeException | EntityDoesNotExistException $e) {
$this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $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;
} }

View file

@ -4,12 +4,21 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Action; namespace Shlinkio\Shlink\Core\Action;
use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Common\Response\PixelResponse; use Shlinkio\Shlink\Common\Response\PixelResponse;
class PixelAction extends AbstractTrackingAction class PixelAction extends AbstractTrackingAction
{ {
protected function createResp(string $longUrl): ResponseInterface protected function createSuccessResp(string $longUrl): ResponseInterface
{ {
return new PixelResponse(); return new PixelResponse();
} }
protected function createErrorResp(
ServerRequestInterface $request,
RequestHandlerInterface $handler
): ResponseInterface {
return new PixelResponse();
}
} }

View file

@ -4,14 +4,50 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Action; namespace Shlinkio\Shlink\Core\Action;
use Psr\Http\Message\ResponseInterface as Response; 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; use Zend\Diactoros\Response\RedirectResponse;
class RedirectAction extends AbstractTrackingAction 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. // Return a redirect response to the long URL.
// Use a temporary redirect to make sure browsers always hit the server for analytics purposes // Use a temporary redirect to make sure browsers always hit the server for analytics purposes
return new RedirectResponse($longUrl); 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);
}
} }

View file

@ -21,18 +21,18 @@ class NotFoundShortUrlOptions extends AbstractOptions
return $this->enableRedirection; return $this->enableRedirection;
} }
protected function enableRedirection(bool $enableRedirection = true): self protected function setEnableRedirection(bool $enableRedirection = true): self
{ {
$this->enableRedirection = $enableRedirection; $this->enableRedirection = $enableRedirection;
return $this; 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; $this->redirectTo = $redirectTo;
return $this; return $this;

View file

@ -5,13 +5,12 @@ namespace ShlinkioTest\Shlink\Core\Action;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Prophecy\Argument; use Prophecy\Argument;
use Prophecy\Prophecy\MethodProphecy;
use Prophecy\Prophecy\ObjectProphecy; use Prophecy\Prophecy\ObjectProphecy;
use Psr\Http\Server\RequestHandlerInterface; use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Action\RedirectAction;
use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; 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\UrlShortener;
use Shlinkio\Shlink\Core\Service\VisitsTracker; use Shlinkio\Shlink\Core\Service\VisitsTracker;
use ShlinkioTest\Shlink\Common\Util\TestUtils; use ShlinkioTest\Shlink\Common\Util\TestUtils;
@ -23,25 +22,31 @@ class RedirectActionTest extends TestCase
/** /**
* @var RedirectAction * @var RedirectAction
*/ */
protected $action; private $action;
/** /**
* @var ObjectProphecy * @var ObjectProphecy
*/ */
protected $urlShortener; private $urlShortener;
/** /**
* @var ObjectProphecy * @var ObjectProphecy
*/ */
protected $visitTracker; private $visitTracker;
/**
* @var Options\NotFoundShortUrlOptions
*/
private $notFoundOptions;
public function setUp() public function setUp()
{ {
$this->urlShortener = $this->prophesize(UrlShortener::class); $this->urlShortener = $this->prophesize(UrlShortener::class);
$this->visitTracker = $this->prophesize(VisitsTracker::class); $this->visitTracker = $this->prophesize(VisitsTracker::class);
$this->notFoundOptions = new Options\NotFoundShortUrlOptions();
$this->action = new RedirectAction( $this->action = new RedirectAction(
$this->urlShortener->reveal(), $this->urlShortener->reveal(),
$this->visitTracker->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); ->shouldBeCalledTimes(1);
$this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled();
$delegate = $this->prophesize(RequestHandlerInterface::class); $handler = $this->prophesize(RequestHandlerInterface::class);
/** @var MethodProphecy $process */ $handle = $handler->handle(Argument::any())->willReturn(new Response());
$process = $delegate->handle(Argument::any())->willReturn(new Response());
$request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $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();
} }
/** /**