From 4fee656f966903205487a317b10a96c0a40ae27e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 1 May 2018 10:10:19 +0200 Subject: [PATCH 01/30] Prepared version 1.9.0 --- module/Rest/config/routes.config.php | 6 ++++++ module/Rest/src/Action/CreateShortcodeAction.php | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index a1bd8dca..28b7cf5f 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -21,6 +21,12 @@ return [ 'middleware' => Action\CreateShortcodeAction::class, 'allowed_methods' => [RequestMethod::METHOD_POST], ], +// [ +// 'name' => Action\CreateShortcodeAction::class, +// 'path' => '/short-codes', +// 'middleware' => Action\CreateShortcodeAction::class, +// 'allowed_methods' => [RequestMethod::METHOD_GET], +// ], [ 'name' => Action\EditShortCodeAction::class, 'path' => '/short-codes/{shortCode}', diff --git a/module/Rest/src/Action/CreateShortcodeAction.php b/module/Rest/src/Action/CreateShortcodeAction.php index b3c093d3..d880d552 100644 --- a/module/Rest/src/Action/CreateShortcodeAction.php +++ b/module/Rest/src/Action/CreateShortcodeAction.php @@ -80,7 +80,7 @@ class CreateShortcodeAction extends AbstractRestAction $this->logger->warning('Provided Invalid URL.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf( + 'message' => \sprintf( $this->translator->translate('Provided URL %s is invalid. Try with a different one.'), $longUrl ), @@ -89,7 +89,7 @@ class CreateShortcodeAction extends AbstractRestAction $this->logger->warning('Provided non-unique slug.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf( + 'message' => \sprintf( $this->translator->translate('Provided slug %s is already in use. Try with a different one.'), $customSlug ), From c9ce56eea5e68a8c5c6755f477b00da05ddf9193 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 1 May 2018 18:16:44 +0200 Subject: [PATCH 02/30] Added public method in AbstractRestAction which builds route definition --- module/Rest/src/Action/AbstractRestAction.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/module/Rest/src/Action/AbstractRestAction.php b/module/Rest/src/Action/AbstractRestAction.php index a9f491b6..1879ef12 100644 --- a/module/Rest/src/Action/AbstractRestAction.php +++ b/module/Rest/src/Action/AbstractRestAction.php @@ -11,6 +11,9 @@ use Psr\Log\NullLogger; abstract class AbstractRestAction implements RequestHandlerInterface, RequestMethodInterface, StatusCodeInterface { + protected const ROUTE_PATH = ''; + protected const ALLOWED_METHODS = []; + /** * @var LoggerInterface */ @@ -20,4 +23,14 @@ abstract class AbstractRestAction implements RequestHandlerInterface, RequestMet { $this->logger = $logger ?: new NullLogger(); } + + public static function getRouteDef(array $prevMiddleware = [], array $postMiddleware = []): array + { + return [ + 'name' => static::class, + 'middleware' => \array_merge($prevMiddleware, [static::class], $postMiddleware), + 'path' => static::ROUTE_PATH, + 'allowed_methods' => static::ALLOWED_METHODS, + ]; + } } From ef3c4aadf2c8584940bfb95334cb7d21aa8b1634 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 1 May 2018 18:28:37 +0200 Subject: [PATCH 03/30] Moved most of rest routes config to their actions --- module/Rest/config/routes.config.php | 70 +++---------------- module/Rest/src/Action/AbstractRestAction.php | 4 +- module/Rest/src/Action/AuthenticateAction.php | 3 + .../Rest/src/Action/EditShortCodeAction.php | 3 + .../src/Action/EditShortcodeTagsAction.php | 3 + module/Rest/src/Action/GetVisitsAction.php | 3 + .../Rest/src/Action/ListShortcodesAction.php | 3 + module/Rest/src/Action/ResolveUrlAction.php | 3 + .../Rest/src/Action/Tag/CreateTagsAction.php | 3 + .../Rest/src/Action/Tag/DeleteTagsAction.php | 3 + module/Rest/src/Action/Tag/ListTagsAction.php | 3 + .../Rest/src/Action/Tag/UpdateTagAction.php | 3 + 12 files changed, 42 insertions(+), 62 deletions(-) diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 28b7cf5f..e2ee8f75 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -7,12 +7,7 @@ use Shlinkio\Shlink\Rest\Action; return [ 'routes' => [ - [ - 'name' => Action\AuthenticateAction::class, - 'path' => '/authenticate', - 'middleware' => Action\AuthenticateAction::class, - 'allowed_methods' => [RequestMethod::METHOD_POST], - ], + Action\AuthenticateAction::getRouteDef(), // Short codes [ @@ -27,64 +22,19 @@ return [ // 'middleware' => Action\CreateShortcodeAction::class, // 'allowed_methods' => [RequestMethod::METHOD_GET], // ], - [ - 'name' => Action\EditShortCodeAction::class, - 'path' => '/short-codes/{shortCode}', - 'middleware' => Action\EditShortCodeAction::class, - 'allowed_methods' => [RequestMethod::METHOD_PUT], - ], - [ - 'name' => Action\ResolveUrlAction::class, - 'path' => '/short-codes/{shortCode}', - 'middleware' => Action\ResolveUrlAction::class, - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], - [ - 'name' => Action\ListShortcodesAction::class, - 'path' => '/short-codes', - 'middleware' => Action\ListShortcodesAction::class, - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], - [ - 'name' => Action\EditShortcodeTagsAction::class, - 'path' => '/short-codes/{shortCode}/tags', - 'middleware' => Action\EditShortcodeTagsAction::class, - 'allowed_methods' => [RequestMethod::METHOD_PUT], - ], + Action\EditShortCodeAction::getRouteDef(), + Action\ResolveUrlAction::getRouteDef(), + Action\ListShortcodesAction::getRouteDef(), + Action\EditShortcodeTagsAction::getRouteDef(), // Visits - [ - 'name' => Action\GetVisitsAction::class, - 'path' => '/short-codes/{shortCode}/visits', - 'middleware' => Action\GetVisitsAction::class, - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], + Action\GetVisitsAction::getRouteDef(), // Tags - [ - 'name' => Action\Tag\ListTagsAction::class, - 'path' => '/tags', - 'middleware' => Action\Tag\ListTagsAction::class, - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], - [ - 'name' => Action\Tag\DeleteTagsAction::class, - 'path' => '/tags', - 'middleware' => Action\Tag\DeleteTagsAction::class, - 'allowed_methods' => [RequestMethod::METHOD_DELETE], - ], - [ - 'name' => Action\Tag\CreateTagsAction::class, - 'path' => '/tags', - 'middleware' => Action\Tag\CreateTagsAction::class, - 'allowed_methods' => [RequestMethod::METHOD_POST], - ], - [ - 'name' => Action\Tag\UpdateTagAction::class, - 'path' => '/tags', - 'middleware' => Action\Tag\UpdateTagAction::class, - 'allowed_methods' => [RequestMethod::METHOD_PUT], - ], + Action\Tag\ListTagsAction::getRouteDef(), + Action\Tag\DeleteTagsAction::getRouteDef(), + Action\Tag\CreateTagsAction::getRouteDef(), + Action\Tag\UpdateTagAction::getRouteDef(), ], ]; diff --git a/module/Rest/src/Action/AbstractRestAction.php b/module/Rest/src/Action/AbstractRestAction.php index 1879ef12..caf4f459 100644 --- a/module/Rest/src/Action/AbstractRestAction.php +++ b/module/Rest/src/Action/AbstractRestAction.php @@ -12,7 +12,7 @@ use Psr\Log\NullLogger; abstract class AbstractRestAction implements RequestHandlerInterface, RequestMethodInterface, StatusCodeInterface { protected const ROUTE_PATH = ''; - protected const ALLOWED_METHODS = []; + protected const ROUTE_ALLOWED_METHODS = []; /** * @var LoggerInterface @@ -30,7 +30,7 @@ abstract class AbstractRestAction implements RequestHandlerInterface, RequestMet 'name' => static::class, 'middleware' => \array_merge($prevMiddleware, [static::class], $postMiddleware), 'path' => static::ROUTE_PATH, - 'allowed_methods' => static::ALLOWED_METHODS, + 'allowed_methods' => static::ROUTE_ALLOWED_METHODS, ]; } } diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index 22dcee43..3a4d2fba 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -15,6 +15,9 @@ use Zend\I18n\Translator\TranslatorInterface; class AuthenticateAction extends AbstractRestAction { + protected const ROUTE_PATH = '/authenticate'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_POST]; + /** * @var TranslatorInterface */ diff --git a/module/Rest/src/Action/EditShortCodeAction.php b/module/Rest/src/Action/EditShortCodeAction.php index 3e9be062..a3a2357f 100644 --- a/module/Rest/src/Action/EditShortCodeAction.php +++ b/module/Rest/src/Action/EditShortCodeAction.php @@ -16,6 +16,9 @@ use Zend\I18n\Translator\TranslatorInterface; class EditShortCodeAction extends AbstractRestAction { + protected const ROUTE_PATH = '/short-codes/{shortCode}'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; + /** * @var ShortUrlServiceInterface */ diff --git a/module/Rest/src/Action/EditShortcodeTagsAction.php b/module/Rest/src/Action/EditShortcodeTagsAction.php index c226605a..0e677032 100644 --- a/module/Rest/src/Action/EditShortcodeTagsAction.php +++ b/module/Rest/src/Action/EditShortcodeTagsAction.php @@ -14,6 +14,9 @@ use Zend\I18n\Translator\TranslatorInterface; class EditShortcodeTagsAction extends AbstractRestAction { + protected const ROUTE_PATH = '/short-codes/{shortCode}/tags'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; + /** * @var ShortUrlServiceInterface */ diff --git a/module/Rest/src/Action/GetVisitsAction.php b/module/Rest/src/Action/GetVisitsAction.php index 0892da85..062e1d7e 100644 --- a/module/Rest/src/Action/GetVisitsAction.php +++ b/module/Rest/src/Action/GetVisitsAction.php @@ -15,6 +15,9 @@ use Zend\I18n\Translator\TranslatorInterface; class GetVisitsAction extends AbstractRestAction { + protected const ROUTE_PATH = '/short-codes/{shortCode}/visits'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + /** * @var VisitsTrackerInterface */ diff --git a/module/Rest/src/Action/ListShortcodesAction.php b/module/Rest/src/Action/ListShortcodesAction.php index fe071a28..a725c5f5 100644 --- a/module/Rest/src/Action/ListShortcodesAction.php +++ b/module/Rest/src/Action/ListShortcodesAction.php @@ -16,6 +16,9 @@ class ListShortcodesAction extends AbstractRestAction { use PaginatorUtilsTrait; + protected const ROUTE_PATH = '/short-codes'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + /** * @var ShortUrlServiceInterface */ diff --git a/module/Rest/src/Action/ResolveUrlAction.php b/module/Rest/src/Action/ResolveUrlAction.php index 54109049..34bcd651 100644 --- a/module/Rest/src/Action/ResolveUrlAction.php +++ b/module/Rest/src/Action/ResolveUrlAction.php @@ -15,6 +15,9 @@ use Zend\I18n\Translator\TranslatorInterface; class ResolveUrlAction extends AbstractRestAction { + protected const ROUTE_PATH = '/short-codes/{shortCode}'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + /** * @var UrlShortenerInterface */ diff --git a/module/Rest/src/Action/Tag/CreateTagsAction.php b/module/Rest/src/Action/Tag/CreateTagsAction.php index 6f93b46f..9e963467 100644 --- a/module/Rest/src/Action/Tag/CreateTagsAction.php +++ b/module/Rest/src/Action/Tag/CreateTagsAction.php @@ -12,6 +12,9 @@ use Zend\Diactoros\Response\JsonResponse; class CreateTagsAction extends AbstractRestAction { + protected const ROUTE_PATH = '/tags'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_POST]; + /** * @var TagServiceInterface */ diff --git a/module/Rest/src/Action/Tag/DeleteTagsAction.php b/module/Rest/src/Action/Tag/DeleteTagsAction.php index e7ae7b48..4c1f4371 100644 --- a/module/Rest/src/Action/Tag/DeleteTagsAction.php +++ b/module/Rest/src/Action/Tag/DeleteTagsAction.php @@ -12,6 +12,9 @@ use Zend\Diactoros\Response\EmptyResponse; class DeleteTagsAction extends AbstractRestAction { + protected const ROUTE_PATH = '/tags'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; + /** * @var TagServiceInterface */ diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index 79c6a79e..02fb968c 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -12,6 +12,9 @@ use Zend\Diactoros\Response\JsonResponse; class ListTagsAction extends AbstractRestAction { + protected const ROUTE_PATH = '/tags'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; + /** * @var TagServiceInterface */ diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index d2e9b871..02f32abb 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -16,6 +16,9 @@ use Zend\I18n\Translator\TranslatorInterface; class UpdateTagAction extends AbstractRestAction { + protected const ROUTE_PATH = '/tags'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; + /** * @var TagServiceInterface */ From 2f5290b9d34c606a4691227b22f35f7ee6a1368f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 1 May 2018 18:35:12 +0200 Subject: [PATCH 04/30] Moved whitelisted routes in CheckAuthenticationMiddleware to external configuration --- module/Rest/config/auth.config.php | 14 ++++++++++++++ module/Rest/config/dependencies.config.php | 7 ++++++- .../Middleware/CheckAuthenticationMiddleware.php | 9 +++++++-- .../CheckAuthenticationMiddlewareTest.php | 8 +++++--- 4 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 module/Rest/config/auth.config.php diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php new file mode 100644 index 00000000..ab3e1319 --- /dev/null +++ b/module/Rest/config/auth.config.php @@ -0,0 +1,14 @@ + [ + 'routes_whitelist' => [ + Action\AuthenticateAction::class, + ], + ], + +]; diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index bfd1a774..9fa488c5 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -59,7 +59,12 @@ return [ Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, Translator::class, LoggerInterface::class], - Middleware\CheckAuthenticationMiddleware::class => [JWTService::class, 'translator', 'Logger_Shlink'], + Middleware\CheckAuthenticationMiddleware::class => [ + JWTService::class, + 'translator', + 'config.auth.routes_whitelist', + 'Logger_Shlink', + ], ], ]; diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index 32e10ed8..dd0adf3b 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php @@ -10,7 +10,6 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; -use Shlinkio\Shlink\Rest\Action\AuthenticateAction; use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Exception\AuthenticationException; use Shlinkio\Shlink\Rest\Util\RestUtils; @@ -35,14 +34,20 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface, StatusCodeIn * @var LoggerInterface */ private $logger; + /** + * @var array + */ + private $routesWhitelist; public function __construct( JWTServiceInterface $jwtService, TranslatorInterface $translator, + array $routesWhitelist, LoggerInterface $logger = null ) { $this->translator = $translator; $this->jwtService = $jwtService; + $this->routesWhitelist = $routesWhitelist; $this->logger = $logger ?: new NullLogger(); } @@ -64,8 +69,8 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface, StatusCodeIn $routeResult = $request->getAttribute(RouteResult::class); if ($routeResult === null || $routeResult->isFailure() - || $routeResult->getMatchedRouteName() === AuthenticateAction::class || $request->getMethod() === 'OPTIONS' + || \in_array($routeResult->getMatchedRouteName(), $this->routesWhitelist, true) ) { return $handler->handle($request); } diff --git a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php index fa7f7d06..7c74a6aa 100644 --- a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php @@ -37,9 +37,11 @@ class CheckAuthenticationMiddlewareTest extends TestCase public function setUp() { $this->jwtService = $this->prophesize(JWTService::class); - $this->middleware = new CheckAuthenticationMiddleware($this->jwtService->reveal(), Translator::factory([])); - $this->dummyMiddleware = middleware(function ($request, $handler) { - return new Response\EmptyResponse; + $this->middleware = new CheckAuthenticationMiddleware($this->jwtService->reveal(), Translator::factory([]), [ + AuthenticateAction::class, + ]); + $this->dummyMiddleware = middleware(function () { + return new Response\EmptyResponse(); }); } From e5e1aa2ff4ec5223afe39af5853f4da748c568b2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 1 May 2018 19:35:12 +0200 Subject: [PATCH 05/30] Defined abstract action which handles short codes generations --- module/Core/src/Model/CreateShortCodeData.php | 56 +++++++++ module/Core/src/Model/ShortUrlMeta.php | 2 +- module/Rest/config/routes.config.php | 7 +- .../Rest/src/Action/CreateShortcodeAction.php | 101 +++------------ .../AbstractCreateShortCodeAction.php | 119 ++++++++++++++++++ module/Rest/src/Util/RestUtils.php | 1 + 6 files changed, 198 insertions(+), 88 deletions(-) create mode 100644 module/Core/src/Model/CreateShortCodeData.php create mode 100644 module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php diff --git a/module/Core/src/Model/CreateShortCodeData.php b/module/Core/src/Model/CreateShortCodeData.php new file mode 100644 index 00000000..880c32c2 --- /dev/null +++ b/module/Core/src/Model/CreateShortCodeData.php @@ -0,0 +1,56 @@ +longUrl = $longUrl; + $this->tags = $tags; + $this->meta = $meta ?? ShortUrlMeta::createFromParams(); + } + + /** + * @return UriInterface + */ + public function getLongUrl(): UriInterface + { + return $this->longUrl; + } + + /** + * @return array + */ + public function getTags(): array + { + return $this->tags; + } + + /** + * @return ShortUrlMeta + */ + public function getMeta(): ShortUrlMeta + { + return $this->meta; + } +} diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index f79edce6..d9acf6b9 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -71,7 +71,7 @@ final class ShortUrlMeta * @param array $data * @throws ValidationException */ - private function validate(array $data) + private function validate(array $data): void { $inputFilter = new ShortUrlMetaInputFilter($data); if (! $inputFilter->isValid()) { diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index e2ee8f75..709b363b 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -10,12 +10,7 @@ return [ Action\AuthenticateAction::getRouteDef(), // Short codes - [ - 'name' => Action\CreateShortcodeAction::class, - 'path' => '/short-codes', - 'middleware' => Action\CreateShortcodeAction::class, - 'allowed_methods' => [RequestMethod::METHOD_POST], - ], + Action\CreateShortcodeAction::getRouteDef(), // [ // 'name' => Action\CreateShortcodeAction::class, // 'path' => '/short-codes', diff --git a/module/Rest/src/Action/CreateShortcodeAction.php b/module/Rest/src/Action/CreateShortcodeAction.php index d880d552..b4fff2db 100644 --- a/module/Rest/src/Action/CreateShortcodeAction.php +++ b/module/Rest/src/Action/CreateShortcodeAction.php @@ -3,104 +3,43 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action; -use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; -use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; -use Shlinkio\Shlink\Rest\Util\RestUtils; -use Zend\Diactoros\Response\JsonResponse; +use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; +use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\CreateShortCodeData; +use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Rest\Action\ShortCode\AbstractCreateShortCodeAction; use Zend\Diactoros\Uri; -use Zend\I18n\Translator\TranslatorInterface; -class CreateShortcodeAction extends AbstractRestAction +class CreateShortcodeAction extends AbstractCreateShortCodeAction { - /** - * @var UrlShortenerInterface - */ - private $urlShortener; - /** - * @var array - */ - private $domainConfig; - /** - * @var TranslatorInterface - */ - private $translator; - - public function __construct( - UrlShortenerInterface $urlShortener, - TranslatorInterface $translator, - array $domainConfig, - LoggerInterface $logger = null - ) { - parent::__construct($logger); - $this->urlShortener = $urlShortener; - $this->translator = $translator; - $this->domainConfig = $domainConfig; - } + protected const ROUTE_PATH = '/short-codes'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_POST]; /** * @param Request $request - * @return Response + * @return CreateShortCodeData + * @throws ValidationException + * @throws InvalidArgumentException * @throws \InvalidArgumentException */ - public function handle(Request $request): Response + protected function buildUrlToShortCodeData(Request $request): CreateShortCodeData { $postData = (array) $request->getParsedBody(); if (! isset($postData['longUrl'])) { - return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => $this->translator->translate('A URL was not provided'), - ], self::STATUS_BAD_REQUEST); + throw new InvalidArgumentException('A URL was not provided'); } - $longUrl = $postData['longUrl']; - $customSlug = $postData['customSlug'] ?? null; - try { - $shortCode = $this->urlShortener->urlToShortCode( - new Uri($longUrl), - (array) ($postData['tags'] ?? []), + return new CreateShortCodeData( + new Uri($postData['longUrl']), + (array) ($postData['tags'] ?? []), + ShortUrlMeta::createFromParams( $this->getOptionalDate($postData, 'validSince'), $this->getOptionalDate($postData, 'validUntil'), - $customSlug, + $postData['customSlug'] ?? null, isset($postData['maxVisits']) ? (int) $postData['maxVisits'] : null - ); - $shortUrl = (new Uri())->withPath($shortCode) - ->withScheme($this->domainConfig['schema']) - ->withHost($this->domainConfig['hostname']); - - return new JsonResponse([ - 'longUrl' => $longUrl, - 'shortUrl' => (string) $shortUrl, - 'shortCode' => $shortCode, - ]); - } catch (InvalidUrlException $e) { - $this->logger->warning('Provided Invalid URL.' . PHP_EOL . $e); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => \sprintf( - $this->translator->translate('Provided URL %s is invalid. Try with a different one.'), - $longUrl - ), - ], self::STATUS_BAD_REQUEST); - } catch (NonUniqueSlugException $e) { - $this->logger->warning('Provided non-unique slug.' . PHP_EOL . $e); - return new JsonResponse([ - 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => \sprintf( - $this->translator->translate('Provided slug %s is already in use. Try with a different one.'), - $customSlug - ), - ], self::STATUS_BAD_REQUEST); - } catch (\Throwable $e) { - $this->logger->error('Unexpected error creating shortcode.' . PHP_EOL . $e); - return new JsonResponse([ - 'error' => RestUtils::UNKNOWN_ERROR, - 'message' => $this->translator->translate('Unexpected error occurred'), - ], self::STATUS_INTERNAL_SERVER_ERROR); - } + ) + ); } private function getOptionalDate(array $postData, string $fieldName) diff --git a/module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php b/module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php new file mode 100644 index 00000000..3bbc15bb --- /dev/null +++ b/module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php @@ -0,0 +1,119 @@ +urlShortener = $urlShortener; + $this->translator = $translator; + $this->domainConfig = $domainConfig; + } + + /** + * @param Request $request + * @return Response + * @throws \InvalidArgumentException + */ + public function handle(Request $request): Response + { + try { + $shortCodeData = $this->buildUrlToShortCodeData($request); + $shortCodeMeta = $shortCodeData->getMeta(); + $longUrl = $shortCodeData->getLongUrl(); + $customSlug = $shortCodeMeta->getCustomSlug(); + + $shortCode = $this->urlShortener->urlToShortCode( + $longUrl, + $shortCodeData->getTags(), + $shortCodeMeta->getValidSince(), + $shortCodeMeta->getValidUntil(), + $customSlug, + $shortCodeMeta->getMaxVisits() + ); + $shortUrl = (new Uri())->withPath($shortCode) + ->withScheme($this->domainConfig['schema']) + ->withHost($this->domainConfig['hostname']); + + // TODO Make response to be generated based on Accept header + return new JsonResponse([ + 'longUrl' => (string) $longUrl, + 'shortUrl' => (string) $shortUrl, + 'shortCode' => $shortCode, + ]); + } catch (InvalidUrlException $e) { + $this->logger->warning('Provided Invalid URL.' . PHP_EOL . $e); + return new JsonResponse([ + 'error' => RestUtils::getRestErrorCodeFromException($e), + 'message' => \sprintf( + $this->translator->translate('Provided URL %s is invalid. Try with a different one.'), + $longUrl + ), + ], self::STATUS_BAD_REQUEST); + } catch (NonUniqueSlugException $e) { + $this->logger->warning('Provided non-unique slug.' . PHP_EOL . $e); + return new JsonResponse([ + 'error' => RestUtils::getRestErrorCodeFromException($e), + 'message' => \sprintf( + $this->translator->translate('Provided slug %s is already in use. Try with a different one.'), + $customSlug + ), + ], self::STATUS_BAD_REQUEST); + } catch (ValidationException | InvalidArgumentException $e) { + $this->logger->warning('Provided data is invalid.' . PHP_EOL . $e); + return new JsonResponse([ + 'error' => RestUtils::INVALID_ARGUMENT_ERROR, + 'message' => $this->translator->translate('Provided data is invalid'), + ], self::STATUS_BAD_REQUEST); + } catch (\Throwable $e) { + $this->logger->error('Unexpected error creating shortcode.' . PHP_EOL . $e); + return new JsonResponse([ + 'error' => RestUtils::UNKNOWN_ERROR, + 'message' => $this->translator->translate('Unexpected error occurred'), + ], self::STATUS_INTERNAL_SERVER_ERROR); + } + } + + /** + * @param Request $request + * @return CreateShortCodeData + * @throws ValidationException + * @throws InvalidArgumentException + */ + abstract protected function buildUrlToShortCodeData(Request $request): CreateShortCodeData; +} diff --git a/module/Rest/src/Util/RestUtils.php b/module/Rest/src/Util/RestUtils.php index 1d697676..666469ed 100644 --- a/module/Rest/src/Util/RestUtils.php +++ b/module/Rest/src/Util/RestUtils.php @@ -30,6 +30,7 @@ class RestUtils case $e instanceof Core\NonUniqueSlugException: return self::INVALID_SLUG_ERROR; case $e instanceof Common\InvalidArgumentException: + case $e instanceof Core\InvalidArgumentException: case $e instanceof Core\ValidationException: return self::INVALID_ARGUMENT_ERROR; case $e instanceof Rest\AuthenticationException: From a2294704e6354210ac574ee07abddd577f2682f1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 1 May 2018 19:38:44 +0200 Subject: [PATCH 06/30] Split try catch to prevent undefined variables --- module/Rest/config/routes.config.php | 1 - .../ShortCode/AbstractCreateShortCodeAction.php | 14 ++++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 709b363b..92783726 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -1,7 +1,6 @@ getMeta(); $longUrl = $shortCodeData->getLongUrl(); $customSlug = $shortCodeMeta->getCustomSlug(); + } catch (ValidationException | InvalidArgumentException $e) { + $this->logger->warning('Provided data is invalid.' . PHP_EOL . $e); + return new JsonResponse([ + 'error' => RestUtils::INVALID_ARGUMENT_ERROR, + 'message' => $this->translator->translate('Provided data is invalid'), + ], self::STATUS_BAD_REQUEST); + } + try { $shortCode = $this->urlShortener->urlToShortCode( $longUrl, $shortCodeData->getTags(), @@ -94,12 +102,6 @@ abstract class AbstractCreateShortCodeAction extends AbstractRestAction $customSlug ), ], self::STATUS_BAD_REQUEST); - } catch (ValidationException | InvalidArgumentException $e) { - $this->logger->warning('Provided data is invalid.' . PHP_EOL . $e); - return new JsonResponse([ - 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => $this->translator->translate('Provided data is invalid'), - ], self::STATUS_BAD_REQUEST); } catch (\Throwable $e) { $this->logger->error('Unexpected error creating shortcode.' . PHP_EOL . $e); return new JsonResponse([ From 28650aee2bc4cc6e40dfd76a4e8918d66924dd93 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 12:19:51 +0200 Subject: [PATCH 07/30] Fixed case sensitivity errors --- module/Rest/config/dependencies.config.php | 12 ++++++------ module/Rest/config/routes.config.php | 10 +++++----- ...ShortcodeAction.php => CreateShortCodeAction.php} | 2 +- ...odeTagsAction.php => EditShortCodeTagsAction.php} | 2 +- ...ShortcodesAction.php => ListShortCodesAction.php} | 2 +- ...eActionTest.php => CreateShortCodeActionTest.php} | 8 ++++---- ...ctionTest.php => EditShortCodeTagsActionTest.php} | 8 ++++---- module/Rest/test/Action/ListShortCodesActionTest.php | 6 +++--- 8 files changed, 25 insertions(+), 25 deletions(-) rename module/Rest/src/Action/{CreateShortcodeAction.php => CreateShortCodeAction.php} (96%) rename module/Rest/src/Action/{EditShortcodeTagsAction.php => EditShortCodeTagsAction.php} (97%) rename module/Rest/src/Action/{ListShortcodesAction.php => ListShortCodesAction.php} (97%) rename module/Rest/test/Action/{CreateShortcodeActionTest.php => CreateShortCodeActionTest.php} (94%) rename module/Rest/test/Action/{EditShortcodeTagsActionTest.php => EditShortCodeTagsActionTest.php} (91%) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 9fa488c5..e4244278 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -20,12 +20,12 @@ return [ ApiKeyService::class => ConfigAbstractFactory::class, Action\AuthenticateAction::class => ConfigAbstractFactory::class, - Action\CreateShortcodeAction::class => ConfigAbstractFactory::class, + Action\CreateShortCodeAction::class => ConfigAbstractFactory::class, Action\EditShortCodeAction::class => ConfigAbstractFactory::class, Action\ResolveUrlAction::class => ConfigAbstractFactory::class, Action\GetVisitsAction::class => ConfigAbstractFactory::class, - Action\ListShortcodesAction::class => ConfigAbstractFactory::class, - Action\EditShortcodeTagsAction::class => ConfigAbstractFactory::class, + Action\ListShortCodesAction::class => ConfigAbstractFactory::class, + Action\EditShortCodeTagsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, Action\Tag\DeleteTagsAction::class => ConfigAbstractFactory::class, Action\Tag\CreateTagsAction::class => ConfigAbstractFactory::class, @@ -43,7 +43,7 @@ return [ ApiKeyService::class => ['em'], Action\AuthenticateAction::class => [ApiKeyService::class, JWTService::class, 'translator', 'Logger_Shlink'], - Action\CreateShortcodeAction::class => [ + Action\CreateShortCodeAction::class => [ Service\UrlShortener::class, 'translator', 'config.url_shortener.domain', @@ -52,8 +52,8 @@ return [ Action\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], Action\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], Action\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], - Action\ListShortcodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], - Action\EditShortcodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], + Action\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], + Action\EditShortCodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], Action\Tag\ListTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 92783726..dac29510 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -9,17 +9,17 @@ return [ Action\AuthenticateAction::getRouteDef(), // Short codes - Action\CreateShortcodeAction::getRouteDef(), + Action\CreateShortCodeAction::getRouteDef(), // [ -// 'name' => Action\CreateShortcodeAction::class, +// 'name' => Action\CreateShortCodeAction::class, // 'path' => '/short-codes', -// 'middleware' => Action\CreateShortcodeAction::class, +// 'middleware' => Action\CreateShortCodeAction::class, // 'allowed_methods' => [RequestMethod::METHOD_GET], // ], Action\EditShortCodeAction::getRouteDef(), Action\ResolveUrlAction::getRouteDef(), - Action\ListShortcodesAction::getRouteDef(), - Action\EditShortcodeTagsAction::getRouteDef(), + Action\ListShortCodesAction::getRouteDef(), + Action\EditShortCodeTagsAction::getRouteDef(), // Visits Action\GetVisitsAction::getRouteDef(), diff --git a/module/Rest/src/Action/CreateShortcodeAction.php b/module/Rest/src/Action/CreateShortCodeAction.php similarity index 96% rename from module/Rest/src/Action/CreateShortcodeAction.php rename to module/Rest/src/Action/CreateShortCodeAction.php index b4fff2db..382e8ac5 100644 --- a/module/Rest/src/Action/CreateShortcodeAction.php +++ b/module/Rest/src/Action/CreateShortCodeAction.php @@ -11,7 +11,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Rest\Action\ShortCode\AbstractCreateShortCodeAction; use Zend\Diactoros\Uri; -class CreateShortcodeAction extends AbstractCreateShortCodeAction +class CreateShortCodeAction extends AbstractCreateShortCodeAction { protected const ROUTE_PATH = '/short-codes'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_POST]; diff --git a/module/Rest/src/Action/EditShortcodeTagsAction.php b/module/Rest/src/Action/EditShortCodeTagsAction.php similarity index 97% rename from module/Rest/src/Action/EditShortcodeTagsAction.php rename to module/Rest/src/Action/EditShortCodeTagsAction.php index 0e677032..309c1e86 100644 --- a/module/Rest/src/Action/EditShortcodeTagsAction.php +++ b/module/Rest/src/Action/EditShortCodeTagsAction.php @@ -12,7 +12,7 @@ use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; use Zend\I18n\Translator\TranslatorInterface; -class EditShortcodeTagsAction extends AbstractRestAction +class EditShortCodeTagsAction extends AbstractRestAction { protected const ROUTE_PATH = '/short-codes/{shortCode}/tags'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; diff --git a/module/Rest/src/Action/ListShortcodesAction.php b/module/Rest/src/Action/ListShortCodesAction.php similarity index 97% rename from module/Rest/src/Action/ListShortcodesAction.php rename to module/Rest/src/Action/ListShortCodesAction.php index a725c5f5..b2864e16 100644 --- a/module/Rest/src/Action/ListShortcodesAction.php +++ b/module/Rest/src/Action/ListShortCodesAction.php @@ -12,7 +12,7 @@ use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; use Zend\I18n\Translator\TranslatorInterface; -class ListShortcodesAction extends AbstractRestAction +class ListShortCodesAction extends AbstractRestAction { use PaginatorUtilsTrait; diff --git a/module/Rest/test/Action/CreateShortcodeActionTest.php b/module/Rest/test/Action/CreateShortCodeActionTest.php similarity index 94% rename from module/Rest/test/Action/CreateShortcodeActionTest.php rename to module/Rest/test/Action/CreateShortCodeActionTest.php index c9d8ff19..609b28a5 100644 --- a/module/Rest/test/Action/CreateShortcodeActionTest.php +++ b/module/Rest/test/Action/CreateShortCodeActionTest.php @@ -9,16 +9,16 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Service\UrlShortener; -use Shlinkio\Shlink\Rest\Action\CreateShortcodeAction; +use Shlinkio\Shlink\Rest\Action\CreateShortCodeAction; use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\Uri; use Zend\I18n\Translator\Translator; -class CreateShortcodeActionTest extends TestCase +class CreateShortCodeActionTest extends TestCase { /** - * @var CreateShortcodeAction + * @var CreateShortCodeAction */ protected $action; /** @@ -29,7 +29,7 @@ class CreateShortcodeActionTest extends TestCase public function setUp() { $this->urlShortener = $this->prophesize(UrlShortener::class); - $this->action = new CreateShortcodeAction($this->urlShortener->reveal(), Translator::factory([]), [ + $this->action = new CreateShortCodeAction($this->urlShortener->reveal(), Translator::factory([]), [ 'schema' => 'http', 'hostname' => 'foo.com', ]); diff --git a/module/Rest/test/Action/EditShortcodeTagsActionTest.php b/module/Rest/test/Action/EditShortCodeTagsActionTest.php similarity index 91% rename from module/Rest/test/Action/EditShortcodeTagsActionTest.php rename to module/Rest/test/Action/EditShortCodeTagsActionTest.php index a2bcfb38..367ea57c 100644 --- a/module/Rest/test/Action/EditShortcodeTagsActionTest.php +++ b/module/Rest/test/Action/EditShortCodeTagsActionTest.php @@ -8,14 +8,14 @@ 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\EditShortcodeTagsAction; +use Shlinkio\Shlink\Rest\Action\EditShortCodeTagsAction; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; -class EditShortcodeTagsActionTest extends TestCase +class EditShortCodeTagsActionTest extends TestCase { /** - * @var EditShortcodeTagsAction + * @var EditShortCodeTagsAction */ protected $action; /** @@ -26,7 +26,7 @@ class EditShortcodeTagsActionTest extends TestCase public function setUp() { $this->shortUrlService = $this->prophesize(ShortUrlService::class); - $this->action = new EditShortcodeTagsAction($this->shortUrlService->reveal(), Translator::factory([])); + $this->action = new EditShortCodeTagsAction($this->shortUrlService->reveal(), Translator::factory([])); } /** diff --git a/module/Rest/test/Action/ListShortCodesActionTest.php b/module/Rest/test/Action/ListShortCodesActionTest.php index 85e6737a..05305d9e 100644 --- a/module/Rest/test/Action/ListShortCodesActionTest.php +++ b/module/Rest/test/Action/ListShortCodesActionTest.php @@ -6,7 +6,7 @@ namespace ShlinkioTest\Shlink\Rest\Action; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Service\ShortUrlService; -use Shlinkio\Shlink\Rest\Action\ListShortcodesAction; +use Shlinkio\Shlink\Rest\Action\ListShortCodesAction; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; use Zend\Paginator\Adapter\ArrayAdapter; @@ -15,7 +15,7 @@ use Zend\Paginator\Paginator; class ListShortCodesActionTest extends TestCase { /** - * @var ListShortcodesAction + * @var ListShortCodesAction */ protected $action; /** @@ -26,7 +26,7 @@ class ListShortCodesActionTest extends TestCase public function setUp() { $this->service = $this->prophesize(ShortUrlService::class); - $this->action = new ListShortcodesAction($this->service->reveal(), Translator::factory([])); + $this->action = new ListShortCodesAction($this->service->reveal(), Translator::factory([])); } /** From e5ef8d7f8c15b8cb90bf0414d6c8715c6417975b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 13:21:43 +0200 Subject: [PATCH 08/30] Created action which allows short URLs to be created on a single API request --- module/Rest/config/auth.config.php | 1 + module/Rest/config/dependencies.config.php | 8 +++ module/Rest/config/routes.config.php | 7 +-- .../Rest/src/Action/CreateShortCodeAction.php | 4 +- .../AbstractCreateShortCodeAction.php | 8 +-- .../SingleStepCreateShortCodeAction.php | 61 +++++++++++++++++++ module/Rest/src/Service/ApiKeyService.php | 10 +-- .../src/Service/ApiKeyServiceInterface.php | 8 +-- 8 files changed, 84 insertions(+), 23 deletions(-) create mode 100644 module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php index ab3e1319..b43c333f 100644 --- a/module/Rest/config/auth.config.php +++ b/module/Rest/config/auth.config.php @@ -8,6 +8,7 @@ return [ 'auth' => [ 'routes_whitelist' => [ Action\AuthenticateAction::class, + Action\ShortCode\SingleStepCreateShortCodeAction::class, ], ], diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index e4244278..e0d9ede9 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -21,6 +21,7 @@ return [ Action\AuthenticateAction::class => ConfigAbstractFactory::class, Action\CreateShortCodeAction::class => ConfigAbstractFactory::class, + Action\ShortCode\SingleStepCreateShortCodeAction::class => ConfigAbstractFactory::class, Action\EditShortCodeAction::class => ConfigAbstractFactory::class, Action\ResolveUrlAction::class => ConfigAbstractFactory::class, Action\GetVisitsAction::class => ConfigAbstractFactory::class, @@ -49,6 +50,13 @@ return [ 'config.url_shortener.domain', 'Logger_Shlink', ], + Action\ShortCode\SingleStepCreateShortCodeAction::class => [ + Service\UrlShortener::class, + 'translator', + ApiKeyService::class, + 'config.url_shortener.domain', + 'Logger_Shlink', + ], Action\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], Action\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], Action\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index dac29510..f11078ce 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -10,12 +10,7 @@ return [ // Short codes Action\CreateShortCodeAction::getRouteDef(), -// [ -// 'name' => Action\CreateShortCodeAction::class, -// 'path' => '/short-codes', -// 'middleware' => Action\CreateShortCodeAction::class, -// 'allowed_methods' => [RequestMethod::METHOD_GET], -// ], + Action\ShortCode\SingleStepCreateShortCodeAction::getRouteDef(), Action\EditShortCodeAction::getRouteDef(), Action\ResolveUrlAction::getRouteDef(), Action\ListShortCodesAction::getRouteDef(), diff --git a/module/Rest/src/Action/CreateShortCodeAction.php b/module/Rest/src/Action/CreateShortCodeAction.php index 382e8ac5..3e8ad560 100644 --- a/module/Rest/src/Action/CreateShortCodeAction.php +++ b/module/Rest/src/Action/CreateShortCodeAction.php @@ -5,7 +5,6 @@ namespace Shlinkio\Shlink\Rest\Action; use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; -use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\CreateShortCodeData; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Rest\Action\ShortCode\AbstractCreateShortCodeAction; @@ -19,7 +18,6 @@ class CreateShortCodeAction extends AbstractCreateShortCodeAction /** * @param Request $request * @return CreateShortCodeData - * @throws ValidationException * @throws InvalidArgumentException * @throws \InvalidArgumentException */ @@ -27,7 +25,7 @@ class CreateShortCodeAction extends AbstractCreateShortCodeAction { $postData = (array) $request->getParsedBody(); if (! isset($postData['longUrl'])) { - throw new InvalidArgumentException('A URL was not provided'); + throw new InvalidArgumentException($this->translator->translate('A URL was not provided')); } return new CreateShortCodeData( diff --git a/module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php b/module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php index 33978675..6d7ba5f2 100644 --- a/module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php +++ b/module/Rest/src/Action/ShortCode/AbstractCreateShortCodeAction.php @@ -9,7 +9,6 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\CreateShortCodeData; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -31,7 +30,7 @@ abstract class AbstractCreateShortCodeAction extends AbstractRestAction /** * @var TranslatorInterface */ - private $translator; + protected $translator; public function __construct( UrlShortenerInterface $urlShortener, @@ -57,11 +56,11 @@ abstract class AbstractCreateShortCodeAction extends AbstractRestAction $shortCodeMeta = $shortCodeData->getMeta(); $longUrl = $shortCodeData->getLongUrl(); $customSlug = $shortCodeMeta->getCustomSlug(); - } catch (ValidationException | InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { $this->logger->warning('Provided data is invalid.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => $this->translator->translate('Provided data is invalid'), + 'message' => $e->getMessage(), ], self::STATUS_BAD_REQUEST); } @@ -114,7 +113,6 @@ abstract class AbstractCreateShortCodeAction extends AbstractRestAction /** * @param Request $request * @return CreateShortCodeData - * @throws ValidationException * @throws InvalidArgumentException */ abstract protected function buildUrlToShortCodeData(Request $request): CreateShortCodeData; diff --git a/module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php b/module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php new file mode 100644 index 00000000..56d81c9a --- /dev/null +++ b/module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php @@ -0,0 +1,61 @@ +apiKeyService = $apiKeyService; + } + + /** + * @param Request $request + * @return CreateShortCodeData + * @throws \InvalidArgumentException + * @throws InvalidArgumentException + */ + protected function buildUrlToShortCodeData(Request $request): CreateShortCodeData + { + $query = $request->getQueryParams(); + + // Check provided API key + $apiKey = $this->apiKeyService->getByKey($query['apiKey'] ?? ''); + if ($apiKey === null || ! $apiKey->isValid()) { + throw new InvalidArgumentException( + $this->translator->translate('No API key was provided or it is not valid') + ); + } + + if (! isset($query['longUrl'])) { + throw new InvalidArgumentException($this->translator->translate('A URL was not provided')); + } + + return new CreateShortCodeData(new Uri($query['longUrl'])); + } +} diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index 6f368e56..e886bcf1 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -28,7 +28,7 @@ class ApiKeyService implements ApiKeyServiceInterface public function create(\DateTime $expirationDate = null) { $key = new ApiKey(); - if (isset($expirationDate)) { + if ($expirationDate !== null) { $key->setExpirationDate($expirationDate); } @@ -44,7 +44,7 @@ class ApiKeyService implements ApiKeyServiceInterface * @param string $key * @return bool */ - public function check($key) + public function check(string $key) { /** @var ApiKey|null $apiKey */ $apiKey = $this->getByKey($key); @@ -58,7 +58,7 @@ class ApiKeyService implements ApiKeyServiceInterface * @return ApiKey * @throws InvalidArgumentException */ - public function disable($key) + public function disable(string $key) { /** @var ApiKey|null $apiKey */ $apiKey = $this->getByKey($key); @@ -77,7 +77,7 @@ class ApiKeyService implements ApiKeyServiceInterface * @param bool $enabledOnly Tells if only enabled keys should be returned * @return ApiKey[] */ - public function listKeys($enabledOnly = false) + public function listKeys(bool $enabledOnly = false) { $conditions = $enabledOnly ? ['enabled' => true] : []; return $this->em->getRepository(ApiKey::class)->findBy($conditions); @@ -89,7 +89,7 @@ class ApiKeyService implements ApiKeyServiceInterface * @param string $key * @return ApiKey|null */ - public function getByKey($key) + public function getByKey(string $key) { /** @var ApiKey|null $apiKey */ $apiKey = $this->em->getRepository(ApiKey::class)->findOneBy([ diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index ceaa3b96..eab38503 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -22,7 +22,7 @@ interface ApiKeyServiceInterface * @param string $key * @return bool */ - public function check($key); + public function check(string $key); /** * Disables provided api key @@ -31,7 +31,7 @@ interface ApiKeyServiceInterface * @return ApiKey * @throws InvalidArgumentException */ - public function disable($key); + public function disable(string $key); /** * Lists all existing api keys @@ -39,7 +39,7 @@ interface ApiKeyServiceInterface * @param bool $enabledOnly Tells if only enabled keys should be returned * @return ApiKey[] */ - public function listKeys($enabledOnly = false); + public function listKeys(bool $enabledOnly = false); /** * Tries to find one API key by its key string @@ -47,5 +47,5 @@ interface ApiKeyServiceInterface * @param string $key * @return ApiKey|null */ - public function getByKey($key); + public function getByKey(string $key); } From eb9a964c668402a54e5a0b354f85e05fc7271df5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 13:34:13 +0200 Subject: [PATCH 09/30] Removed unused use statement --- .../src/Action/ShortCode/SingleStepCreateShortCodeAction.php | 1 - 1 file changed, 1 deletion(-) diff --git a/module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php b/module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php index 56d81c9a..99c8af17 100644 --- a/module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php +++ b/module/Rest/src/Action/ShortCode/SingleStepCreateShortCodeAction.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Rest\Action\ShortCode; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Exception\InvalidArgumentException; -use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\CreateShortCodeData; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; From b99d6624172a00e73c43e6111c06b169a967b9d7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 17:57:43 +0200 Subject: [PATCH 10/30] Created SingleStepCreateShortCodeActionTest --- .../SingleStepCreateShortCodeActionTest.php | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 module/Rest/test/Action/ShortCode/SingleStepCreateShortCodeActionTest.php diff --git a/module/Rest/test/Action/ShortCode/SingleStepCreateShortCodeActionTest.php b/module/Rest/test/Action/ShortCode/SingleStepCreateShortCodeActionTest.php new file mode 100644 index 00000000..88d722e5 --- /dev/null +++ b/module/Rest/test/Action/ShortCode/SingleStepCreateShortCodeActionTest.php @@ -0,0 +1,123 @@ +urlShortener = $this->prophesize(UrlShortenerInterface::class); + $this->apiKeyService = $this->prophesize(ApiKeyServiceInterface::class); + + $this->action = new SingleStepCreateShortCodeAction( + $this->urlShortener->reveal(), + Translator::factory([]), + $this->apiKeyService->reveal(), + [ + 'schema' => 'http', + 'hostname' => 'foo.com', + ] + ); + } + + /** + * @test + * @dataProvider provideInvalidApiKeys + */ + public function errorResponseIsReturnedIfInvalidApiKeyIsProvided(?ApiKey $apiKey) + { + $request = ServerRequestFactory::fromGlobals()->withQueryParams(['apiKey' => 'abc123']); + $findApiKey = $this->apiKeyService->getByKey('abc123')->willReturn($apiKey); + + /** @var JsonResponse $resp */ + $resp = $this->action->handle($request); + $payload = $resp->getPayload(); + + $this->assertEquals(400, $resp->getStatusCode()); + $this->assertEquals('INVALID_ARGUMENT', $payload['error']); + $this->assertEquals('No API key was provided or it is not valid', $payload['message']); + $findApiKey->shouldHaveBeenCalled(); + } + + public function provideInvalidApiKeys(): array + { + return [ + [null], + [(new ApiKey())->disable()], + ]; + } + + /** + * @test + */ + public function errorResponseIsReturnedIfNoUrlIsProvided() + { + $request = ServerRequestFactory::fromGlobals()->withQueryParams(['apiKey' => 'abc123']); + $findApiKey = $this->apiKeyService->getByKey('abc123')->willReturn(new ApiKey()); + + /** @var JsonResponse $resp */ + $resp = $this->action->handle($request); + $payload = $resp->getPayload(); + + $this->assertEquals(400, $resp->getStatusCode()); + $this->assertEquals('INVALID_ARGUMENT', $payload['error']); + $this->assertEquals('A URL was not provided', $payload['message']); + $findApiKey->shouldHaveBeenCalled(); + } + + /** + * @test + */ + public function properDataIsPassedWhenGeneratingShortCode() + { + $request = ServerRequestFactory::fromGlobals()->withQueryParams([ + 'apiKey' => 'abc123', + 'longUrl' => 'http://foobar.com', + ]); + $findApiKey = $this->apiKeyService->getByKey('abc123')->willReturn(new ApiKey()); + $generateShortCode = $this->urlShortener->urlToShortCode( + Argument::that(function (UriInterface $argument) { + Assert::assertEquals('http://foobar.com', (string) $argument); + return $argument; + }), + [], + null, + null, + null, + null + ); + + $resp = $this->action->handle($request); + + $this->assertEquals(200, $resp->getStatusCode()); + $findApiKey->shouldHaveBeenCalled(); + $generateShortCode->shouldHaveBeenCalled(); + } +} From fdc637c23d156e6babc0b0723d7a9a60804b94d9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 17:59:28 +0200 Subject: [PATCH 11/30] Moved action to subnamespace --- module/Rest/config/dependencies.config.php | 4 ++-- module/Rest/config/routes.config.php | 2 +- .../Rest/src/Action/{ => ShortCode}/CreateShortCodeAction.php | 3 +-- module/Rest/test/Action/CreateShortCodeActionTest.php | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) rename module/Rest/src/Action/{ => ShortCode}/CreateShortCodeAction.php (93%) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index e0d9ede9..d5de2114 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -20,7 +20,7 @@ return [ ApiKeyService::class => ConfigAbstractFactory::class, Action\AuthenticateAction::class => ConfigAbstractFactory::class, - Action\CreateShortCodeAction::class => ConfigAbstractFactory::class, + Action\ShortCode\CreateShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\SingleStepCreateShortCodeAction::class => ConfigAbstractFactory::class, Action\EditShortCodeAction::class => ConfigAbstractFactory::class, Action\ResolveUrlAction::class => ConfigAbstractFactory::class, @@ -44,7 +44,7 @@ return [ ApiKeyService::class => ['em'], Action\AuthenticateAction::class => [ApiKeyService::class, JWTService::class, 'translator', 'Logger_Shlink'], - Action\CreateShortCodeAction::class => [ + Action\ShortCode\CreateShortCodeAction::class => [ Service\UrlShortener::class, 'translator', 'config.url_shortener.domain', diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index f11078ce..1b90fd01 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -9,7 +9,7 @@ return [ Action\AuthenticateAction::getRouteDef(), // Short codes - Action\CreateShortCodeAction::getRouteDef(), + Action\ShortCode\CreateShortCodeAction::getRouteDef(), Action\ShortCode\SingleStepCreateShortCodeAction::getRouteDef(), Action\EditShortCodeAction::getRouteDef(), Action\ResolveUrlAction::getRouteDef(), diff --git a/module/Rest/src/Action/CreateShortCodeAction.php b/module/Rest/src/Action/ShortCode/CreateShortCodeAction.php similarity index 93% rename from module/Rest/src/Action/CreateShortCodeAction.php rename to module/Rest/src/Action/ShortCode/CreateShortCodeAction.php index 3e8ad560..da556420 100644 --- a/module/Rest/src/Action/CreateShortCodeAction.php +++ b/module/Rest/src/Action/ShortCode/CreateShortCodeAction.php @@ -1,13 +1,12 @@ Date: Thu, 3 May 2018 18:00:32 +0200 Subject: [PATCH 12/30] Moved action to subnamespace --- module/Rest/config/dependencies.config.php | 4 ++-- module/Rest/config/routes.config.php | 2 +- .../Rest/src/Action/{ => ShortCode}/EditShortCodeAction.php | 3 ++- module/Rest/test/Action/EditShortCodeActionTest.php | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) rename module/Rest/src/Action/{ => ShortCode}/EditShortCodeAction.php (96%) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index d5de2114..7b55290c 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -22,7 +22,7 @@ return [ Action\AuthenticateAction::class => ConfigAbstractFactory::class, Action\ShortCode\CreateShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\SingleStepCreateShortCodeAction::class => ConfigAbstractFactory::class, - Action\EditShortCodeAction::class => ConfigAbstractFactory::class, + Action\ShortCode\EditShortCodeAction::class => ConfigAbstractFactory::class, Action\ResolveUrlAction::class => ConfigAbstractFactory::class, Action\GetVisitsAction::class => ConfigAbstractFactory::class, Action\ListShortCodesAction::class => ConfigAbstractFactory::class, @@ -57,7 +57,7 @@ return [ 'config.url_shortener.domain', 'Logger_Shlink', ], - Action\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], + Action\ShortCode\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], Action\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], Action\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], Action\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 1b90fd01..cb06e7ae 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -11,7 +11,7 @@ return [ // Short codes Action\ShortCode\CreateShortCodeAction::getRouteDef(), Action\ShortCode\SingleStepCreateShortCodeAction::getRouteDef(), - Action\EditShortCodeAction::getRouteDef(), + Action\ShortCode\EditShortCodeAction::getRouteDef(), Action\ResolveUrlAction::getRouteDef(), Action\ListShortCodesAction::getRouteDef(), Action\EditShortCodeTagsAction::getRouteDef(), diff --git a/module/Rest/src/Action/EditShortCodeAction.php b/module/Rest/src/Action/ShortCode/EditShortCodeAction.php similarity index 96% rename from module/Rest/src/Action/EditShortCodeAction.php rename to module/Rest/src/Action/ShortCode/EditShortCodeAction.php index a3a2357f..6a750a62 100644 --- a/module/Rest/src/Action/EditShortCodeAction.php +++ b/module/Rest/src/Action/ShortCode/EditShortCodeAction.php @@ -1,7 +1,7 @@ Date: Thu, 3 May 2018 18:01:57 +0200 Subject: [PATCH 13/30] Moved action to subnamespace --- module/Rest/config/dependencies.config.php | 4 ++-- module/Rest/config/routes.config.php | 2 +- .../Rest/src/Action/{ => ShortCode}/ListShortCodesAction.php | 3 ++- module/Rest/test/Action/ListShortCodesActionTest.php | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) rename module/Rest/src/Action/{ => ShortCode}/ListShortCodesAction.php (95%) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 7b55290c..273399da 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -25,7 +25,7 @@ return [ Action\ShortCode\EditShortCodeAction::class => ConfigAbstractFactory::class, Action\ResolveUrlAction::class => ConfigAbstractFactory::class, Action\GetVisitsAction::class => ConfigAbstractFactory::class, - Action\ListShortCodesAction::class => ConfigAbstractFactory::class, + Action\ShortCode\ListShortCodesAction::class => ConfigAbstractFactory::class, Action\EditShortCodeTagsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, Action\Tag\DeleteTagsAction::class => ConfigAbstractFactory::class, @@ -60,7 +60,7 @@ return [ Action\ShortCode\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], Action\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], Action\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], - Action\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], + Action\ShortCode\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], Action\EditShortCodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], Action\Tag\ListTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index cb06e7ae..d652bfb4 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -13,7 +13,7 @@ return [ Action\ShortCode\SingleStepCreateShortCodeAction::getRouteDef(), Action\ShortCode\EditShortCodeAction::getRouteDef(), Action\ResolveUrlAction::getRouteDef(), - Action\ListShortCodesAction::getRouteDef(), + Action\ShortCode\ListShortCodesAction::getRouteDef(), Action\EditShortCodeTagsAction::getRouteDef(), // Visits diff --git a/module/Rest/src/Action/ListShortCodesAction.php b/module/Rest/src/Action/ShortCode/ListShortCodesAction.php similarity index 95% rename from module/Rest/src/Action/ListShortCodesAction.php rename to module/Rest/src/Action/ShortCode/ListShortCodesAction.php index b2864e16..58f26d77 100644 --- a/module/Rest/src/Action/ListShortCodesAction.php +++ b/module/Rest/src/Action/ShortCode/ListShortCodesAction.php @@ -1,13 +1,14 @@ Date: Thu, 3 May 2018 18:02:45 +0200 Subject: [PATCH 14/30] Moved action to subnamespace --- module/Rest/config/dependencies.config.php | 4 ++-- module/Rest/config/routes.config.php | 2 +- module/Rest/src/Action/{ => ShortCode}/ResolveUrlAction.php | 3 ++- module/Rest/test/Action/ResolveUrlActionTest.php | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) rename module/Rest/src/Action/{ => ShortCode}/ResolveUrlAction.php (96%) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 273399da..cb946f85 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -23,7 +23,7 @@ return [ Action\ShortCode\CreateShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\SingleStepCreateShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\EditShortCodeAction::class => ConfigAbstractFactory::class, - Action\ResolveUrlAction::class => ConfigAbstractFactory::class, + Action\ShortCode\ResolveUrlAction::class => ConfigAbstractFactory::class, Action\GetVisitsAction::class => ConfigAbstractFactory::class, Action\ShortCode\ListShortCodesAction::class => ConfigAbstractFactory::class, Action\EditShortCodeTagsAction::class => ConfigAbstractFactory::class, @@ -58,7 +58,7 @@ return [ 'Logger_Shlink', ], Action\ShortCode\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], - Action\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], + Action\ShortCode\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], Action\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], Action\ShortCode\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], Action\EditShortCodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index d652bfb4..27075685 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -12,7 +12,7 @@ return [ Action\ShortCode\CreateShortCodeAction::getRouteDef(), Action\ShortCode\SingleStepCreateShortCodeAction::getRouteDef(), Action\ShortCode\EditShortCodeAction::getRouteDef(), - Action\ResolveUrlAction::getRouteDef(), + Action\ShortCode\ResolveUrlAction::getRouteDef(), Action\ShortCode\ListShortCodesAction::getRouteDef(), Action\EditShortCodeTagsAction::getRouteDef(), diff --git a/module/Rest/src/Action/ResolveUrlAction.php b/module/Rest/src/Action/ShortCode/ResolveUrlAction.php similarity index 96% rename from module/Rest/src/Action/ResolveUrlAction.php rename to module/Rest/src/Action/ShortCode/ResolveUrlAction.php index 34bcd651..8a7f2ba7 100644 --- a/module/Rest/src/Action/ResolveUrlAction.php +++ b/module/Rest/src/Action/ShortCode/ResolveUrlAction.php @@ -1,7 +1,7 @@ Date: Thu, 3 May 2018 18:03:10 +0200 Subject: [PATCH 15/30] Moved action to subnamespace --- module/Rest/config/dependencies.config.php | 4 ++-- module/Rest/config/routes.config.php | 2 +- .../src/Action/{ => ShortCode}/EditShortCodeTagsAction.php | 3 ++- module/Rest/test/Action/EditShortCodeTagsActionTest.php | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) rename module/Rest/src/Action/{ => ShortCode}/EditShortCodeTagsAction.php (95%) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index cb946f85..0193e2b0 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -26,7 +26,7 @@ return [ Action\ShortCode\ResolveUrlAction::class => ConfigAbstractFactory::class, Action\GetVisitsAction::class => ConfigAbstractFactory::class, Action\ShortCode\ListShortCodesAction::class => ConfigAbstractFactory::class, - Action\EditShortCodeTagsAction::class => ConfigAbstractFactory::class, + Action\ShortCode\EditShortCodeTagsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, Action\Tag\DeleteTagsAction::class => ConfigAbstractFactory::class, Action\Tag\CreateTagsAction::class => ConfigAbstractFactory::class, @@ -61,7 +61,7 @@ return [ Action\ShortCode\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], Action\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], Action\ShortCode\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], - Action\EditShortCodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], + Action\ShortCode\EditShortCodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], Action\Tag\ListTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 27075685..e2e6294d 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -14,7 +14,7 @@ return [ Action\ShortCode\EditShortCodeAction::getRouteDef(), Action\ShortCode\ResolveUrlAction::getRouteDef(), Action\ShortCode\ListShortCodesAction::getRouteDef(), - Action\EditShortCodeTagsAction::getRouteDef(), + Action\ShortCode\EditShortCodeTagsAction::getRouteDef(), // Visits Action\GetVisitsAction::getRouteDef(), diff --git a/module/Rest/src/Action/EditShortCodeTagsAction.php b/module/Rest/src/Action/ShortCode/EditShortCodeTagsAction.php similarity index 95% rename from module/Rest/src/Action/EditShortCodeTagsAction.php rename to module/Rest/src/Action/ShortCode/EditShortCodeTagsAction.php index 309c1e86..b863720b 100644 --- a/module/Rest/src/Action/EditShortCodeTagsAction.php +++ b/module/Rest/src/Action/ShortCode/EditShortCodeTagsAction.php @@ -1,13 +1,14 @@ Date: Thu, 3 May 2018 18:04:00 +0200 Subject: [PATCH 16/30] Moved action to subnamespace --- module/Rest/config/dependencies.config.php | 4 ++-- module/Rest/config/routes.config.php | 2 +- module/Rest/src/Action/{ => Visit}/GetVisitsAction.php | 3 ++- module/Rest/test/Action/GetVisitsActionTest.php | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) rename module/Rest/src/Action/{ => Visit}/GetVisitsAction.php (96%) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 0193e2b0..d260fde0 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -24,7 +24,7 @@ return [ Action\ShortCode\SingleStepCreateShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\EditShortCodeAction::class => ConfigAbstractFactory::class, Action\ShortCode\ResolveUrlAction::class => ConfigAbstractFactory::class, - Action\GetVisitsAction::class => ConfigAbstractFactory::class, + Action\Visit\GetVisitsAction::class => ConfigAbstractFactory::class, Action\ShortCode\ListShortCodesAction::class => ConfigAbstractFactory::class, Action\ShortCode\EditShortCodeTagsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, @@ -59,7 +59,7 @@ return [ ], Action\ShortCode\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], Action\ShortCode\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], - Action\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], + Action\Visit\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], Action\ShortCode\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], Action\ShortCode\EditShortCodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], Action\Tag\ListTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index e2e6294d..24d70cdd 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -17,7 +17,7 @@ return [ Action\ShortCode\EditShortCodeTagsAction::getRouteDef(), // Visits - Action\GetVisitsAction::getRouteDef(), + Action\Visit\GetVisitsAction::getRouteDef(), // Tags Action\Tag\ListTagsAction::getRouteDef(), diff --git a/module/Rest/src/Action/GetVisitsAction.php b/module/Rest/src/Action/Visit/GetVisitsAction.php similarity index 96% rename from module/Rest/src/Action/GetVisitsAction.php rename to module/Rest/src/Action/Visit/GetVisitsAction.php index 062e1d7e..96010ce0 100644 --- a/module/Rest/src/Action/GetVisitsAction.php +++ b/module/Rest/src/Action/Visit/GetVisitsAction.php @@ -1,7 +1,7 @@ Date: Thu, 3 May 2018 18:05:16 +0200 Subject: [PATCH 17/30] Fixed coding styles in config file --- module/Rest/config/dependencies.config.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index d260fde0..72198c3f 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -61,7 +61,11 @@ return [ Action\ShortCode\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], Action\Visit\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], Action\ShortCode\ListShortCodesAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], - Action\ShortCode\EditShortCodeTagsAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink'], + Action\ShortCode\EditShortCodeTagsAction::class => [ + Service\ShortUrlService::class, + 'translator', + 'Logger_Shlink', + ], Action\Tag\ListTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], From 334710e92c5419a9fc0418250c5fb25f4e6f6241 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 18:25:57 +0200 Subject: [PATCH 18/30] Added middleware which injects the content-length header in the response if not present --- config/autoload/middleware-pipeline.global.php | 1 + 1 file changed, 1 insertion(+) diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index afdd78ef..90587cd3 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -16,6 +16,7 @@ return [ 'pre-routing' => [ 'middleware' => [ ErrorHandler::class, + Expressive\Helper\ContentLengthMiddleware::class, LocaleMiddleware::class, ], 'priority' => 11, From 59f10619ba4a64e1a43720d365839f6d714d436d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 18:26:31 +0200 Subject: [PATCH 19/30] Created middleware used with short codes creation actions to handle content negotiation --- module/Rest/config/dependencies.config.php | 1 + module/Rest/config/routes.config.php | 10 +++- .../AbstractCreateShortCodeAction.php | 1 - ...eShortCodeContentNegotiationMiddleware.php | 47 +++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 72198c3f..304e667d 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -36,6 +36,7 @@ return [ Middleware\CrossDomainMiddleware::class => InvokableFactory::class, Middleware\PathVersionMiddleware::class => InvokableFactory::class, Middleware\CheckAuthenticationMiddleware::class => ConfigAbstractFactory::class, + Middleware\ShortCode\CreateShortCodeContentNegotiationMiddleware::class => InvokableFactory::class, ], ], diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 24d70cdd..0f453e26 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -1,6 +1,8 @@ withScheme($this->domainConfig['schema']) ->withHost($this->domainConfig['hostname']); - // TODO Make response to be generated based on Accept header return new JsonResponse([ 'longUrl' => (string) $longUrl, 'shortUrl' => (string) $shortUrl, diff --git a/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php new file mode 100644 index 00000000..41ce8813 --- /dev/null +++ b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php @@ -0,0 +1,47 @@ +handle($request); + $acceptedType = $this->determineAcceptedType($request); + if ($acceptedType === self::JSON) { + return $response; + } + + // If requested, return a plain text response containing the short URL only + $resp = (new Response())->withHeader('Content-Type', 'text/plain'); + $body = $resp->getBody(); + $body->write($response->getPayload()['shortUrl'] ?? ''); + $body->rewind(); + return $resp; + } + + private function determineAcceptedType(ServerRequestInterface $request): string + { + $accepts = \explode(',', $request->getHeaderLine('Accept')); + $accept = \strtolower(\array_shift($accepts)); + return \strpos($accept, 'text/plain') !== false ? self::PLAIN_TEXT : self::JSON; + } +} From 1f78b5c5247ab454339ae068d91e2f27aed08e40 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 18:32:32 +0200 Subject: [PATCH 20/30] Improved CreateShortCodeContentNegotiationMiddleware so that it can determine the format based on a query partameter --- ...eShortCodeContentNegotiationMiddleware.php | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php index 41ce8813..6de16ae7 100644 --- a/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php @@ -25,7 +25,11 @@ class CreateShortCodeContentNegotiationMiddleware implements MiddlewareInterface { /** @var JsonResponse $response */ $response = $handler->handle($request); - $acceptedType = $this->determineAcceptedType($request); + $acceptedType = $request->hasHeader('Accept') + ? $this->determineAcceptTypeFromHeader($request->getHeaderLine('Accept')) + : $this->determineAcceptTypeFromQuery($request->getQueryParams()); + + // If JSON was requested, return the response from next handler as is if ($acceptedType === self::JSON) { return $response; } @@ -38,9 +42,19 @@ class CreateShortCodeContentNegotiationMiddleware implements MiddlewareInterface return $resp; } - private function determineAcceptedType(ServerRequestInterface $request): string + private function determineAcceptTypeFromQuery(array $query): string { - $accepts = \explode(',', $request->getHeaderLine('Accept')); + if (! isset($query['format'])) { + return self::JSON; + } + + $format = \strtolower((string) $query['format']); + return $format === 'txt' ? self::PLAIN_TEXT : self::JSON; + } + + private function determineAcceptTypeFromHeader(string $acceptValue): string + { + $accepts = \explode(',', $acceptValue); $accept = \strtolower(\array_shift($accepts)); return \strpos($accept, 'text/plain') !== false ? self::PLAIN_TEXT : self::JSON; } From 0932d049070d444139da013d09c137333c566bd1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 3 May 2018 18:34:45 +0200 Subject: [PATCH 21/30] Fixed tests namespaces to match their subject under test --- .../test/Action/{ => ShortCode}/CreateShortCodeActionTest.php | 2 +- .../test/Action/{ => ShortCode}/EditShortCodeActionTest.php | 2 +- .../test/Action/{ => ShortCode}/EditShortCodeTagsActionTest.php | 2 +- .../test/Action/{ => ShortCode}/ListShortCodesActionTest.php | 2 +- .../Rest/test/Action/{ => ShortCode}/ResolveUrlActionTest.php | 2 +- module/Rest/test/Action/{ => Visit}/GetVisitsActionTest.php | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) rename module/Rest/test/Action/{ => ShortCode}/CreateShortCodeActionTest.php (98%) rename module/Rest/test/Action/{ => ShortCode}/EditShortCodeActionTest.php (98%) rename module/Rest/test/Action/{ => ShortCode}/EditShortCodeTagsActionTest.php (97%) rename module/Rest/test/Action/{ => ShortCode}/ListShortCodesActionTest.php (97%) rename module/Rest/test/Action/{ => ShortCode}/ResolveUrlActionTest.php (98%) rename module/Rest/test/Action/{ => Visit}/GetVisitsActionTest.php (98%) diff --git a/module/Rest/test/Action/CreateShortCodeActionTest.php b/module/Rest/test/Action/ShortCode/CreateShortCodeActionTest.php similarity index 98% rename from module/Rest/test/Action/CreateShortCodeActionTest.php rename to module/Rest/test/Action/ShortCode/CreateShortCodeActionTest.php index a75a248a..3ac5ad33 100644 --- a/module/Rest/test/Action/CreateShortCodeActionTest.php +++ b/module/Rest/test/Action/ShortCode/CreateShortCodeActionTest.php @@ -1,7 +1,7 @@ Date: Thu, 3 May 2018 19:04:40 +0200 Subject: [PATCH 22/30] Created CreateShortCodeContentNegotiationMiddleware --- ...rtCodeContentNegotiationMiddlewareTest.php | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php diff --git a/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php new file mode 100644 index 00000000..b61f06bf --- /dev/null +++ b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php @@ -0,0 +1,68 @@ +middleware = new CreateShortCodeContentNegotiationMiddleware(); + $this->requestHandler = new class implements RequestHandlerInterface { + /** + * Handle the request and return a response. + */ + public function handle(ServerRequestInterface $request): ResponseInterface + { + return new JsonResponse(['shortUrl' => 'http://doma.in/foo']); + } + }; + } + + /** + * @test + * @dataProvider provideData + * @param array $query + */ + public function properResponseIsReturned(?string $accept, array $query, string $expectedContentType) + { + $request = ServerRequestFactory::fromGlobals()->withQueryParams($query); + if ($accept !== null) { + $request = $request->withHeader('Accept', $accept); + } + + $response = $this->middleware->process($request, $this->requestHandler); + + $this->assertEquals($expectedContentType, $response->getHeaderLine('Content-type')); + } + + public function provideData(): array + { + return [ + [null, [], 'application/json'], + [null, ['format' => 'json'], 'application/json'], + [null, ['format' => 'invalid'], 'application/json'], + [null, ['format' => 'txt'], 'text/plain'], + ['application/json', [], 'application/json'], + ['application/xml', [], 'application/json'], + ['text/plain', [], 'text/plain'], + ]; + } +} From 98ad2816e8cf383108a755447f48d80f2dd43795 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 May 2018 12:19:08 +0200 Subject: [PATCH 23/30] Documented new endpoint to create short URLs in a single step --- .../swagger/paths/v1_short-codes_shorten.json | 125 ++++++++++++++++++ docs/swagger/swagger.json | 3 + 2 files changed, 128 insertions(+) create mode 100644 docs/swagger/paths/v1_short-codes_shorten.json diff --git a/docs/swagger/paths/v1_short-codes_shorten.json b/docs/swagger/paths/v1_short-codes_shorten.json new file mode 100644 index 00000000..1ee3387c --- /dev/null +++ b/docs/swagger/paths/v1_short-codes_shorten.json @@ -0,0 +1,125 @@ +{ + "get": { + "tags": [ + "ShortCodes" + ], + "summary": "Create a short URL", + "description": "Creates a short URL in a single API call. Useful for third party integrations", + "parameters": [ + { + "name": "apiKey", + "in": "query", + "description": "The API key used to authenticate the request", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "longUrl", + "in": "query", + "description": "The URL to be shortened", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "format", + "in": "query", + "description": "The format in which you want the response to be returned. You can also use the \"Accept\" header instead of this", + "required": false, + "schema": { + "type": "string", + "enum": [ + "txt", + "json" + ] + } + } + ], + "responses": { + "200": { + "description": "The list of short URLs", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "longUrl": { + "type": "string", + "description": "The original long URL that has been shortened" + }, + "shortUrl": { + "type": "string", + "description": "The generated short URL" + }, + "shortCode": { + "type": "string", + "description": "the short code that is being used in the short URL" + } + } + } + }, + "text/plain": { + "schema": { + "type": "string" + } + } + }, + "examples": { + "application/json": { + "longUrl": "https://github.com/shlinkio/shlink", + "shortUrl": "https://dom.ain/abc123", + "shortCode": "abc123" + }, + "text/plain": "https://dom.ain/abc123" + } + }, + "400": { + "description": "The long URL was not provided or is invalid.", + "content": { + "application/json": { + "schema": { + "$ref": "../definitions/Error.json" + } + }, + "text/plain": { + "schema": { + "type": "string" + } + } + }, + "examples": { + "application/json": { + "error": "INVALID_URL", + "message": "Provided URL foo is invalid. Try with a different one." + }, + "text/plain": "INVALID_URL" + } + }, + "500": { + "description": "Unexpected error.", + "content": { + "application/json": { + "schema": { + "$ref": "../definitions/Error.json" + } + }, + "text/plain": { + "schema": { + "type": "string" + } + } + }, + "examples": { + "application/json": { + "error": "UNKNOWN_ERROR", + "message": "Unexpected error occurred" + }, + "text/plain": "UNKNOWN_ERROR" + } + } + } + } +} diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index b45b4e3a..c5dbd791 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -40,6 +40,9 @@ "/v1/short-codes": { "$ref": "paths/v1_short-codes.json" }, + "/v1/short-codes/shorten": { + "$ref": "paths/v1_short-codes_shorten.json" + }, "/v1/short-codes/{shortCode}": { "$ref": "paths/v1_short-codes_{shortCode}.json" }, From 52d8ffa212c27ced0f482938b423424d03a7e580 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 May 2018 12:28:22 +0200 Subject: [PATCH 24/30] Improved CreateShortCodeContentNegotiationMiddleware sho that it takes into account the case in which an error is returned from next middleware --- ...eShortCodeContentNegotiationMiddleware.php | 15 ++++- ...rtCodeContentNegotiationMiddlewareTest.php | 60 +++++++++++++++---- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php index 6de16ae7..6a76795e 100644 --- a/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php @@ -23,8 +23,13 @@ class CreateShortCodeContentNegotiationMiddleware implements MiddlewareInterface */ public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - /** @var JsonResponse $response */ $response = $handler->handle($request); + + // If the response is not JSON, return it as is + if (! $response instanceof JsonResponse) { + return $response; + } + $acceptedType = $request->hasHeader('Accept') ? $this->determineAcceptTypeFromHeader($request->getHeaderLine('Accept')) : $this->determineAcceptTypeFromQuery($request->getQueryParams()); @@ -37,7 +42,7 @@ class CreateShortCodeContentNegotiationMiddleware implements MiddlewareInterface // If requested, return a plain text response containing the short URL only $resp = (new Response())->withHeader('Content-Type', 'text/plain'); $body = $resp->getBody(); - $body->write($response->getPayload()['shortUrl'] ?? ''); + $body->write($this->determineBody($response)); $body->rewind(); return $resp; } @@ -58,4 +63,10 @@ class CreateShortCodeContentNegotiationMiddleware implements MiddlewareInterface $accept = \strtolower(\array_shift($accepts)); return \strpos($accept, 'text/plain') !== false ? self::PLAIN_TEXT : self::JSON; } + + private function determineBody(JsonResponse $resp): string + { + $payload = $resp->getPayload(); + return $payload['shortUrl'] ?? $payload['error'] ?? ''; + } } diff --git a/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php index b61f06bf..a4feb7d5 100644 --- a/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php +++ b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php @@ -4,10 +4,12 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Middleware\ShortCode; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Middleware\ShortCode\CreateShortCodeContentNegotiationMiddleware; +use Zend\Diactoros\Response; use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\ServerRequestFactory; @@ -25,15 +27,20 @@ class CreateShortCodeContentNegotiationMiddlewareTest extends TestCase public function setUp() { $this->middleware = new CreateShortCodeContentNegotiationMiddleware(); - $this->requestHandler = new class implements RequestHandlerInterface { - /** - * Handle the request and return a response. - */ - public function handle(ServerRequestInterface $request): ResponseInterface - { - return new JsonResponse(['shortUrl' => 'http://doma.in/foo']); - } - }; + $this->requestHandler = $this->prophesize(RequestHandlerInterface::class); + } + + /** + * @test + */ + public function whenNoJsonResponseIsReturnedNoFurtherOperationsArePerformed() + { + $expectedResp = new Response(); + $this->requestHandler->handle(Argument::type(ServerRequestInterface::class))->willReturn($expectedResp); + + $resp = $this->middleware->process(ServerRequestFactory::fromGlobals(), $this->requestHandler->reveal()); + + $this->assertSame($expectedResp, $resp); } /** @@ -48,9 +55,14 @@ class CreateShortCodeContentNegotiationMiddlewareTest extends TestCase $request = $request->withHeader('Accept', $accept); } - $response = $this->middleware->process($request, $this->requestHandler); + $handle = $this->requestHandler->handle(Argument::type(ServerRequestInterface::class))->willReturn( + new JsonResponse(['shortUrl' => 'http://doma.in/foo']) + ); + + $response = $this->middleware->process($request, $this->requestHandler->reveal()); $this->assertEquals($expectedContentType, $response->getHeaderLine('Content-type')); + $handle->shouldHaveBeenCalled(); } public function provideData(): array @@ -65,4 +77,32 @@ class CreateShortCodeContentNegotiationMiddlewareTest extends TestCase ['text/plain', [], 'text/plain'], ]; } + + /** + * @test + * @dataProvider provideTextBodies + * @param array $json + */ + public function properBodyIsReturnedInPlainTextResponses(array $json, string $expectedBody) + { + $request = ServerRequestFactory::fromGlobals()->withQueryParams(['format' => 'txt']); + + $handle = $this->requestHandler->handle(Argument::type(ServerRequestInterface::class))->willReturn( + new JsonResponse($json) + ); + + $response = $this->middleware->process($request, $this->requestHandler->reveal()); + + $this->assertEquals($expectedBody, (string) $response->getBody()); + $handle->shouldHaveBeenCalled(); + } + + public function provideTextBodies(): array + { + return [ + [['shortUrl' => 'foobar'], 'foobar'], + [['error' => 'FOO_BAR'], 'FOO_BAR'], + [[], ''], + ]; + } } From d8acc3c24748fbd2ff47ba76e2f1cc6450e4963b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 May 2018 12:34:21 +0200 Subject: [PATCH 25/30] Removed unused use statement --- .../CreateShortCodeContentNegotiationMiddlewareTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php index a4feb7d5..8a867788 100644 --- a/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php +++ b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php @@ -5,7 +5,6 @@ namespace ShlinkioTest\Shlink\Rest\Middleware\ShortCode; use PHPUnit\Framework\TestCase; use Prophecy\Argument; -use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Rest\Middleware\ShortCode\CreateShortCodeContentNegotiationMiddleware; From 63294f20eef53bd2dc56508790fb3af84bfd9e7f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 May 2018 12:36:07 +0200 Subject: [PATCH 26/30] Updated language files --- module/Rest/lang/es.mo | Bin 2835 -> 2978 bytes module/Rest/lang/es.po | 23 +++++++++++++---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/module/Rest/lang/es.mo b/module/Rest/lang/es.mo index 89c5cdbd2bdd5655ce75420c6baa422176245088..95fbf8b6f194f05a258e938c22a1d8e1a3ee9dc2 100644 GIT binary patch delta 556 zcmX}pJuE{}6u|M*FQv5T;49P}h=!m(il9SFZi1j$zNl58U<9Tegk8}J6( z@e%v+6Zf#mDbkJSsPGB<@C&1;Tq4uBh|PG2*7_ZGitNZcgBTBd?(z!>oMN8DWjsf# zk;5bM!KR}aVjl8}^kNLhFpc+kh7*{q6dA`O9K~0RV|`V*?*fL|U#=Kf3omFjZ1IWA zU;?ch*~fXjz%2gYFs@efQ3!VUsq7&v+fhfYCj;f_f8}EA;2a_mveurKOU2*V`W+lx zIE?UJ!X?>JQ`~iYcg5#sRa)Otn_6D2t>;W5rKePGT^R*s delta 436 zcmXZYKQ9D97{~G7&i!%zoJDbWk_+2NBrLnKf=d*H1f5gryZ~N+$R#Ql8ri5MA|g@I zY#|DXLMJMP?kZ8JG^#83&Ymav?PqsqcAnX}D#xWpYbbjrYLHaPH7S!xhYNLtC2Zm- zUg9)9;R$}DhX+QQ!V8?mS6snNMjFO7wBtKChNrkFHFWho3b0 zz!9Eb@{*5VIFFSs=@B=vhHgQc!wp=*L$vEX<2ZWVo%ieL(_hEjtTOO`_5^;hj@2G% z7xys27P1dPTK2?>gudz{1Einuxq1ox#36`%rp5lnKg#VqDrdEm(u!}LcJgMOm aLN8dTMZrW6#{0Q\n" "Language-Team: \n" "Language: es_ES\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -"X-Generator: Poedit 2.0.4\n" +"X-Generator: Poedit 2.0.6\n" "X-Poedit-Basepath: ..\n" "Plural-Forms: nplurals=2; plural=(n != 1);\n" "X-Poedit-SourceCharset: UTF-8\n" @@ -25,9 +25,6 @@ msgstr "" msgid "Provided API key does not exist or is invalid." msgstr "La clave de API proporcionada no existe o es inválida." -msgid "A URL was not provided" -msgstr "No se ha proporcionado una URL" - #, php-format msgid "Provided URL %s is invalid. Try with a different one." msgstr "La URL proporcionada \"%s\" es inválida. Prueba con una diferente." @@ -39,6 +36,9 @@ msgstr "El slug proporcionado \"%s\" ya está en uso. Prueba con uno diferente." msgid "Unexpected error occurred" msgstr "Ocurrió un error inesperado" +msgid "A URL was not provided" +msgstr "No se ha proporcionado una URL" + #, php-format msgid "No URL found for short code \"%s\"" msgstr "No se ha encontrado ninguna URL para el código corto \"%s\"" @@ -49,14 +49,13 @@ msgstr "Los datos proporcionados son inválidos." msgid "A list of tags was not provided" msgstr "No se ha proporcionado una lista de etiquetas" -#, php-format -msgid "Provided short code %s does not exist" -msgstr "El código corto \"%s\" proporcionado no existe" - #, php-format msgid "Provided short code \"%s\" has an invalid format" msgstr "El código corto proporcionado \"%s\" tiene un formato no inválido" +msgid "No API key was provided or it is not valid" +msgstr "No se ha proporcionado una clave de API o esta es inválida" + msgid "" "You have to provide both 'oldName' and 'newName' params in order to properly " "rename the tag" @@ -68,6 +67,10 @@ msgstr "" msgid "It wasn't possible to find a tag with name '%s'" msgstr "No fue posible encontrar una etiqueta con el nombre '%s'" +#, php-format +msgid "Provided short code %s does not exist" +msgstr "El código corto \"%s\" proporcionado no existe" + #, php-format msgid "You need to provide the Bearer type in the %s header." msgstr "Debes proporcionar el typo Bearer en la cabecera %s." From 1437ff48ce6799112706f2bfd87551c3bd85bb9f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 7 May 2018 10:58:49 +0200 Subject: [PATCH 27/30] Ensured all core actions log errors --- config/autoload/middleware-pipeline.global.php | 4 ++-- module/Core/config/dependencies.config.php | 14 ++++++-------- module/Core/src/Action/AbstractTrackingAction.php | 11 ++++++++++- module/Core/src/Action/PreviewAction.php | 15 +++++++++++++-- module/Core/src/Action/QrCodeAction.php | 10 +++++----- .../src/Action/Util/ErrorResponseBuilderTrait.php | 4 ++-- .../{NotFoundDelegate.php => NotFoundHandler.php} | 6 +++--- ...ndDelegateTest.php => NotFoundHandlerTest.php} | 8 ++++---- 8 files changed, 45 insertions(+), 27 deletions(-) rename module/Core/src/Response/{NotFoundDelegate.php => NotFoundHandler.php} (90%) rename module/Core/test/Response/{NotFoundDelegateTest.php => NotFoundHandlerTest.php} (88%) diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 90587cd3..dc17bd53 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -2,7 +2,7 @@ declare(strict_types=1); use Shlinkio\Shlink\Common\Middleware\LocaleMiddleware; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Shlinkio\Shlink\Rest\Middleware\BodyParserMiddleware; use Shlinkio\Shlink\Rest\Middleware\CheckAuthenticationMiddleware; use Shlinkio\Shlink\Rest\Middleware\CrossDomainMiddleware; @@ -50,7 +50,7 @@ return [ 'post-routing' => [ 'middleware' => [ Expressive\Router\Middleware\DispatchMiddleware::class, - NotFoundDelegate::class, + NotFoundHandler::class, ], 'priority' => 1, ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 9061a3f5..58def9ec 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -6,7 +6,7 @@ use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Action; use Shlinkio\Shlink\Core\Middleware; use Shlinkio\Shlink\Core\Options; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Shlinkio\Shlink\Core\Service; use Zend\Expressive\Router\RouterInterface; use Zend\Expressive\Template\TemplateRendererInterface; @@ -17,7 +17,7 @@ return [ 'dependencies' => [ 'factories' => [ Options\AppOptions::class => Options\AppOptionsFactory::class, - NotFoundDelegate::class => ConfigAbstractFactory::class, + NotFoundHandler::class => ConfigAbstractFactory::class, // Services Service\UrlShortener::class => ConfigAbstractFactory::class, @@ -33,14 +33,10 @@ return [ Action\PreviewAction::class => ConfigAbstractFactory::class, Middleware\QrCodeCacheMiddleware::class => ConfigAbstractFactory::class, ], - - 'aliases' => [ - 'Zend\Expressive\Delegate\DefaultDelegate' => NotFoundDelegate::class, - ], ], ConfigAbstractFactory::class => [ - NotFoundDelegate::class => [TemplateRendererInterface::class], + NotFoundHandler::class => [TemplateRendererInterface::class], // Services Service\UrlShortener::class => [ @@ -60,14 +56,16 @@ return [ Service\UrlShortener::class, Service\VisitsTracker::class, Options\AppOptions::class, + 'Logger_Shlink', ], Action\PixelAction::class => [ Service\UrlShortener::class, Service\VisitsTracker::class, Options\AppOptions::class, + 'Logger_Shlink', ], Action\QrCodeAction::class => [RouterInterface::class, Service\UrlShortener::class, 'Logger_Shlink'], - Action\PreviewAction::class => [PreviewGenerator::class, Service\UrlShortener::class], + Action\PreviewAction::class => [PreviewGenerator::class, Service\UrlShortener::class, 'Logger_Shlink'], Middleware\QrCodeCacheMiddleware::class => [Cache::class], ], diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 0ee475db..147e31b6 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -7,6 +7,8 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; 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; @@ -30,15 +32,21 @@ abstract class AbstractTrackingAction implements MiddlewareInterface * @var AppOptions */ private $appOptions; + /** + * @var LoggerInterface + */ + private $logger; public function __construct( UrlShortenerInterface $urlShortener, VisitsTrackerInterface $visitTracker, - AppOptions $appOptions + AppOptions $appOptions, + LoggerInterface $logger = null ) { $this->urlShortener = $urlShortener; $this->visitTracker = $visitTracker; $this->appOptions = $appOptions; + $this->logger = $logger ?: new NullLogger(); } /** @@ -66,6 +74,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface return $this->createResp($longUrl); } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { + $this->logger->warning('An error occurred while tracking short code.' . PHP_EOL . $e); return $this->buildErrorResponse($request, $handler); } } diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index 3dd3ba43..83748a3b 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -7,6 +7,8 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Exception\PreviewGenerationException; use Shlinkio\Shlink\Common\Service\PreviewGeneratorInterface; use Shlinkio\Shlink\Common\Util\ResponseUtilsTrait; @@ -28,11 +30,19 @@ class PreviewAction implements MiddlewareInterface * @var UrlShortenerInterface */ private $urlShortener; + /** + * @var LoggerInterface + */ + private $logger; - public function __construct(PreviewGeneratorInterface $previewGenerator, UrlShortenerInterface $urlShortener) - { + public function __construct( + PreviewGeneratorInterface $previewGenerator, + UrlShortenerInterface $urlShortener, + LoggerInterface $logger = null + ) { $this->previewGenerator = $previewGenerator; $this->urlShortener = $urlShortener; + $this->logger = $logger ?: new NullLogger(); } /** @@ -53,6 +63,7 @@ class PreviewAction implements MiddlewareInterface $imagePath = $this->previewGenerator->generatePreview($url); return $this->generateImageResponse($imagePath); } catch (InvalidShortCodeException | EntityDoesNotExistException | PreviewGenerationException $e) { + $this->logger->warning('An error occurred while generating preview image.' . PHP_EOL . $e); return $this->buildErrorResponse($request, $handler); } } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 3e5aeaf9..81b1d326 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -15,6 +15,7 @@ use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; +use Zend\Expressive\Router\Exception\RuntimeException; use Zend\Expressive\Router\RouterInterface; class QrCodeAction implements MiddlewareInterface @@ -52,6 +53,8 @@ class QrCodeAction implements MiddlewareInterface * @param RequestHandlerInterface $handler * * @return Response + * @throws \InvalidArgumentException + * @throws RuntimeException */ public function process(Request $request, RequestHandlerInterface $handler): Response { @@ -59,11 +62,8 @@ class QrCodeAction implements MiddlewareInterface $shortCode = $request->getAttribute('shortCode'); try { $this->urlShortener->shortCodeToUrl($shortCode); - } catch (InvalidShortCodeException $e) { - $this->logger->warning('Tried to create a QR code with an invalid short code' . PHP_EOL . $e); - return $this->buildErrorResponse($request, $handler); - } catch (EntityDoesNotExistException $e) { - $this->logger->warning('Tried to create a QR code with a not found short code' . PHP_EOL . $e); + } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { + $this->logger->warning('An error occurred while creating QR code' . PHP_EOL . $e); return $this->buildErrorResponse($request, $handler); } diff --git a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php b/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php index efd0bc2a..89a88f47 100644 --- a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php +++ b/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Action\Util; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; trait ErrorResponseBuilderTrait { @@ -14,7 +14,7 @@ trait ErrorResponseBuilderTrait ServerRequestInterface $request, RequestHandlerInterface $handler ): ResponseInterface { - $request = $request->withAttribute(NotFoundDelegate::NOT_FOUND_TEMPLATE, 'ShlinkCore::invalid-short-code'); + $request = $request->withAttribute(NotFoundHandler::NOT_FOUND_TEMPLATE, 'ShlinkCore::invalid-short-code'); return $handler->handle($request); } } diff --git a/module/Core/src/Response/NotFoundDelegate.php b/module/Core/src/Response/NotFoundHandler.php similarity index 90% rename from module/Core/src/Response/NotFoundDelegate.php rename to module/Core/src/Response/NotFoundHandler.php index db5ab040..fe055311 100644 --- a/module/Core/src/Response/NotFoundDelegate.php +++ b/module/Core/src/Response/NotFoundHandler.php @@ -6,13 +6,13 @@ namespace Shlinkio\Shlink\Core\Response; use Fig\Http\Message\StatusCodeInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\RequestHandlerInterface as DelegateInterface; +use Psr\Http\Server\RequestHandlerInterface; use Zend\Diactoros\Response; use Zend\Expressive\Template\TemplateRendererInterface; -class NotFoundDelegate implements DelegateInterface +class NotFoundHandler implements RequestHandlerInterface { - const NOT_FOUND_TEMPLATE = 'notFoundTemplate'; + public const NOT_FOUND_TEMPLATE = 'notFoundTemplate'; /** * @var TemplateRendererInterface diff --git a/module/Core/test/Response/NotFoundDelegateTest.php b/module/Core/test/Response/NotFoundHandlerTest.php similarity index 88% rename from module/Core/test/Response/NotFoundDelegateTest.php rename to module/Core/test/Response/NotFoundHandlerTest.php index b3ce2e66..80b6171e 100644 --- a/module/Core/test/Response/NotFoundDelegateTest.php +++ b/module/Core/test/Response/NotFoundHandlerTest.php @@ -7,15 +7,15 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; use Zend\Expressive\Template\TemplateRendererInterface; -class NotFoundDelegateTest extends TestCase +class NotFoundHandlerTest extends TestCase { /** - * @var NotFoundDelegate + * @var NotFoundHandler */ private $delegate; /** @@ -26,7 +26,7 @@ class NotFoundDelegateTest extends TestCase public function setUp() { $this->renderer = $this->prophesize(TemplateRendererInterface::class); - $this->delegate = new NotFoundDelegate($this->renderer->reveal()); + $this->delegate = new NotFoundHandler($this->renderer->reveal()); } /** From 386b0dfb7bfec2c4bb2de01cc7e306f6aeb66807 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 7 May 2018 11:03:28 +0200 Subject: [PATCH 28/30] Updated changelog --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f4ed213..ace9527e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ ## CHANGELOG +### 1.9.0 + +**Features** + +* [147: Allow short URLs to be created on the fly with query param authentication](https://github.com/shlinkio/shlink/issues/147) + +**Bugs:** + +* [139: Make sure all core actions log exceptions](https://github.com/shlinkio/shlink/issues/139) + ### 1.8.1 **Tasks** From 7c6da4985d974a4bf92c3fc319857a1685886da4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 7 May 2018 11:09:32 +0200 Subject: [PATCH 29/30] Updated build script to delete more development-specific files --- build.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build.sh b/build.sh index b90d0b2e..de9102d6 100755 --- a/build.sh +++ b/build.sh @@ -33,8 +33,12 @@ rm composer.* rm LICENSE rm indocker rm docker-compose.yml +rm docker-compose.override.yml +rm docker-compose.override.yml.dist +rm func_tests_bootstrap.php rm php* rm README.md +rm infection.json rm -rf build rm -ff data/database.sqlite rm -rf data/infra From b0dbb2dae44ede2c48e70aa0160f0e1619317355 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 7 May 2018 11:17:10 +0200 Subject: [PATCH 30/30] Updated CreateShortCodeContentNegotiationMiddleware so that query parameter takes precedence over Accept header --- .../CreateShortCodeContentNegotiationMiddleware.php | 7 ++++--- .../CreateShortCodeContentNegotiationMiddlewareTest.php | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php index 6a76795e..3a2f0f4d 100644 --- a/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php +++ b/module/Rest/src/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddleware.php @@ -30,9 +30,10 @@ class CreateShortCodeContentNegotiationMiddleware implements MiddlewareInterface return $response; } - $acceptedType = $request->hasHeader('Accept') - ? $this->determineAcceptTypeFromHeader($request->getHeaderLine('Accept')) - : $this->determineAcceptTypeFromQuery($request->getQueryParams()); + $query = $request->getQueryParams(); + $acceptedType = isset($query['format']) + ? $this->determineAcceptTypeFromQuery($query) + : $this->determineAcceptTypeFromHeader($request->getHeaderLine('Accept')); // If JSON was requested, return the response from next handler as is if ($acceptedType === self::JSON) { diff --git a/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php index 8a867788..450b9e1c 100644 --- a/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php +++ b/module/Rest/test/Middleware/ShortCode/CreateShortCodeContentNegotiationMiddlewareTest.php @@ -74,6 +74,7 @@ class CreateShortCodeContentNegotiationMiddlewareTest extends TestCase ['application/json', [], 'application/json'], ['application/xml', [], 'application/json'], ['text/plain', [], 'text/plain'], + ['application/json', ['format' => 'txt'], 'text/plain'], ]; }