Moved InvalidShortCode exception handling to problem details

This commit is contained in:
Alejandro Celaya 2019-11-24 12:41:12 +01:00
parent 09321eaa93
commit 6f0afe269d
12 changed files with 113 additions and 85 deletions

View file

@ -8,6 +8,14 @@ use Zend\Stratigility\Middleware\ErrorHandler;
return [
'backwards_compatible_problem_details' => [
'default_type_fallbacks' => [
404 => 'NOT_FOUND',
500 => 'INTERNAL_SERVER_ERROR',
],
'json_flags' => JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE | JSON_PRESERVE_ZERO_FRACTION,
],
'error_handler' => [
'listeners' => [Logger\ErrorLogger::class],
],

View file

@ -13,19 +13,20 @@ return [
'middleware_pipeline' => [
'error-handler' => [
'middleware' => [
Expressive\Helper\ContentLengthMiddleware::class,
ErrorHandler::class,
],
],
'error-handler-rest' => [
'path' => '/rest',
'middleware' => [
Rest\Middleware\BackwardsCompatibleProblemDetailsMiddleware::class,
ProblemDetails\ProblemDetailsMiddleware::class,
],
],
'pre-routing' => [
'middleware' => [
Expressive\Helper\ContentLengthMiddleware::class,
Common\Middleware\CloseDbConnectionMiddleware::class,
],
],

View file

@ -14,20 +14,17 @@ class InvalidShortCodeException extends RuntimeException implements ProblemDetai
{
use CommonProblemDetailsExceptionTrait;
public const TITLE = 'Invalid short code';
private const TITLE = 'Invalid short code';
public const TYPE = 'INVALID_SHORTCODE';
public static function fromNotFoundShortCode(string $shortCode): self
{
$e = new self(sprintf('No URL found for short code "%s"', $shortCode));
$e->detail = $e->getMessage();
$e->title = self::TITLE;
$e->type = self::TYPE;
$e->status = StatusCodeInterface::STATUS_NOT_FOUND;
$e->additional = [
'error' => $e->type,
'message' => $e->detail,
];
return $e;
}

View file

@ -39,6 +39,7 @@ return [
Middleware\BodyParserMiddleware::class => InvokableFactory::class,
Middleware\CrossDomainMiddleware::class => InvokableFactory::class,
Middleware\PathVersionMiddleware::class => InvokableFactory::class,
Middleware\BackwardsCompatibleProblemDetailsMiddleware::class => ConfigAbstractFactory::class,
Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class,
Middleware\ShortUrl\ShortCodePathMiddleware::class => InvokableFactory::class,
],
@ -75,6 +76,11 @@ return [
Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class],
Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class],
Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, LoggerInterface::class],
Middleware\BackwardsCompatibleProblemDetailsMiddleware::class => [
'config.backwards_compatible_problem_details.default_type_fallbacks',
'config.backwards_compatible_problem_details.json_flags',
],
],
];

View file

@ -15,8 +15,6 @@ use Shlinkio\Shlink\Rest\Util\RestUtils;
use Zend\Diactoros\Response\EmptyResponse;
use Zend\Diactoros\Response\JsonResponse;
use function sprintf;
class EditShortUrlAction extends AbstractRestAction
{
protected const ROUTE_PATH = '/short-urls/{shortCode}';
@ -51,12 +49,6 @@ class EditShortUrlAction extends AbstractRestAction
ShortUrlMeta::createFromRawData($postData)
);
return new EmptyResponse();
} catch (Exception\InvalidShortCodeException $e) {
$this->logger->warning('Provided data is invalid. {e}', ['e' => $e]);
return new JsonResponse([
'error' => RestUtils::getRestErrorCodeFromException($e),
'message' => sprintf('No URL found for short code "%s"', $shortCode),
], self::STATUS_NOT_FOUND);
} catch (Exception\ValidationException $e) {
$this->logger->warning('Provided data is invalid. {e}', ['e' => $e]);
return new JsonResponse([

View file

@ -7,14 +7,11 @@ namespace Shlinkio\Shlink\Rest\Action\ShortUrl;
use Psr\Http\Message\ResponseInterface as Response;
use Psr\Http\Message\ServerRequestInterface as Request;
use Psr\Log\LoggerInterface;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface;
use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
use Shlinkio\Shlink\Rest\Util\RestUtils;
use Zend\Diactoros\Response\JsonResponse;
use function sprintf;
class EditShortUrlTagsAction extends AbstractRestAction
{
protected const ROUTE_PATH = '/short-urls/{shortCode}/tags';
@ -47,14 +44,7 @@ class EditShortUrlTagsAction extends AbstractRestAction
}
$tags = $bodyParams['tags'];
try {
$shortUrl = $this->shortUrlService->setTagsByShortCode($shortCode, $tags);
return new JsonResponse(['tags' => $shortUrl->getTags()->toArray()]);
} catch (InvalidShortCodeException $e) {
return new JsonResponse([
'error' => RestUtils::getRestErrorCodeFromException($e),
'message' => sprintf('No URL found for short code "%s"', $shortCode),
], self::STATUS_NOT_FOUND);
}
$shortUrl = $this->shortUrlService->setTagsByShortCode($shortCode, $tags);
return new JsonResponse(['tags' => $shortUrl->getTags()->toArray()]);
}
}

View file

@ -33,11 +33,6 @@ class GetVisitsAction extends AbstractRestAction
$this->visitsTracker = $visitsTracker;
}
/**
* @param Request $request
* @return Response
* @throws \InvalidArgumentException
*/
public function handle(Request $request): Response
{
$shortCode = $request->getAttribute('shortCode');

View file

@ -11,7 +11,7 @@ use function sprintf;
class ConfigProvider
{
private const ROUTES_PREFIX = '/rest/v{version:1}';
private const ROUTES_PREFIX = '/rest/v{version:1|2}';
public function __invoke()
{

View file

@ -0,0 +1,87 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Rest\Middleware;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Throwable;
use Zend\Diactoros\Response\JsonResponse;
use function Functional\reduce_left;
use function Shlinkio\Shlink\Common\json_decode;
use function strpos;
class BackwardsCompatibleProblemDetailsMiddleware implements MiddlewareInterface
{
private const BACKWARDS_COMPATIBLE_FIELDS = [
'error' => 'type',
'message' => 'detail',
];
/** @var array */
private $defaultTypeFallbacks;
/** @var int */
private $jsonFlags;
public function __construct(array $defaultTypeFallbacks, int $jsonFlags)
{
$this->defaultTypeFallbacks = $defaultTypeFallbacks;
$this->jsonFlags = $jsonFlags;
}
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
$resp = $handler->handle($request);
if ($resp->getHeaderLine('Content-type') !== 'application/problem+json') {
return $resp;
}
try {
$body = (string) $resp->getBody();
$payload = json_decode($body);
} catch (Throwable $e) {
return $resp;
}
$payload = $this->mapStandardErrorTypes($payload, $resp->getStatusCode());
if ($this->isVersionOne($request)) {
$payload = $this->makePayloadBackwardsCompatible($payload);
}
return new JsonResponse($payload, $resp->getStatusCode(), $resp->getHeaders(), $this->jsonFlags);
}
private function mapStandardErrorTypes(array $payload, int $respStatusCode): array
{
$type = $payload['type'] ?? '';
if (strpos($type, 'https://httpstatus.es') === 0) {
$payload['type'] = $this->defaultTypeFallbacks[$respStatusCode] ?? $type;
}
return $payload;
}
/** @deprecated When Shlink 2 is released, do not chekc the version */
private function isVersionOne(ServerRequestInterface $request): bool
{
$uri = $request->getUri();
$path = $uri->getPath();
return strpos($path, '/v') === false || strpos($path, '/v1') === 0;
}
/** @deprecated When Shlink v2 is released, do not map old fields */
private function makePayloadBackwardsCompatible(array $payload): array
{
return reduce_left(self::BACKWARDS_COMPATIBLE_FIELDS, function (string $newKey, string $oldKey, $c, $acc) {
$acc[$oldKey] = $acc[$newKey];
return $acc;
}, $payload);
}
}

View file

@ -15,23 +15,12 @@ class PathVersionMiddleware implements MiddlewareInterface
{
// TODO The /health endpoint needs this middleware in order to work without the version.
// Take it into account if this middleware is ever removed.
/**
* Process an incoming server request and return a response, optionally delegating
* to the next middleware component to create the response.
*
* @param Request $request
* @param RequestHandlerInterface $handler
*
* @return Response
* @throws \InvalidArgumentException
*/
public function process(Request $request, RequestHandlerInterface $handler): Response
{
$uri = $request->getUri();
$path = $uri->getPath();
// If the path does not begin with the version number, prepend v1 by default for BC compatibility purposes
// If the path does not begin with the version number, prepend v1 by default for BC purposes
if (strpos($path, '/v') !== 0) {
$request = $request->withUri($uri->withPath('/v1' . $uri->getPath()));
}

View file

@ -8,7 +8,6 @@ use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface;
use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlAction;
use Shlinkio\Shlink\Rest\Util\RestUtils;
@ -29,7 +28,7 @@ class EditShortUrlActionTest extends TestCase
}
/** @test */
public function invalidDataReturnsError()
public function invalidDataReturnsError(): void
{
$request = (new ServerRequest())->withParsedBody([
'maxVisits' => 'invalid',
@ -45,28 +44,7 @@ class EditShortUrlActionTest extends TestCase
}
/** @test */
public function incorrectShortCodeReturnsError()
{
$request = (new ServerRequest())->withAttribute('shortCode', 'abc123')
->withParsedBody([
'maxVisits' => 5,
]);
$updateMeta = $this->shortUrlService->updateMetadataByShortCode(Argument::cetera())->willThrow(
InvalidShortCodeException::class
);
/** @var JsonResponse $resp */
$resp = $this->action->handle($request);
$payload = $resp->getPayload();
$this->assertEquals(404, $resp->getStatusCode());
$this->assertEquals(RestUtils::INVALID_SHORTCODE_ERROR, $payload['error']);
$this->assertEquals('No URL found for short code "abc123"', $payload['message']);
$updateMeta->shouldHaveBeenCalled();
}
/** @test */
public function correctShortCodeReturnsSuccess()
public function correctShortCodeReturnsSuccess(): void
{
$request = (new ServerRequest())->withAttribute('shortCode', 'abc123')
->withParsedBody([

View file

@ -7,7 +7,6 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl;
use PHPUnit\Framework\TestCase;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Service\ShortUrlService;
use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlTagsAction;
use Zend\Diactoros\ServerRequest;
@ -26,28 +25,14 @@ class EditShortUrlTagsActionTest extends TestCase
}
/** @test */
public function notProvidingTagsReturnsError()
public function notProvidingTagsReturnsError(): void
{
$response = $this->action->handle((new ServerRequest())->withAttribute('shortCode', 'abc123'));
$this->assertEquals(400, $response->getStatusCode());
}
/** @test */
public function anInvalidShortCodeReturnsNotFound()
{
$shortCode = 'abc123';
$this->shortUrlService->setTagsByShortCode($shortCode, [])->willThrow(InvalidShortCodeException::class)
->shouldBeCalledOnce();
$response = $this->action->handle(
(new ServerRequest())->withAttribute('shortCode', 'abc123')
->withParsedBody(['tags' => []])
);
$this->assertEquals(404, $response->getStatusCode());
}
/** @test */
public function tagsListIsReturnedIfCorrectShortCodeIsProvided()
public function tagsListIsReturnedIfCorrectShortCodeIsProvided(): void
{
$shortCode = 'abc123';
$this->shortUrlService->setTagsByShortCode($shortCode, [])->willReturn(new ShortUrl(''))