From 0e2a289f9f847cde8de4ce8b077229335bc166b8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 20:34:18 +0100 Subject: [PATCH 01/16] Updated to phpunit 6 --- composer.json | 7 +++---- config/config.php | 6 +++--- module/CLI/test/Command/Api/DisableKeyCommandTest.php | 2 +- module/CLI/test/Command/Api/GenerateKeyCommandTest.php | 2 +- module/CLI/test/Command/Api/ListKeysCommandTest.php | 2 +- .../CLI/test/Command/Config/GenerateCharsetCommandTest.php | 2 +- module/CLI/test/Command/Install/InstallCommandTest.php | 2 +- .../test/Command/Shortcode/GeneratePreviewCommandTest.php | 2 +- .../Command/Shortcode/GenerateShortcodeCommandTest.php | 2 +- module/CLI/test/Command/Shortcode/GetVisitsCommandTest.php | 2 +- .../test/Command/Shortcode/ListShortcodesCommandTest.php | 2 +- .../CLI/test/Command/Shortcode/ResolveUrlCommandTest.php | 2 +- module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php | 2 +- module/CLI/test/ConfigProviderTest.php | 2 +- module/CLI/test/Factory/ApplicationFactoryTest.php | 2 +- module/Common/test/ConfigProviderTest.php | 2 +- module/Common/test/Factory/CacheFactoryTest.php | 2 +- module/Common/test/Factory/EntityManagerFactoryTest.php | 2 +- module/Common/test/Factory/LoggerFactoryTest.php | 2 +- module/Common/test/Factory/TranslatorFactoryTest.php | 2 +- module/Common/test/Image/ImageBuilderFactoryTest.php | 2 +- module/Common/test/Image/ImageFactoryTest.php | 2 +- module/Common/test/Middleware/LocaleMiddlewareTest.php | 2 +- .../test/Paginator/PaginableRepositoryAdapterTest.php | 2 +- module/Common/test/Service/IpLocationResolverTest.php | 2 +- module/Common/test/Service/PreviewGeneratorTest.php | 2 +- .../Common/test/Twig/Extension/TranslatorExtensionTest.php | 2 +- module/Common/test/Util/DateRangeTest.php | 2 +- module/Core/test/Action/PreviewActionTest.php | 2 +- module/Core/test/Action/QrCodeActionTest.php | 2 +- module/Core/test/Action/RedirectActionTest.php | 2 +- module/Core/test/ConfigProviderTest.php | 2 +- module/Core/test/Entity/TagTest.php | 2 +- module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php | 2 +- module/Core/test/Options/AppOptionsFactoryTest.php | 2 +- module/Core/test/Service/ShortUrlServiceTest.php | 2 +- module/Core/test/Service/UrlShortenerTest.php | 2 +- module/Core/test/Service/VisitServiceTest.php | 2 +- module/Core/test/Service/VisitsTrackerTest.php | 2 +- module/Rest/test/Action/AuthenticateActionTest.php | 2 +- module/Rest/test/Action/CreateShortcodeActionTest.php | 2 +- module/Rest/test/Action/EditTagsActionTest.php | 2 +- module/Rest/test/Action/GetVisitsActionTest.php | 2 +- module/Rest/test/Action/ListShortcodesActionTest.php | 2 +- module/Rest/test/Action/ResolveUrlActionTest.php | 2 +- module/Rest/test/Authentication/JWTServiceTest.php | 2 +- module/Rest/test/ConfigProviderTest.php | 2 +- module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php | 2 +- module/Rest/test/Middleware/BodyParserMiddlewareTest.php | 2 +- .../test/Middleware/CheckAuthenticationMiddlewareTest.php | 2 +- module/Rest/test/Middleware/CrossDomainMiddlewareTest.php | 2 +- module/Rest/test/Middleware/PathVersionMiddlewareTest.php | 2 +- module/Rest/test/Service/ApiKeyServiceTest.php | 2 +- module/Rest/test/Util/RestUtilsTest.php | 2 +- 54 files changed, 58 insertions(+), 59 deletions(-) diff --git a/composer.json b/composer.json index f012cab4..da746f03 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "zendframework/zend-paginator": "^2.6", "zendframework/zend-config": "^2.6", "zendframework/zend-i18n": "^2.7", - "mtymek/expressive-config-manager": "^0.4", + "zendframework/zend-config-aggregator": "^0.1", "acelaya/zsm-annotated-services": "^0.2.0", "acelaya/ze-content-based-error-handler": "^1.0", "doctrine/orm": "^2.5", @@ -37,13 +37,12 @@ "doctrine/migrations": "^1.4" }, "require-dev": { - "phpunit/phpunit": "^5.0", + "phpunit/phpunit": "^5.7 || ^6.0", "squizlabs/php_codesniffer": "^2.3", "roave/security-advisories": "dev-master", "filp/whoops": "^2.0", "symfony/var-dumper": "^3.0", - "vlucas/phpdotenv": "^2.2", - "phly/changelog-generator": "^2.1" + "vlucas/phpdotenv": "^2.2" }, "autoload": { "psr-4": { diff --git a/config/config.php b/config/config.php index 5eec4734..32644060 100644 --- a/config/config.php +++ b/config/config.php @@ -4,7 +4,7 @@ use Shlinkio\Shlink\CLI; use Shlinkio\Shlink\Common; use Shlinkio\Shlink\Core; use Shlinkio\Shlink\Rest; -use Zend\Expressive\ConfigManager; +use Zend\ConfigAggregator; /** * Configuration files are loaded in a specific order. First ``global.php``, then ``*.global.php``. @@ -15,11 +15,11 @@ use Zend\Expressive\ConfigManager; * Obviously, if you use closures in your config you can't cache it. */ -return (new ConfigManager\ConfigManager([ +return (new ConfigAggregator\ConfigAggregator([ ExpressiveErrorHandler\ConfigProvider::class, Common\ConfigProvider::class, Core\ConfigProvider::class, CLI\ConfigProvider::class, Rest\ConfigProvider::class, - new ConfigManager\ZendConfigProvider('config/{autoload/{{,*.}global,{,*.}local},params/generated_config}.php'), + new ConfigAggregator\ZendConfigProvider('config/{autoload/{{,*.}global,{,*.}local},params/generated_config}.php'), ], 'data/cache/app_config.php'))->getMergedConfig(); diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php index 68d5f8c2..c265b3dd 100644 --- a/module/CLI/test/Command/Api/DisableKeyCommandTest.php +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -1,7 +1,7 @@ Date: Fri, 24 Mar 2017 21:10:25 +0100 Subject: [PATCH 02/16] Updated to expressive 2 and used new error handling system --- composer.json | 18 +++++------ config/autoload/dependencies.global.php | 2 ++ config/autoload/errorhandler.local.php.dist | 5 ++- .../autoload/middleware-pipeline.global.php | 31 +++++++++++++++++++ .../config/middleware-pipeline.config.php | 15 --------- module/Common/test/ConfigProviderTest.php | 1 - .../config/middleware-pipeline.config.php | 25 --------------- module/Rest/test/ConfigProviderTest.php | 1 - 8 files changed, 44 insertions(+), 54 deletions(-) delete mode 100644 module/Common/config/middleware-pipeline.config.php delete mode 100644 module/Rest/config/middleware-pipeline.config.php diff --git a/composer.json b/composer.json index da746f03..ec6e6ce9 100644 --- a/composer.json +++ b/composer.json @@ -1,29 +1,29 @@ { "name": "shlinkio/shlink", "type": "project", - "homepage": "http://shlink.io", + "homepage": "https://shlink.io", "description": "A self-hosted and PHP-based URL shortener application with CLI and REST interfaces", "license": "MIT", "authors": [ { "name": "Alejandro Celaya Alastrué", - "homepage": "http://www.alejandrocelaya.com", + "homepage": "https://www.alejandrocelaya.com", "email": "alejandro@alejandrocelaya.com" } ], "require": { "php": "^5.6 || ^7.0", - "zendframework/zend-expressive": "^1.0", - "zendframework/zend-expressive-fastroute": "^1.3", - "zendframework/zend-expressive-twigrenderer": "^1.0", - "zendframework/zend-stdlib": "^2.7", + "zendframework/zend-expressive": "^2.0", + "zendframework/zend-expressive-fastroute": "^2.0", + "zendframework/zend-expressive-twigrenderer": "^1.4", + "zendframework/zend-stdlib": "^3.0", "zendframework/zend-servicemanager": "^3.0", "zendframework/zend-paginator": "^2.6", - "zendframework/zend-config": "^2.6", + "zendframework/zend-config": "^3.0", "zendframework/zend-i18n": "^2.7", "zendframework/zend-config-aggregator": "^0.1", - "acelaya/zsm-annotated-services": "^0.2.0", - "acelaya/ze-content-based-error-handler": "^1.0", + "acelaya/zsm-annotated-services": "^1.0", + "acelaya/ze-content-based-error-handler": "^2.0", "doctrine/orm": "^2.5", "guzzlehttp/guzzle": "^6.2", "symfony/console": "^3.0", diff --git a/config/autoload/dependencies.global.php b/config/autoload/dependencies.global.php index d014f9ab..e2b88ba8 100644 --- a/config/autoload/dependencies.global.php +++ b/config/autoload/dependencies.global.php @@ -4,6 +4,7 @@ use Zend\Expressive\Container; use Zend\Expressive\Router; use Zend\Expressive\Template; use Zend\Expressive\Twig; +use Zend\Stratigility\Middleware\ErrorHandler; return [ @@ -13,6 +14,7 @@ return [ Template\TemplateRendererInterface::class => Twig\TwigRendererFactory::class, \Twig_Environment::class => Twig\TwigEnvironmentFactory::class, Router\RouterInterface::class => Router\FastRouteRouterFactory::class, + ErrorHandler::class => Container\ErrorHandlerFactory::class, ], ], diff --git a/config/autoload/errorhandler.local.php.dist b/config/autoload/errorhandler.local.php.dist index 40316fd9..552b6ffb 100644 --- a/config/autoload/errorhandler.local.php.dist +++ b/config/autoload/errorhandler.local.php.dist @@ -1,6 +1,5 @@ [ @@ -21,7 +20,7 @@ return [ 'error_handler' => [ 'plugins' => [ 'factories' => [ - ContentBasedErrorHandler::DEFAULT_CONTENT => WhoopsErrorHandlerFactory::class, + 'text/html' => WhoopsErrorResponseGeneratorFactory::class, ], ], ], diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 0e214307..e07149a5 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -1,9 +1,30 @@ [ + 'pre-routing' => [ + 'middleware' => [ + ErrorHandler::class, + LocaleMiddleware::class, + ], + 'priority' => 11, + ], + 'pre-routing-rest' => [ + 'path' => '/rest', + 'middleware' => [ + PathVersionMiddleware::class, + ], + 'priority' => 11, + ], + 'routing' => [ 'middleware' => [ ApplicationFactory::ROUTING_MIDDLEWARE, @@ -11,6 +32,16 @@ return [ 'priority' => 10, ], + 'rest' => [ + 'path' => '/rest', + 'middleware' => [ + CrossDomainMiddleware::class, + BodyParserMiddleware::class, + CheckAuthenticationMiddleware::class, + ], + 'priority' => 5, + ], + 'post-routing' => [ 'middleware' => [ ApplicationFactory::DISPATCH_MIDDLEWARE, diff --git a/module/Common/config/middleware-pipeline.config.php b/module/Common/config/middleware-pipeline.config.php deleted file mode 100644 index aab5af85..00000000 --- a/module/Common/config/middleware-pipeline.config.php +++ /dev/null @@ -1,15 +0,0 @@ - [ - 'pre-routing' => [ - 'middleware' => [ - Middleware\LocaleMiddleware::class, - ], - 'priority' => 5, - ], - ], - -]; diff --git a/module/Common/test/ConfigProviderTest.php b/module/Common/test/ConfigProviderTest.php index 920e94e4..05b5da49 100644 --- a/module/Common/test/ConfigProviderTest.php +++ b/module/Common/test/ConfigProviderTest.php @@ -23,7 +23,6 @@ class ConfigProviderTest extends TestCase { $config = $this->configProvider->__invoke(); - $this->assertArrayHasKey('middleware_pipeline', $config); $this->assertArrayHasKey('dependencies', $config); $this->assertArrayHasKey('twig', $config); } diff --git a/module/Rest/config/middleware-pipeline.config.php b/module/Rest/config/middleware-pipeline.config.php deleted file mode 100644 index 19f20198..00000000 --- a/module/Rest/config/middleware-pipeline.config.php +++ /dev/null @@ -1,25 +0,0 @@ - [ - 'pre-routing' => [ - 'path' => '/rest', - 'middleware' => [ - Middleware\PathVersionMiddleware::class, - ], - 'priority' => 11, - ], - - 'rest' => [ - 'path' => '/rest', - 'middleware' => [ - Middleware\CrossDomainMiddleware::class, - Middleware\BodyParserMiddleware::class, - Middleware\CheckAuthenticationMiddleware::class, - ], - 'priority' => 5, - ], - ], -]; diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index d6aedf79..a512e31a 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -24,7 +24,6 @@ class ConfigProviderTest extends TestCase $config = $this->configProvider->__invoke(); $this->assertArrayHasKey('error_handler', $config); - $this->assertArrayHasKey('middleware_pipeline', $config); $this->assertArrayHasKey('routes', $config); $this->assertArrayHasKey('dependencies', $config); $this->assertArrayHasKey('translator', $config); From fe7928ae0e1923af83780f040820634b1b954cd4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 21:31:55 +0100 Subject: [PATCH 03/16] Fixed JsonErrorHandler and prevented AuthorizationMiddleware to eat exceptions --- .../src/ErrorHandler/JsonErrorHandler.php | 22 +++++++------------ .../CheckAuthenticationMiddleware.php | 6 ----- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/module/Rest/src/ErrorHandler/JsonErrorHandler.php b/module/Rest/src/ErrorHandler/JsonErrorHandler.php index 79667c73..3dc55e8b 100644 --- a/module/Rest/src/ErrorHandler/JsonErrorHandler.php +++ b/module/Rest/src/ErrorHandler/JsonErrorHandler.php @@ -1,34 +1,28 @@ getAttribute(RouteResult::class) !== null; - $isNotFound = ! $hasRoute && ! isset($err); - if ($isNotFound) { - $responsePhrase = 'Not found'; - $status = 404; - } else { - $status = $response->getStatusCode(); - $responsePhrase = $status < 400 ? 'Internal Server Error' : $response->getReasonPhrase(); - $status = $status < 400 ? 500 : $status; - } + $status = $response->getStatusCode(); + $responsePhrase = $status < 400 ? 'Internal Server Error' : $response->getReasonPhrase(); + $status = $status < 400 ? self::STATUS_INTERNAL_SERVER_ERROR : $status; return new JsonResponse([ 'error' => $this->responsePhraseToCode($responsePhrase), diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index 5b18ea31..0e7c1cdb 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php @@ -134,12 +134,6 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface } catch (AuthenticationException $e) { $this->logger->warning('Tried to access API with an invalid JWT.' . PHP_EOL . $e); return $this->createTokenErrorResponse(); - } catch (\Exception $e) { - $this->logger->warning('Unexpected error occurred.' . PHP_EOL . $e); - return $this->createTokenErrorResponse(); - } catch (\Throwable $e) { - $this->logger->warning('Unexpected error occurred.' . PHP_EOL . $e); - return $this->createTokenErrorResponse(); } finally { ErrorHandler::clean(); } From d1018b6da7428e82dd96d80906f9a3e4ae5df497 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 21:38:43 +0100 Subject: [PATCH 04/16] Fixed tests --- .../ErrorHandler/JsonErrorHandlerTest.php | 47 +++---------------- .../CheckAuthenticationMiddlewareTest.php | 15 +++--- 2 files changed, 14 insertions(+), 48 deletions(-) diff --git a/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php b/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php index 1e410d67..75d4ad0e 100644 --- a/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php +++ b/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php @@ -5,7 +5,6 @@ use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Rest\ErrorHandler\JsonErrorHandler; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; -use Zend\Expressive\Router\RouteResult; class JsonErrorHandlerTest extends TestCase { @@ -22,58 +21,24 @@ class JsonErrorHandlerTest extends TestCase /** * @test */ - public function noMatchedRouteReturnsNotFoundResponse() + public function noErrorStatusReturnsInternalServerError() { - $response = $this->errorHandler->__invoke(ServerRequestFactory::fromGlobals(), new Response()); + $response = $this->errorHandler->__invoke(null, ServerRequestFactory::fromGlobals(), new Response()); $this->assertInstanceOf(Response\JsonResponse::class, $response); - $this->assertEquals(404, $response->getStatusCode()); + $this->assertEquals(500, $response->getStatusCode()); } /** * @test */ - public function matchedRouteWithErrorReturnsMethodNotAllowedResponse() + public function errorStatusReturnsThatStatus() { $response = $this->errorHandler->__invoke( + null, ServerRequestFactory::fromGlobals(), - (new Response())->withStatus(405), - 405 + (new Response())->withStatus(405) ); $this->assertInstanceOf(Response\JsonResponse::class, $response); $this->assertEquals(405, $response->getStatusCode()); } - - /** - * @test - */ - public function responseWithErrorKeepsStatus() - { - $response = $this->errorHandler->__invoke( - ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRouteMatch('foo', 'bar', []) - ), - (new Response())->withStatus(401), - 401 - ); - $this->assertInstanceOf(Response\JsonResponse::class, $response); - $this->assertEquals(401, $response->getStatusCode()); - } - - /** - * @test - */ - public function responseWithoutErrorReturnsStatus500() - { - $response = $this->errorHandler->__invoke( - ServerRequestFactory::fromGlobals()->withAttribute( - RouteResult::class, - RouteResult::fromRouteMatch('foo', 'bar', []) - ), - (new Response())->withStatus(200), - 'Some error' - ); - $this->assertInstanceOf(Response\JsonResponse::class, $response); - $this->assertEquals(500, $response->getStatusCode()); - } } diff --git a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php index fca26c2e..4f472726 100644 --- a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php @@ -7,6 +7,7 @@ use Shlinkio\Shlink\Rest\Authentication\JWTService; use Shlinkio\Shlink\Rest\Middleware\CheckAuthenticationMiddleware; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; +use Zend\Expressive\Router\Route; use Zend\Expressive\Router\RouteResult; use Zend\I18n\Translator\Translator; @@ -55,7 +56,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, - RouteResult::fromRouteMatch('rest-authenticate', 'foo', []) + RouteResult::fromRoute(new Route('foo', '', Route::HTTP_METHOD_ANY, 'rest-authenticate'), []) ); $response = new Response(); $isCalled = false; @@ -67,7 +68,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, - RouteResult::fromRouteMatch('bar', 'foo', []) + RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withMethod('OPTIONS'); $response = new Response(); $isCalled = false; @@ -85,7 +86,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase { $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, - RouteResult::fromRouteMatch('bar', 'foo', []) + RouteResult::fromRoute(new Route('bar', 'foo'), []) ); $response = $this->middleware->__invoke($request, new Response()); $this->assertEquals(401, $response->getStatusCode()); @@ -99,7 +100,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, - RouteResult::fromRouteMatch('bar', 'foo', []) + RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); $response = $this->middleware->__invoke($request, new Response()); @@ -115,7 +116,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, - RouteResult::fromRouteMatch('bar', 'foo', []) + RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Basic ' . $authToken); $response = $this->middleware->__invoke($request, new Response()); @@ -133,7 +134,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, - RouteResult::fromRouteMatch('bar', 'foo', []) + RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Bearer ' . $authToken); $this->jwtService->verify($authToken)->willReturn(false)->shouldBeCalledTimes(1); @@ -149,7 +150,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, - RouteResult::fromRouteMatch('bar', 'foo', []) + RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'bearer ' . $authToken); $this->jwtService->verify($authToken)->willReturn(true)->shouldBeCalledTimes(1); $this->jwtService->refresh($authToken)->willReturn($authToken)->shouldBeCalledTimes(1); From c3c03a3a3b701b7c7027f8742d116e75833765bb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 21:49:31 +0100 Subject: [PATCH 05/16] Migrated LocaleMiddleware to psr-15 middleware --- .../src/Middleware/LocaleMiddleware.php | 37 ++++++------------- .../test/Middleware/LocaleMiddlewareTest.php | 20 ++++------ module/Common/test/Util/TestUtils.php | 35 ++++++++++++++++++ 3 files changed, 54 insertions(+), 38 deletions(-) create mode 100644 module/Common/test/Util/TestUtils.php diff --git a/module/Common/src/Middleware/LocaleMiddleware.php b/module/Common/src/Middleware/LocaleMiddleware.php index 20f796ff..0cb81502 100644 --- a/module/Common/src/Middleware/LocaleMiddleware.php +++ b/module/Common/src/Middleware/LocaleMiddleware.php @@ -2,10 +2,11 @@ namespace Shlinkio\Shlink\Common\Middleware; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; +use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Zend\I18n\Translator\Translator; -use Zend\Stratigility\MiddlewareInterface; class LocaleMiddleware implements MiddlewareInterface { @@ -25,40 +26,26 @@ class LocaleMiddleware implements MiddlewareInterface $this->translator = $translator; } + + /** - * Process an incoming request and/or response. - * - * Accepts a server-side request and a response instance, and does - * something with them. - * - * If the response is not complete and/or further processing would not - * interfere with the work done in the middleware, or if the middleware - * wants to delegate to another process, it can use the `$out` callable - * if present. - * - * If the middleware does not return a value, execution of the current - * request is considered complete, and the response instance provided will - * be considered the response to return. - * - * Alternately, the middleware may return a response instance. - * - * Often, middleware will `return $out();`, with the assumption that a - * later middleware will return a response. + * Process an incoming server request and return a response, optionally delegating + * to the next middleware component to create the response. * * @param Request $request - * @param Response $response - * @param null|callable $out - * @return null|Response + * @param DelegateInterface $delegate + * + * @return Response */ - public function __invoke(Request $request, Response $response, callable $out = null) + public function process(Request $request, DelegateInterface $delegate) { if (! $request->hasHeader('Accept-Language')) { - return $out($request, $response); + return $delegate->process($request); } $locale = $request->getHeaderLine('Accept-Language'); $this->translator->setLocale($this->normalizeLocale($locale)); - return $out($request, $response); + return $delegate->process($request); } /** diff --git a/module/Common/test/Middleware/LocaleMiddlewareTest.php b/module/Common/test/Middleware/LocaleMiddlewareTest.php index 4d9e81ce..762dc31f 100644 --- a/module/Common/test/Middleware/LocaleMiddlewareTest.php +++ b/module/Common/test/Middleware/LocaleMiddlewareTest.php @@ -3,7 +3,7 @@ namespace ShlinkioTest\Shlink\Common\Middleware; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Middleware\LocaleMiddleware; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; @@ -30,9 +30,7 @@ class LocaleMiddlewareTest extends TestCase public function whenNoHeaderIsPresentLocaleIsNotChanged() { $this->assertEquals('ru', $this->translator->getLocale()); - $this->middleware->__invoke(ServerRequestFactory::fromGlobals(), new Response(), function ($req, $resp) { - return $resp; - }); + $this->middleware->process(ServerRequestFactory::fromGlobals(), TestUtils::createDelegateMock()->reveal()); $this->assertEquals('ru', $this->translator->getLocale()); } @@ -43,9 +41,7 @@ class LocaleMiddlewareTest extends TestCase { $this->assertEquals('ru', $this->translator->getLocale()); $request = ServerRequestFactory::fromGlobals()->withHeader('Accept-Language', 'es'); - $this->middleware->__invoke($request, new Response(), function ($req, $resp) { - return $resp; - }); + $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals('es', $this->translator->getLocale()); } @@ -54,18 +50,16 @@ class LocaleMiddlewareTest extends TestCase */ public function localeGetsNormalized() { + $delegate = TestUtils::createDelegateMock(); + $this->assertEquals('ru', $this->translator->getLocale()); $request = ServerRequestFactory::fromGlobals()->withHeader('Accept-Language', 'es_ES'); - $this->middleware->__invoke($request, new Response(), function ($req, $resp) { - return $resp; - }); + $this->middleware->process($request, $delegate->reveal()); $this->assertEquals('es', $this->translator->getLocale()); $request = ServerRequestFactory::fromGlobals()->withHeader('Accept-Language', 'en-US'); - $this->middleware->__invoke($request, new Response(), function ($req, $resp) { - return $resp; - }); + $this->middleware->process($request, $delegate->reveal()); $this->assertEquals('en', $this->translator->getLocale()); } } diff --git a/module/Common/test/Util/TestUtils.php b/module/Common/test/Util/TestUtils.php new file mode 100644 index 00000000..8bd4969d --- /dev/null +++ b/module/Common/test/Util/TestUtils.php @@ -0,0 +1,35 @@ +prophesize(DelegateInterface::class); + $delegate->process($argument)->willReturn($response ?: new Response()); + + return $delegate; + } + + /** + * @return Prophet + */ + private static function getProphet() + { + if (static::$prophet === null) { + static::$prophet = new Prophet(); + } + + return static::$prophet; + } +} From 7530048fbdd0b530c175a3ddbca9b8b46161af05 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 21:58:56 +0100 Subject: [PATCH 06/16] Removed exception catch that used to return a 500, and now returns a 404 due to a behavior change --- data/proxies/.gitignore | 0 module/Core/src/Action/PreviewAction.php | 2 -- module/Core/test/Action/PreviewActionTest.php | 20 ------------------- 3 files changed, 22 deletions(-) mode change 100644 => 100755 data/proxies/.gitignore diff --git a/data/proxies/.gitignore b/data/proxies/.gitignore old mode 100644 new mode 100755 diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index 4c21501c..f2145e1b 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -78,8 +78,6 @@ class PreviewAction implements MiddlewareInterface return $this->generateImageResponse($imagePath); } catch (InvalidShortCodeException $e) { return $out($request, $response->withStatus(404), 'Not found'); - } catch (PreviewGenerationException $e) { - return $out($request, $response->withStatus(500), 'Preview generation error'); } } } diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index 2e8e0149..8ef2883a 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -91,24 +91,4 @@ class PreviewActionTest extends TestCase $this->assertEquals(404, $resp->getStatusCode()); } - - /** - * @test - */ - public function previewExceptionReturnsNotFound() - { - $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(PreviewGenerationException::class) - ->shouldBeCalledTimes(1); - - $resp = $this->action->__invoke( - ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response(), - function ($req, $resp) { - return $resp; - } - ); - - $this->assertEquals(500, $resp->getStatusCode()); - } } From 46db736af82127beaeb3c0667f0c2a778e96ba70 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 22:07:28 +0100 Subject: [PATCH 07/16] Migrated PreviewAction to psr-15 middleware --- module/Core/src/Action/PreviewAction.php | 36 ++++++------------- module/Core/test/Action/PreviewActionTest.php | 30 ++++++++-------- 2 files changed, 24 insertions(+), 42 deletions(-) diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index f2145e1b..291225be 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -2,16 +2,16 @@ namespace Shlinkio\Shlink\Core\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; +use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Common\Exception\PreviewGenerationException; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Common\Service\PreviewGeneratorInterface; use Shlinkio\Shlink\Common\Util\ResponseUtilsTrait; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; -use Zend\Stratigility\MiddlewareInterface; class PreviewAction implements MiddlewareInterface { @@ -40,44 +40,28 @@ class PreviewAction implements MiddlewareInterface } /** - * Process an incoming request and/or response. - * - * Accepts a server-side request and a response instance, and does - * something with them. - * - * If the response is not complete and/or further processing would not - * interfere with the work done in the middleware, or if the middleware - * wants to delegate to another process, it can use the `$out` callable - * if present. - * - * If the middleware does not return a value, execution of the current - * request is considered complete, and the response instance provided will - * be considered the response to return. - * - * Alternately, the middleware may return a response instance. - * - * Often, middleware will `return $out();`, with the assumption that a - * later middleware will return a response. + * Process an incoming server request and return a response, optionally delegating + * to the next middleware component to create the response. * * @param Request $request - * @param Response $response - * @param null|callable $out - * @return null|Response + * @param DelegateInterface $delegate + * + * @return Response */ - public function __invoke(Request $request, Response $response, callable $out = null) + public function process(Request $request, DelegateInterface $delegate) { $shortCode = $request->getAttribute('shortCode'); try { $url = $this->urlShortener->shortCodeToUrl($shortCode); if (! isset($url)) { - return $out($request, $response->withStatus(404), 'Not found'); + return $delegate->process($request); } $imagePath = $this->previewGenerator->generatePreview($url); return $this->generateImageResponse($imagePath); } catch (InvalidShortCodeException $e) { - return $out($request, $response->withStatus(404), 'Not found'); + return $delegate->process($request); } } } diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index 8ef2883a..a5e57d2d 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -2,12 +2,14 @@ namespace ShlinkioTest\Shlink\Core\Action; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Exception\PreviewGenerationException; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Action\PreviewAction; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; @@ -36,20 +38,18 @@ class PreviewActionTest extends TestCase /** * @test */ - public function invalidShortCodeFallbacksToNextMiddlewareWithStatusNotFound() + public function invalidShortCodeFallsBackToNextMiddleware() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); + $delegate = TestUtils::createDelegateMock(); - $resp = $this->action->__invoke( + $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response(), - function ($req, $resp) { - return $resp; - } + $delegate->reveal() ); - $this->assertEquals(404, $resp->getStatusCode()); + $delegate->process(Argument::cetera())->shouldHaveBeenCalledTimes(1); } /** @@ -63,9 +63,9 @@ class PreviewActionTest extends TestCase $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($url)->shouldBeCalledTimes(1); $this->previewGenerator->generatePreview($url)->willReturn($path)->shouldBeCalledTimes(1); - $resp = $this->action->__invoke( + $resp = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(filesize($path), $resp->getHeaderLine('Content-length')); @@ -75,20 +75,18 @@ class PreviewActionTest extends TestCase /** * @test */ - public function invalidShortcodeExceptionReturnsNotFound() + public function invalidShortcodeExceptionFallsBackToNextMiddleware() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) ->shouldBeCalledTimes(1); + $delegate = TestUtils::createDelegateMock(); - $resp = $this->action->__invoke( + $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response(), - function ($req, $resp) { - return $resp; - } + $delegate->reveal() ); - $this->assertEquals(404, $resp->getStatusCode()); + $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1); } } From 85ca366893c5f74e9583956efd9c45d67abb5e68 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 23:19:42 +0100 Subject: [PATCH 08/16] Migrated QrCodeAction to psr-15 middleware --- module/Core/src/Action/QrCodeAction.php | 43 ++++++-------------- module/Core/test/Action/QrCodeActionTest.php | 35 ++++++++-------- 2 files changed, 29 insertions(+), 49 deletions(-) diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 5d0549eb..3970d740 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -3,6 +3,8 @@ namespace Shlinkio\Shlink\Core\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Endroid\QrCode\QrCode; +use Interop\Http\ServerMiddleware\DelegateInterface; +use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -12,7 +14,6 @@ use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Zend\Expressive\Router\RouterInterface; -use Zend\Stratigility\MiddlewareInterface; class QrCodeAction implements MiddlewareInterface { @@ -48,42 +49,26 @@ class QrCodeAction implements MiddlewareInterface } /** - * Process an incoming request and/or response. - * - * Accepts a server-side request and a response instance, and does - * something with them. - * - * If the response is not complete and/or further processing would not - * interfere with the work done in the middleware, or if the middleware - * wants to delegate to another process, it can use the `$out` callable - * if present. - * - * If the middleware does not return a value, execution of the current - * request is considered complete, and the response instance provided will - * be considered the response to return. - * - * Alternately, the middleware may return a response instance. - * - * Often, middleware will `return $out();`, with the assumption that a - * later middleware will return a response. + * Process an incoming server request and return a response, optionally delegating + * to the next middleware component to create the response. * * @param Request $request - * @param Response $response - * @param null|callable $out - * @return null|Response + * @param DelegateInterface $delegate + * + * @return Response */ - public function __invoke(Request $request, Response $response, callable $out = null) + public function process(Request $request, DelegateInterface $delegate) { // Make sure the short URL exists for this short code $shortCode = $request->getAttribute('shortCode'); try { $shortUrl = $this->urlShortener->shortCodeToUrl($shortCode); - if (! isset($shortUrl)) { - return $out($request, $response->withStatus(404), 'Not Found'); + if ($shortUrl === null) { + return $delegate->process($request); } } catch (InvalidShortCodeException $e) { $this->logger->warning('Tried to create a QR code with an invalid short code' . PHP_EOL . $e); - return $out($request, $response->withStatus(404), 'Not Found'); + return $delegate->process($request); } $path = $this->router->generateUri('long-url-redirect', ['shortCode' => $shortCode]); @@ -101,13 +86,11 @@ class QrCodeAction implements MiddlewareInterface */ protected function getSizeParam(Request $request) { - $size = intval($request->getAttribute('size', 300)); + $size = (int) $request->getAttribute('size', 300); if ($size < 50) { return 50; - } elseif ($size > 1000) { - return 1000; } - return $size; + return $size > 1000 ? 1000 : $size; } } diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 6ef8a5f5..dc005b11 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Action\QrCodeAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\Expressive\Router\RouterInterface; @@ -37,19 +37,18 @@ class QrCodeActionTest extends TestCase /** * @test */ - public function aNonexistentShortCodeWillReturnNotFoundResponse() + public function aNotFoundShortCodeWillDelegateIntoNextMiddleware() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); + $delegate = TestUtils::createDelegateMock(); - $resp = $this->action->__invoke( + $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response(), - function ($req, $resp) { - return $resp; - } + $delegate->reveal() ); - $this->assertEquals(404, $resp->getStatusCode()); + + $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1); } /** @@ -60,15 +59,14 @@ class QrCodeActionTest extends TestCase $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) ->shouldBeCalledTimes(1); + $delegate = TestUtils::createDelegateMock(); - $resp = $this->action->__invoke( + $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response(), - function ($req, $resp) { - return $resp; - } + $delegate->reveal() ); - $this->assertEquals(404, $resp->getStatusCode()); + + $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1); } /** @@ -78,16 +76,15 @@ class QrCodeActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(new ShortUrl())->shouldBeCalledTimes(1); + $delegate = TestUtils::createDelegateMock(); - $resp = $this->action->__invoke( + $resp = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response(), - function ($req, $resp) { - return $resp; - } + $delegate->reveal() ); $this->assertInstanceOf(QrCodeResponse::class, $resp); $this->assertEquals(200, $resp->getStatusCode()); + $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(0); } } From 734dac9456f3ea2b703b650e0bc50b5211109d60 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 23:24:11 +0100 Subject: [PATCH 09/16] Migrated RedirectAction to psr-15 middleware --- module/Core/src/Action/RedirectAction.php | 48 +++++-------------- .../Core/test/Action/RedirectActionTest.php | 45 +++++------------ 2 files changed, 23 insertions(+), 70 deletions(-) diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index a86a98c4..d8b4a04e 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -2,6 +2,8 @@ namespace Shlinkio\Shlink\Core\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; +use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -11,7 +13,6 @@ use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Service\VisitsTracker; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Zend\Diactoros\Response\RedirectResponse; -use Zend\Stratigility\MiddlewareInterface; class RedirectAction implements MiddlewareInterface { @@ -47,31 +48,15 @@ class RedirectAction implements MiddlewareInterface } /** - * Process an incoming request and/or response. - * - * Accepts a server-side request and a response instance, and does - * something with them. - * - * If the response is not complete and/or further processing would not - * interfere with the work done in the middleware, or if the middleware - * wants to delegate to another process, it can use the `$out` callable - * if present. - * - * If the middleware does not return a value, execution of the current - * request is considered complete, and the response instance provided will - * be considered the response to return. - * - * Alternately, the middleware may return a response instance. - * - * Often, middleware will `return $out();`, with the assumption that a - * later middleware will return a response. + * Process an incoming server request and return a response, optionally delegating + * to the next middleware component to create the response. * * @param Request $request - * @param Response $response - * @param null|callable $out - * @return null|Response + * @param DelegateInterface $delegate + * + * @return Response */ - public function __invoke(Request $request, Response $response, callable $out = null) + public function process(Request $request, DelegateInterface $delegate) { $shortCode = $request->getAttribute('shortCode', ''); @@ -80,8 +65,8 @@ class RedirectAction implements MiddlewareInterface // If provided shortCode does not belong to a valid long URL, dispatch next middleware, which will trigger // a not-found error - if (! isset($longUrl)) { - return $this->notFoundResponse($request, $response, $out); + if ($longUrl === null) { + return $delegate->process($request); } // Track visit to this short code @@ -93,18 +78,7 @@ class RedirectAction implements MiddlewareInterface } catch (\Exception $e) { // In case of error, dispatch 404 error $this->logger->error('Error redirecting to long URL.' . PHP_EOL . $e); - return $this->notFoundResponse($request, $response, $out); + return $delegate->process($request); } } - - /** - * @param Request $request - * @param Response $response - * @param callable $out - * @return Response - */ - protected function notFoundResponse(Request $request, Response $response, callable $out) - { - return $out($request, $response->withStatus(404), 'Not Found'); - } } diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 75726684..65b6250f 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -4,11 +4,10 @@ namespace ShlinkioTest\Shlink\Core\Action; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; -use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\VisitsTracker; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; @@ -42,7 +41,7 @@ class RedirectActionTest extends TestCase ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertInstanceOf(Response\RedirectResponse::class, $response); $this->assertEquals(302, $response->getStatusCode()); @@ -53,52 +52,32 @@ class RedirectActionTest extends TestCase /** * @test */ - public function nextErrorMiddlewareIsInvokedIfLongUrlIsNotFound() + public function nextMiddlewareIsInvokedIfLongUrlIsNotFound() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null) ->shouldBeCalledTimes(1); + $delegate = TestUtils::createDelegateMock(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $originalResponse = new Response(); - $test = $this; - $this->action->__invoke($request, $originalResponse, function ( - ServerRequestInterface $req, - ResponseInterface $resp, - $error - ) use ( - $test, - $request - ) { - $test->assertSame($request, $req); - $test->assertEquals(404, $resp->getStatusCode()); - $test->assertEquals('Not Found', $error); - }); + $this->action->process($request, $delegate->reveal()); + + $delegate->process($request)->shouldHaveBeenCalledTimes(1); } /** * @test */ - public function nextErrorMiddlewareIsInvokedIfAnExceptionIsThrown() + public function nextMiddlewareIsInvokedIfAnExceptionIsThrown() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(\Exception::class) ->shouldBeCalledTimes(1); + $delegate = TestUtils::createDelegateMock(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $originalResponse = new Response(); - $test = $this; - $this->action->__invoke($request, $originalResponse, function ( - ServerRequestInterface $req, - ResponseInterface $resp, - $error - ) use ( - $test, - $request - ) { - $test->assertSame($request, $req); - $test->assertEquals(404, $resp->getStatusCode()); - $test->assertEquals('Not Found', $error); - }); + $this->action->process($request, $delegate->reveal()); + + $delegate->process($request)->shouldHaveBeenCalledTimes(1); } } From 6c87436a966b6bca30d5fb541ba1b4786b7b4b23 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 24 Mar 2017 23:34:17 +0100 Subject: [PATCH 10/16] Migrated QrCodeCacheMiddleware to psr-15 middleware --- .../src/Middleware/QrCodeCacheMiddleware.php | 35 ++++++------------- .../Middleware/QrCodeCacheMiddlewareTest.php | 31 ++++++++-------- 2 files changed, 25 insertions(+), 41 deletions(-) diff --git a/module/Core/src/Middleware/QrCodeCacheMiddleware.php b/module/Core/src/Middleware/QrCodeCacheMiddleware.php index d0f132bb..67e9bdab 100644 --- a/module/Core/src/Middleware/QrCodeCacheMiddleware.php +++ b/module/Core/src/Middleware/QrCodeCacheMiddleware.php @@ -3,9 +3,11 @@ namespace Shlinkio\Shlink\Core\Middleware; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; use Doctrine\Common\Cache\Cache; +use Interop\Http\ServerMiddleware\DelegateInterface; +use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Zend\Stratigility\MiddlewareInterface; +use Zend\Diactoros\Response as DiactResp; class QrCodeCacheMiddleware implements MiddlewareInterface { @@ -26,44 +28,29 @@ class QrCodeCacheMiddleware implements MiddlewareInterface } /** - * Process an incoming request and/or response. - * - * Accepts a server-side request and a response instance, and does - * something with them. - * - * If the response is not complete and/or further processing would not - * interfere with the work done in the middleware, or if the middleware - * wants to delegate to another process, it can use the `$out` callable - * if present. - * - * If the middleware does not return a value, execution of the current - * request is considered complete, and the response instance provided will - * be considered the response to return. - * - * Alternately, the middleware may return a response instance. - * - * Often, middleware will `return $out();`, with the assumption that a - * later middleware will return a response. + * Process an incoming server request and return a response, optionally delegating + * to the next middleware component to create the response. * * @param Request $request - * @param Response $response - * @param null|callable $out - * @return null|Response + * @param DelegateInterface $delegate + * + * @return Response */ - public function __invoke(Request $request, Response $response, callable $out = null) + public function process(Request $request, DelegateInterface $delegate) { $cacheKey = $request->getUri()->getPath(); // If this QR code is already cached, just return it if ($this->cache->contains($cacheKey)) { $qrData = $this->cache->fetch($cacheKey); + $response = new DiactResp(); $response->getBody()->write($qrData['body']); return $response->withHeader('Content-Type', $qrData['content-type']); } // If not, call the next middleware and cache it /** @var Response $resp */ - $resp = $out($request, $response); + $resp = $delegate->process($request); $this->cache->save($cacheKey, [ 'body' => $resp->getBody()->__toString(), 'content-type' => $resp->getHeaderLine('Content-Type'), diff --git a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php index 50c2e736..abc1801d 100644 --- a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php +++ b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php @@ -4,7 +4,9 @@ namespace ShlinkioTest\Shlink\Core\Middleware; use Doctrine\Common\Cache\ArrayCache; use Doctrine\Common\Cache\Cache; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Shlinkio\Shlink\Core\Middleware\QrCodeCacheMiddleware; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\Uri; @@ -29,18 +31,15 @@ class QrCodeCacheMiddlewareTest extends TestCase /** * @test */ - public function noCachedPathFallbacksToNextMiddleware() + public function noCachedPathFallsBackToNextMiddleware() { - $isCalled = false; - $this->middleware->__invoke( - ServerRequestFactory::fromGlobals(), - new Response(), - function ($req, $resp) use (&$isCalled) { - $isCalled = true; - return $resp; - } - ); - $this->assertTrue($isCalled); + $delegate = TestUtils::createDelegateMock(); + $this->middleware->process(ServerRequestFactory::fromGlobals()->withUri( + new Uri('/foo/bar') + ), $delegate->reveal()); + + $this->assertTrue($this->cache->contains('/foo/bar')); + $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1); } /** @@ -51,19 +50,17 @@ class QrCodeCacheMiddlewareTest extends TestCase $isCalled = false; $uri = (new Uri())->withPath('/foo'); $this->cache->save('/foo', ['body' => 'the body', 'content-type' => 'image/png']); + $delegate = TestUtils::createDelegateMock(); - $resp = $this->middleware->__invoke( + $resp = $this->middleware->process( ServerRequestFactory::fromGlobals()->withUri($uri), - new Response(), - function ($req, $resp) use (&$isCalled) { - $isCalled = true; - return $resp; - } + $delegate->reveal() ); $this->assertFalse($isCalled); $resp->getBody()->rewind(); $this->assertEquals('the body', $resp->getBody()->getContents()); $this->assertEquals('image/png', $resp->getHeaderLine('Content-Type')); + $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(0); } } From 22c76df8e6065fa889892e3c7180fdce56db74a1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2017 09:22:00 +0100 Subject: [PATCH 11/16] Migrated BodyParserMiddleware to psr-15 middleware --- module/Core/test/Action/PreviewActionTest.php | 12 ++- module/Core/test/Action/QrCodeActionTest.php | 8 +- .../Core/test/Action/RedirectActionTest.php | 5 +- .../Middleware/QrCodeCacheMiddlewareTest.php | 9 ++- .../src/Middleware/BodyParserMiddleware.php | 48 +++++------- .../Middleware/BodyParserMiddlewareTest.php | 76 ++++++++++++------- 6 files changed, 86 insertions(+), 72 deletions(-) diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index a5e57d2d..fa7a84c0 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -1,16 +1,15 @@ urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); + $delegate->process(Argument::cetera())->shouldBeCalledTimes(1); $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), $delegate->reveal() ); - - $delegate->process(Argument::cetera())->shouldHaveBeenCalledTimes(1); } /** @@ -75,12 +73,12 @@ class PreviewActionTest extends TestCase /** * @test */ - public function invalidShortcodeExceptionFallsBackToNextMiddleware() + public function invalidShortCodeExceptionFallsBackToNextMiddleware() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) ->shouldBeCalledTimes(1); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index dc005b11..ea71d855 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -1,6 +1,7 @@ urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), @@ -59,7 +59,7 @@ class QrCodeActionTest extends TestCase $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) ->shouldBeCalledTimes(1); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), @@ -76,7 +76,7 @@ class QrCodeActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(new ShortUrl())->shouldBeCalledTimes(1); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); $resp = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 65b6250f..6e629772 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -1,6 +1,7 @@ urlShortener->shortCodeToUrl($shortCode)->willReturn(null) ->shouldBeCalledTimes(1); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $this->action->process($request, $delegate->reveal()); @@ -73,7 +74,7 @@ class RedirectActionTest extends TestCase $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(\Exception::class) ->shouldBeCalledTimes(1); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $this->action->process($request, $delegate->reveal()); diff --git a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php index abc1801d..7e1ffc65 100644 --- a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php +++ b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php @@ -3,10 +3,10 @@ namespace ShlinkioTest\Shlink\Core\Middleware; use Doctrine\Common\Cache\ArrayCache; use Doctrine\Common\Cache\Cache; +use Interop\Http\ServerMiddleware\DelegateInterface; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Shlinkio\Shlink\Core\Middleware\QrCodeCacheMiddleware; -use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\Uri; @@ -33,13 +33,14 @@ class QrCodeCacheMiddlewareTest extends TestCase */ public function noCachedPathFallsBackToNextMiddleware() { - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); + $delegate->process(Argument::any())->willReturn(new Response())->shouldBeCalledTimes(1); + $this->middleware->process(ServerRequestFactory::fromGlobals()->withUri( new Uri('/foo/bar') ), $delegate->reveal()); $this->assertTrue($this->cache->contains('/foo/bar')); - $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1); } /** @@ -50,7 +51,7 @@ class QrCodeCacheMiddlewareTest extends TestCase $isCalled = false; $uri = (new Uri())->withPath('/foo'); $this->cache->save('/foo', ['body' => 'the body', 'content-type' => 'image/png']); - $delegate = TestUtils::createDelegateMock(); + $delegate = $this->prophesize(DelegateInterface::class); $resp = $this->middleware->process( ServerRequestFactory::fromGlobals()->withUri($uri), diff --git a/module/Rest/src/Middleware/BodyParserMiddleware.php b/module/Rest/src/Middleware/BodyParserMiddleware.php index 7344ccc4..b12c8ef1 100644 --- a/module/Rest/src/Middleware/BodyParserMiddleware.php +++ b/module/Rest/src/Middleware/BodyParserMiddleware.php @@ -1,55 +1,45 @@ getMethod(); $currentParams = $request->getParsedBody(); // In requests that do not allow body or if the body has already been parsed, continue to next middleware - if (in_array($method, ['GET', 'HEAD', 'OPTIONS']) || ! empty($currentParams)) { - return $out($request, $response); + if (! empty($currentParams) || in_array($method, [ + self::METHOD_GET, + self::METHOD_HEAD, + self::METHOD_OPTIONS + ], true)) { + return $delegate->process($request); } // If the accepted content is JSON, try to parse the body from JSON $contentType = $this->getRequestContentType($request); - if (in_array($contentType, ['application/json', 'text/json', 'application/x-json'])) { - return $out($this->parseFromJson($request), $response); + if (in_array($contentType, ['application/json', 'text/json', 'application/x-json'], true)) { + return $delegate->process($this->parseFromJson($request)); } - return $out($this->parseFromUrlEncoded($request), $response); + return $delegate->process($this->parseFromUrlEncoded($request)); } /** diff --git a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php index 67261b93..f42c99ef 100644 --- a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php +++ b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php @@ -1,8 +1,11 @@ withMethod('GET'); - $test = $this; - $this->middleware->__invoke($request, new Response(), function ($req, $resp) use ($test, $request) { - $test->assertSame($request, $req); - }); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); - $request = $request->withMethod('POST'); - $test = $this; - $this->middleware->__invoke($request, new Response(), function ($req, $resp) use ($test, $request) { - $test->assertSame($request, $req); - }); + $this->middleware->process($request, $delegate->reveal()); + + $process->shouldHaveBeenCalledTimes(1); } /** @@ -43,19 +43,31 @@ class BodyParserMiddlewareTest extends TestCase */ public function jsonRequestsAreJsonDecoded() { + $test = $this; $body = new Stream('php://temp', 'wr'); $body->write('{"foo": "bar", "bar": ["one", 5]}'); $request = ServerRequestFactory::fromGlobals()->withMethod('PUT') ->withBody($body) ->withHeader('content-type', 'application/json'); - $test = $this; - $this->middleware->__invoke($request, new Response(), function (Request $req, $resp) use ($test, $request) { - $test->assertNotSame($request, $req); - $test->assertEquals([ - 'foo' => 'bar', - 'bar' => ['one', 5], - ], $req->getParsedBody()); - }); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process(Argument::type(ServerRequestInterface::class))->will( + function (array $args) use ($test) { + /** @var ServerRequestInterface $req */ + $req = array_shift($args); + + $test->assertEquals([ + 'foo' => 'bar', + 'bar' => ['one', 5], + ], $req->getParsedBody()); + + return new Response(); + } + ); + + $this->middleware->process($request, $delegate->reveal()); + + $process->shouldHaveBeenCalledTimes(1); } /** @@ -63,17 +75,29 @@ class BodyParserMiddlewareTest extends TestCase */ public function regularRequestsAreUrlDecoded() { + $test = $this; $body = new Stream('php://temp', 'wr'); $body->write('foo=bar&bar[]=one&bar[]=5'); $request = ServerRequestFactory::fromGlobals()->withMethod('PUT') ->withBody($body); - $test = $this; - $this->middleware->__invoke($request, new Response(), function (Request $req, $resp) use ($test, $request) { - $test->assertNotSame($request, $req); - $test->assertEquals([ - 'foo' => 'bar', - 'bar' => ['one', 5], - ], $req->getParsedBody()); - }); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process(Argument::type(ServerRequestInterface::class))->will( + function (array $args) use ($test) { + /** @var ServerRequestInterface $req */ + $req = array_shift($args); + + $test->assertEquals([ + 'foo' => 'bar', + 'bar' => ['one', 5], + ], $req->getParsedBody()); + + return new Response(); + } + ); + + $this->middleware->process($request, $delegate->reveal()); + + $process->shouldHaveBeenCalledTimes(1); } } From 9bd18ee04154cff821a09d8966df36b2ef193fc3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2017 09:37:13 +0100 Subject: [PATCH 12/16] Migrated CheckAuthenticationMiddleware to psr-15 middleware --- .../CheckAuthenticationMiddleware.php | 45 ++++------- .../CheckAuthenticationMiddlewareTest.php | 78 +++++++++---------- 2 files changed, 53 insertions(+), 70 deletions(-) diff --git a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index 0e7c1cdb..0304a408 100644 --- a/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php +++ b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php @@ -2,6 +2,9 @@ namespace Shlinkio\Shlink\Rest\Middleware; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Fig\Http\Message\StatusCodeInterface; +use Interop\Http\ServerMiddleware\DelegateInterface; +use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -14,9 +17,8 @@ use Zend\Diactoros\Response\JsonResponse; use Zend\Expressive\Router\RouteResult; use Zend\I18n\Translator\TranslatorInterface; use Zend\Stdlib\ErrorHandler; -use Zend\Stratigility\MiddlewareInterface; -class CheckAuthenticationMiddleware implements MiddlewareInterface +class CheckAuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterface { const AUTHORIZATION_HEADER = 'Authorization'; @@ -52,31 +54,15 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface } /** - * Process an incoming request and/or response. - * - * Accepts a server-side request and a response instance, and does - * something with them. - * - * If the response is not complete and/or further processing would not - * interfere with the work done in the middleware, or if the middleware - * wants to delegate to another process, it can use the `$out` callable - * if present. - * - * If the middleware does not return a value, execution of the current - * request is considered complete, and the response instance provided will - * be considered the response to return. - * - * Alternately, the middleware may return a response instance. - * - * Often, middleware will `return $out();`, with the assumption that a - * later middleware will return a response. + * Process an incoming server request and return a response, optionally delegating + * to the next middleware component to create the response. * * @param Request $request - * @param Response $response - * @param null|callable $out - * @return null|Response + * @param DelegateInterface $delegate + * + * @return Response */ - public function __invoke(Request $request, Response $response, callable $out = null) + public function process(Request $request, DelegateInterface $delegate) { // If current route is the authenticate route or an OPTIONS request, continue to the next middleware /** @var RouteResult $routeResult */ @@ -86,7 +72,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface || $routeResult->getMatchedRouteName() === 'rest-authenticate' || $request->getMethod() === 'OPTIONS' ) { - return $out($request, $response); + return $delegate->process($request); } // Check that the auth header was provided, and that it belongs to a non-expired token @@ -103,7 +89,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface 'message' => sprintf($this->translator->translate( 'You need to provide the Bearer type in the %s header.' ), self::AUTHORIZATION_HEADER), - ], 401); + ], self::STATUS_UNAUTHORIZED); } // Make sure the authorization type is Bearer @@ -114,7 +100,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface 'message' => sprintf($this->translator->translate( 'Provided authorization type %s is not supported. Use Bearer instead.' ), $authType), - ], 401); + ], self::STATUS_UNAUTHORIZED); } try { @@ -126,8 +112,7 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface // Update the token expiration and continue to next middleware $jwt = $this->jwtService->refresh($jwt); - /** @var Response $response */ - $response = $out($request, $response); + $response = $delegate->process($request); // Return the response with the updated token on it return $response->withHeader(self::AUTHORIZATION_HEADER, 'Bearer ' . $jwt); @@ -150,6 +135,6 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface ), self::AUTHORIZATION_HEADER ), - ], 401); + ], self::STATUS_UNAUTHORIZED); } } diff --git a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php index 4f472726..65523f62 100644 --- a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php @@ -1,10 +1,13 @@ assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRouteFailure(['GET']) ); - $response = new Response(); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('foo', '', Route::HTTP_METHOD_ANY, 'rest-authenticate'), []) ); - $response = new Response(); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withMethod('OPTIONS'); - $response = new Response(); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, $response, function ($req, $resp) use (&$isCalled) { - $isCalled = true; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $this->middleware->process($request, $delegate->reveal()); + $process->shouldHaveBeenCalledTimes(1); } /** @@ -88,7 +84,7 @@ class CheckAuthenticationMiddlewareTest extends TestCase RouteResult::class, RouteResult::fromRoute(new Route('bar', 'foo'), []) ); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(401, $response->getStatusCode()); } @@ -103,7 +99,8 @@ class CheckAuthenticationMiddlewareTest extends TestCase RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $authToken); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); + $this->assertEquals(401, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), 'You need to provide the Bearer type') > 0); } @@ -119,7 +116,8 @@ class CheckAuthenticationMiddlewareTest extends TestCase RouteResult::fromRoute(new Route('bar', 'foo'), []) )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Basic ' . $authToken); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); + $this->assertEquals(401, $response->getStatusCode()); $this->assertTrue( strpos($response->getBody()->getContents(), 'Provided authorization type Basic is not supported') > 0 @@ -138,14 +136,14 @@ class CheckAuthenticationMiddlewareTest extends TestCase )->withHeader(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, 'Bearer ' . $authToken); $this->jwtService->verify($authToken)->willReturn(false)->shouldBeCalledTimes(1); - $response = $this->middleware->__invoke($request, new Response()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(401, $response->getStatusCode()); } /** * @test */ - public function provideCorrectTokenUpdatesExpirationAndFallbacksToNextMiddleware() + public function provideCorrectTokenUpdatesExpirationAndFallsBackToNextMiddleware() { $authToken = 'ABC-abc'; $request = ServerRequestFactory::fromGlobals()->withAttribute( @@ -155,12 +153,12 @@ class CheckAuthenticationMiddlewareTest extends TestCase $this->jwtService->verify($authToken)->willReturn(true)->shouldBeCalledTimes(1); $this->jwtService->refresh($authToken)->willReturn($authToken)->shouldBeCalledTimes(1); - $isCalled = false; - $this->assertFalse($isCalled); - $this->middleware->__invoke($request, new Response(), function ($req, $resp) use (&$isCalled) { - $isCalled = true; - return $resp; - }); - $this->assertTrue($isCalled); + $delegate = $this->prophesize(DelegateInterface::class); + /** @var MethodProphecy $process */ + $process = $delegate->process($request)->willReturn(new Response()); + $resp = $this->middleware->process($request, $delegate->reveal()); + + $process->shouldHaveBeenCalledTimes(1); + $this->assertArrayHasKey(CheckAuthenticationMiddleware::AUTHORIZATION_HEADER, $resp->getHeaders()); } } From cd47aae90234abaf1f8096d06450eadefbf5af27 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2017 09:44:34 +0100 Subject: [PATCH 13/16] Migrated CrossDomainMiddleware to psr-15 middleware --- .../src/Middleware/CrossDomainMiddleware.php | 39 ++++++------------- .../Middleware/CrossDomainMiddlewareTest.php | 34 ++++++++-------- 2 files changed, 30 insertions(+), 43 deletions(-) diff --git a/module/Rest/src/Middleware/CrossDomainMiddleware.php b/module/Rest/src/Middleware/CrossDomainMiddleware.php index d6b84d5b..0ce57e4f 100644 --- a/module/Rest/src/Middleware/CrossDomainMiddleware.php +++ b/module/Rest/src/Middleware/CrossDomainMiddleware.php @@ -1,41 +1,26 @@ process($request); if (! $request->hasHeader('Origin')) { return $response; } @@ -49,9 +34,9 @@ class CrossDomainMiddleware implements MiddlewareInterface // Add OPTIONS-specific headers foreach ([ - 'Access-Control-Allow-Methods' => 'GET,POST,PUT,DELETE,OPTIONS', // TODO Should be based on path - 'Access-Control-Max-Age' => '1000', - 'Access-Control-Allow-Headers' => $request->getHeaderLine('Access-Control-Request-Headers'), + 'Access-Control-Allow-Methods' => 'GET,POST,PUT,DELETE,OPTIONS', // TODO Should be based on path + 'Access-Control-Max-Age' => '1000', + 'Access-Control-Allow-Headers' => $request->getHeaderLine('Access-Control-Request-Headers'), ] as $key => $value) { $response = $response->withHeader($key, $value); } diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index 016cc04c..8ca260fd 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -1,7 +1,10 @@ middleware = new CrossDomainMiddleware(); + $this->delegate = $this->prophesize(DelegateInterface::class); } /** @@ -24,13 +32,9 @@ class CrossDomainMiddlewareTest extends TestCase public function nonCrossDomainRequestsAreNotAffected() { $originalResponse = new Response(); - $response = $this->middleware->__invoke( - ServerRequestFactory::fromGlobals(), - $originalResponse, - function ($req, $resp) { - return $resp; - } - ); + $this->delegate->process(Argument::any())->willReturn($originalResponse)->shouldbeCalledTimes(1); + + $response = $this->middleware->process(ServerRequestFactory::fromGlobals(), $this->delegate->reveal()); $this->assertSame($originalResponse, $response); $headers = $response->getHeaders(); @@ -44,12 +48,11 @@ class CrossDomainMiddlewareTest extends TestCase public function anyRequestIncludesTheAllowAccessHeader() { $originalResponse = new Response(); - $response = $this->middleware->__invoke( + $this->delegate->process(Argument::any())->willReturn($originalResponse)->shouldbeCalledTimes(1); + + $response = $this->middleware->process( ServerRequestFactory::fromGlobals()->withHeader('Origin', 'local'), - $originalResponse, - function ($req, $resp) { - return $resp; - } + $this->delegate->reveal() ); $this->assertNotSame($originalResponse, $response); @@ -64,11 +67,10 @@ class CrossDomainMiddlewareTest extends TestCase public function optionsRequestIncludesMoreHeaders() { $originalResponse = new Response(); - $request = ServerRequestFactory::fromGlobals(['REQUEST_METHOD' => 'OPTIONS'])->withHeader('Origin', 'local'); + $request = ServerRequestFactory::fromGlobals()->withMethod('OPTIONS')->withHeader('Origin', 'local'); + $this->delegate->process(Argument::any())->willReturn($originalResponse)->shouldbeCalledTimes(1); - $response = $this->middleware->__invoke($request, $originalResponse, function ($req, $resp) { - return $resp; - }); + $response = $this->middleware->process($request, $this->delegate->reveal()); $this->assertNotSame($originalResponse, $response); $headers = $response->getHeaders(); From 288249d0b8047d60ea95879482110dc2cf4768ff Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2017 09:46:29 +0100 Subject: [PATCH 14/16] Renamed JsonErrorHandler to JsonErrorResponseGenerator --- module/Rest/config/error-handler.config.php | 4 ++-- ...sonErrorHandler.php => JsonErrorResponseGenerator.php} | 2 +- ...HandlerTest.php => JsonErrorResponseGeneratorTest.php} | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) rename module/Rest/src/ErrorHandler/{JsonErrorHandler.php => JsonErrorResponseGenerator.php} (92%) rename module/Rest/test/ErrorHandler/{JsonErrorHandlerTest.php => JsonErrorResponseGeneratorTest.php} (81%) diff --git a/module/Rest/config/error-handler.config.php b/module/Rest/config/error-handler.config.php index fef71303..cb06f6bf 100644 --- a/module/Rest/config/error-handler.config.php +++ b/module/Rest/config/error-handler.config.php @@ -1,12 +1,12 @@ [ 'plugins' => [ 'invokables' => [ - 'application/json' => JsonErrorHandler::class, + 'application/json' => JsonErrorResponseGenerator::class, ], 'aliases' => [ 'application/x-json' => 'application/json', diff --git a/module/Rest/src/ErrorHandler/JsonErrorHandler.php b/module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php similarity index 92% rename from module/Rest/src/ErrorHandler/JsonErrorHandler.php rename to module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php index 3dc55e8b..0f26957c 100644 --- a/module/Rest/src/ErrorHandler/JsonErrorHandler.php +++ b/module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php @@ -8,7 +8,7 @@ use Psr\Http\Message\ServerRequestInterface as Request; use Zend\Diactoros\Response\JsonResponse; use Zend\Expressive\Router\RouteResult; -class JsonErrorHandler implements ErrorResponseGeneratorInterface, StatusCodeInterface +class JsonErrorResponseGenerator implements ErrorResponseGeneratorInterface, StatusCodeInterface { /** * Final handler for an application. diff --git a/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php b/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php similarity index 81% rename from module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php rename to module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php index 75d4ad0e..164ce6eb 100644 --- a/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php +++ b/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php @@ -2,20 +2,20 @@ namespace ShlinkioTest\Shlink\Rest\ErrorHandler; use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Rest\ErrorHandler\JsonErrorHandler; +use Shlinkio\Shlink\Rest\ErrorHandler\JsonErrorResponseGenerator; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; -class JsonErrorHandlerTest extends TestCase +class JsonErrorResponseGeneratorTest extends TestCase { /** - * @var JsonErrorHandler + * @var JsonErrorResponseGenerator */ protected $errorHandler; public function setUp() { - $this->errorHandler = new JsonErrorHandler(); + $this->errorHandler = new JsonErrorResponseGenerator(); } /** From 2e5a7d76dff758d40f04cbdee91fb21c724b14d7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2017 10:04:48 +0100 Subject: [PATCH 15/16] Migrated rest actions to psr-15 middleware --- module/Rest/src/Action/AbstractRestAction.php | 47 +++++++------------ module/Rest/src/Action/AuthenticateAction.php | 19 ++++---- .../Rest/src/Action/CreateShortcodeAction.php | 12 ++--- module/Rest/src/Action/EditTagsAction.php | 10 ++-- module/Rest/src/Action/GetVisitsAction.php | 10 ++-- .../Rest/src/Action/ListShortcodesAction.php | 8 ++-- module/Rest/src/Action/ResolveUrlAction.php | 12 ++--- .../test/Action/AuthenticateActionTest.php | 8 ++-- .../test/Action/CreateShortcodeActionTest.php | 13 +++-- .../Rest/test/Action/EditTagsActionTest.php | 14 +++--- .../Rest/test/Action/GetVisitsActionTest.php | 18 +++---- .../test/Action/ListShortcodesActionTest.php | 12 ++--- .../Rest/test/Action/ResolveUrlActionTest.php | 10 ++-- 13 files changed, 93 insertions(+), 100 deletions(-) diff --git a/module/Rest/src/Action/AbstractRestAction.php b/module/Rest/src/Action/AbstractRestAction.php index 4acbe2ba..c71c0176 100644 --- a/module/Rest/src/Action/AbstractRestAction.php +++ b/module/Rest/src/Action/AbstractRestAction.php @@ -1,13 +1,17 @@ getMethod() === 'OPTIONS') { - return $response; + if ($request->getMethod() === self::METHOD_OPTIONS) { + return new EmptyResponse(); } - return $this->dispatch($request, $response, $out); + return $this->dispatch($request, $delegate); } /** * @param Request $request - * @param Response $response - * @param callable|null $out + * @param DelegateInterface $delegate * @return null|Response */ - abstract protected function dispatch(Request $request, Response $response, callable $out = null); + abstract protected function dispatch(Request $request, DelegateInterface $delegate); } diff --git a/module/Rest/src/Action/AuthenticateAction.php b/module/Rest/src/Action/AuthenticateAction.php index a20e5964..27d67bc3 100644 --- a/module/Rest/src/Action/AuthenticateAction.php +++ b/module/Rest/src/Action/AuthenticateAction.php @@ -2,9 +2,10 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; -use Firebase\JWT\JWT; +use Interop\Http\ServerMiddleware\DelegateInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Rest\Authentication\JWTService; use Shlinkio\Shlink\Rest\Authentication\JWTServiceInterface; use Shlinkio\Shlink\Rest\Service\ApiKeyService; @@ -33,14 +34,17 @@ class AuthenticateAction extends AbstractRestAction * @param ApiKeyServiceInterface|ApiKeyService $apiKeyService * @param JWTServiceInterface|JWTService $jwtService * @param TranslatorInterface $translator + * @param LoggerInterface|null $logger * - * @Inject({ApiKeyService::class, JWTService::class, "translator"}) + * @Inject({ApiKeyService::class, JWTService::class, "translator", "Logger_Shlink"}) */ public function __construct( ApiKeyServiceInterface $apiKeyService, JWTServiceInterface $jwtService, - TranslatorInterface $translator + TranslatorInterface $translator, + LoggerInterface $logger = null ) { + parent::__construct($logger); $this->translator = $translator; $this->apiKeyService = $apiKeyService; $this->jwtService = $jwtService; @@ -48,11 +52,10 @@ class AuthenticateAction extends AbstractRestAction /** * @param Request $request - * @param Response $response - * @param callable|null $out + * @param DelegateInterface $delegate * @return null|Response */ - public function dispatch(Request $request, Response $response, callable $out = null) + public function dispatch(Request $request, DelegateInterface $delegate) { $authData = $request->getParsedBody(); if (! isset($authData['apiKey'])) { @@ -61,7 +64,7 @@ class AuthenticateAction extends AbstractRestAction 'message' => $this->translator->translate( 'You have to provide a valid API key under the "apiKey" param name.' ), - ], 400); + ], self::STATUS_BAD_REQUEST); } // Authenticate using provided API key @@ -70,7 +73,7 @@ class AuthenticateAction extends AbstractRestAction return new JsonResponse([ 'error' => RestUtils::INVALID_API_KEY_ERROR, 'message' => $this->translator->translate('Provided API key does not exist or is invalid.'), - ], 401); + ], self::STATUS_UNAUTHORIZED); } // Generate a JSON Web Token that will be used for authorization in next requests diff --git a/module/Rest/src/Action/CreateShortcodeAction.php b/module/Rest/src/Action/CreateShortcodeAction.php index fdecedef..e63db63a 100644 --- a/module/Rest/src/Action/CreateShortcodeAction.php +++ b/module/Rest/src/Action/CreateShortcodeAction.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -52,18 +53,17 @@ class CreateShortcodeAction extends AbstractRestAction /** * @param Request $request - * @param Response $response - * @param callable|null $out + * @param DelegateInterface $delegate * @return null|Response */ - public function dispatch(Request $request, Response $response, callable $out = null) + public function dispatch(Request $request, DelegateInterface $delegate) { $postData = $request->getParsedBody(); if (! isset($postData['longUrl'])) { return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, 'message' => $this->translator->translate('A URL was not provided'), - ], 400); + ], self::STATUS_BAD_REQUEST); } $longUrl = $postData['longUrl']; $tags = isset($postData['tags']) && is_array($postData['tags']) ? $postData['tags'] : []; @@ -87,13 +87,13 @@ class CreateShortcodeAction extends AbstractRestAction $this->translator->translate('Provided URL %s is invalid. Try with a different one.'), $longUrl ), - ], 400); + ], self::STATUS_BAD_REQUEST); } catch (\Exception $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'), - ], 500); + ], self::STATUS_INTERNAL_SERVER_ERROR); } } } diff --git a/module/Rest/src/Action/EditTagsAction.php b/module/Rest/src/Action/EditTagsAction.php index 3dd76333..a61e940a 100644 --- a/module/Rest/src/Action/EditTagsAction.php +++ b/module/Rest/src/Action/EditTagsAction.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -43,11 +44,10 @@ class EditTagsAction extends AbstractRestAction /** * @param Request $request - * @param Response $response - * @param callable|null $out + * @param DelegateInterface $delegate * @return null|Response */ - protected function dispatch(Request $request, Response $response, callable $out = null) + protected function dispatch(Request $request, DelegateInterface $delegate) { $shortCode = $request->getAttribute('shortCode'); $bodyParams = $request->getParsedBody(); @@ -56,7 +56,7 @@ class EditTagsAction extends AbstractRestAction return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, 'message' => $this->translator->translate('A list of tags was not provided'), - ], 400); + ], self::STATUS_BAD_REQUEST); } $tags = $bodyParams['tags']; @@ -67,7 +67,7 @@ class EditTagsAction extends AbstractRestAction return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), 'message' => sprintf($this->translator->translate('No URL found for short code "%s"'), $shortCode), - ], 404); + ], self::STATUS_NOT_FOUND); } } } diff --git a/module/Rest/src/Action/GetVisitsAction.php b/module/Rest/src/Action/GetVisitsAction.php index 0d78bcc1..c5e6b71b 100644 --- a/module/Rest/src/Action/GetVisitsAction.php +++ b/module/Rest/src/Action/GetVisitsAction.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -44,11 +45,10 @@ class GetVisitsAction extends AbstractRestAction /** * @param Request $request - * @param Response $response - * @param callable|null $out + * @param DelegateInterface $delegate * @return null|Response */ - public function dispatch(Request $request, Response $response, callable $out = null) + public function dispatch(Request $request, DelegateInterface $delegate) { $shortCode = $request->getAttribute('shortCode'); $startDate = $this->getDateQueryParam($request, 'startDate'); @@ -70,13 +70,13 @@ class GetVisitsAction extends AbstractRestAction $this->translator->translate('Provided short code %s does not exist'), $shortCode ), - ], 404); + ], self::STATUS_NOT_FOUND); } catch (\Exception $e) { $this->logger->error('Unexpected error while parsing short code'. PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, 'message' => $this->translator->translate('Unexpected error occurred'), - ], 500); + ], self::STATUS_INTERNAL_SERVER_ERROR); } } diff --git a/module/Rest/src/Action/ListShortcodesAction.php b/module/Rest/src/Action/ListShortcodesAction.php index aa987c2f..b7099c1c 100644 --- a/module/Rest/src/Action/ListShortcodesAction.php +++ b/module/Rest/src/Action/ListShortcodesAction.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -46,11 +47,10 @@ class ListShortcodesAction extends AbstractRestAction /** * @param Request $request - * @param Response $response - * @param callable|null $out + * @param DelegateInterface $delegate * @return null|Response */ - public function dispatch(Request $request, Response $response, callable $out = null) + public function dispatch(Request $request, DelegateInterface $delegate) { try { $params = $this->queryToListParams($request->getQueryParams()); @@ -61,7 +61,7 @@ class ListShortcodesAction extends AbstractRestAction return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, 'message' => $this->translator->translate('Unexpected error occurred'), - ], 500); + ], self::STATUS_INTERNAL_SERVER_ERROR); } } diff --git a/module/Rest/src/Action/ResolveUrlAction.php b/module/Rest/src/Action/ResolveUrlAction.php index c99e233a..b705814a 100644 --- a/module/Rest/src/Action/ResolveUrlAction.php +++ b/module/Rest/src/Action/ResolveUrlAction.php @@ -2,6 +2,7 @@ namespace Shlinkio\Shlink\Rest\Action; use Acelaya\ZsmAnnotatedServices\Annotation\Inject; +use Interop\Http\ServerMiddleware\DelegateInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; @@ -43,11 +44,10 @@ class ResolveUrlAction extends AbstractRestAction /** * @param Request $request - * @param Response $response - * @param callable|null $out + * @param DelegateInterface $delegate * @return null|Response */ - public function dispatch(Request $request, Response $response, callable $out = null) + public function dispatch(Request $request, DelegateInterface $delegate) { $shortCode = $request->getAttribute('shortCode'); @@ -57,7 +57,7 @@ class ResolveUrlAction extends AbstractRestAction return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, 'message' => sprintf($this->translator->translate('No URL found for short code "%s"'), $shortCode), - ], 404); + ], self::STATUS_NOT_FOUND); } return new JsonResponse([ @@ -71,13 +71,13 @@ class ResolveUrlAction extends AbstractRestAction $this->translator->translate('Provided short code "%s" has an invalid format'), $shortCode ), - ], 400); + ], self::STATUS_BAD_REQUEST); } catch (\Exception $e) { $this->logger->error('Unexpected error while resolving the URL behind a short code.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::UNKNOWN_ERROR, 'message' => $this->translator->translate('Unexpected error occurred'), - ], 500); + ], self::STATUS_INTERNAL_SERVER_ERROR); } } } diff --git a/module/Rest/test/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index 13167073..2a929495 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -7,7 +7,7 @@ use Shlinkio\Shlink\Rest\Action\AuthenticateAction; use Shlinkio\Shlink\Rest\Authentication\JWTService; use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyService; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; @@ -42,7 +42,7 @@ class AuthenticateActionTest extends TestCase */ public function notProvidingAuthDataReturnsError() { - $resp = $this->action->__invoke(ServerRequestFactory::fromGlobals(), new Response()); + $resp = $this->action->process(ServerRequestFactory::fromGlobals(), TestUtils::createDelegateMock()->reveal()); $this->assertEquals(400, $resp->getStatusCode()); } @@ -57,7 +57,7 @@ class AuthenticateActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'apiKey' => 'foo', ]); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(200, $response->getStatusCode()); $response->getBody()->rewind(); @@ -75,7 +75,7 @@ class AuthenticateActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'apiKey' => 'foo', ]); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(401, $response->getStatusCode()); } } diff --git a/module/Rest/test/Action/CreateShortcodeActionTest.php b/module/Rest/test/Action/CreateShortcodeActionTest.php index 4593fced..3b2eec10 100644 --- a/module/Rest/test/Action/CreateShortcodeActionTest.php +++ b/module/Rest/test/Action/CreateShortcodeActionTest.php @@ -8,7 +8,7 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\CreateShortcodeAction; use Shlinkio\Shlink\Rest\Util\RestUtils; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\Diactoros\Uri; use Zend\I18n\Translator\Translator; @@ -38,7 +38,10 @@ class CreateShortcodeActionTest extends TestCase */ public function missingLongUrlParamReturnsError() { - $response = $this->action->__invoke(ServerRequestFactory::fromGlobals(), new Response()); + $response = $this->action->process( + ServerRequestFactory::fromGlobals(), + TestUtils::createDelegateMock()->reveal() + ); $this->assertEquals(400, $response->getStatusCode()); } @@ -54,7 +57,7 @@ class CreateShortcodeActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', ]); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(200, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), 'http://foo.com/abc123') > 0); } @@ -71,7 +74,7 @@ class CreateShortcodeActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', ]); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(400, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_URL_ERROR) > 0); } @@ -88,7 +91,7 @@ class CreateShortcodeActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', ]); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(500, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::UNKNOWN_ERROR) > 0); } diff --git a/module/Rest/test/Action/EditTagsActionTest.php b/module/Rest/test/Action/EditTagsActionTest.php index da3082f4..518925ab 100644 --- a/module/Rest/test/Action/EditTagsActionTest.php +++ b/module/Rest/test/Action/EditTagsActionTest.php @@ -7,7 +7,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\EditTagsAction; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; @@ -33,9 +33,9 @@ class EditTagsActionTest extends TestCase */ public function notProvidingTagsReturnsError() { - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', 'abc123'), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(400, $response->getStatusCode()); } @@ -49,10 +49,10 @@ class EditTagsActionTest extends TestCase $this->shortUrlService->setTagsByShortCode($shortCode, [])->willThrow(InvalidShortCodeException::class) ->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', 'abc123') ->withParsedBody(['tags' => []]), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(404, $response->getStatusCode()); } @@ -66,10 +66,10 @@ class EditTagsActionTest extends TestCase $this->shortUrlService->setTagsByShortCode($shortCode, [])->willReturn(new ShortUrl()) ->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', 'abc123') ->withParsedBody(['tags' => []]), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(200, $response->getStatusCode()); } diff --git a/module/Rest/test/Action/GetVisitsActionTest.php b/module/Rest/test/Action/GetVisitsActionTest.php index 152e5f48..ee356d1e 100644 --- a/module/Rest/test/Action/GetVisitsActionTest.php +++ b/module/Rest/test/Action/GetVisitsActionTest.php @@ -8,7 +8,7 @@ use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Service\VisitsTracker; use Shlinkio\Shlink\Rest\Action\GetVisitsAction; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; @@ -38,9 +38,9 @@ class GetVisitsActionTest extends TestCase $this->visitsTracker->info($shortCode, Argument::type(DateRange::class))->willReturn([]) ->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(200, $response->getStatusCode()); } @@ -55,9 +55,9 @@ class GetVisitsActionTest extends TestCase InvalidArgumentException::class )->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(404, $response->getStatusCode()); } @@ -72,9 +72,9 @@ class GetVisitsActionTest extends TestCase \Exception::class )->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(500, $response->getStatusCode()); } @@ -89,10 +89,10 @@ class GetVisitsActionTest extends TestCase ->willReturn([]) ->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode) ->withQueryParams(['endDate' => '2016-01-01 00:00:00']), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(200, $response->getStatusCode()); } diff --git a/module/Rest/test/Action/ListShortcodesActionTest.php b/module/Rest/test/Action/ListShortcodesActionTest.php index 7c318f47..1fee9b28 100644 --- a/module/Rest/test/Action/ListShortcodesActionTest.php +++ b/module/Rest/test/Action/ListShortcodesActionTest.php @@ -5,13 +5,13 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ListShortcodesAction; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; use Zend\Paginator\Adapter\ArrayAdapter; use Zend\Paginator\Paginator; -class ListShortcodesActionTest extends TestCase +class ListShortCodesActionTest extends TestCase { /** * @var ListShortcodesAction @@ -37,11 +37,11 @@ class ListShortcodesActionTest extends TestCase $this->service->listShortUrls($page, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) ->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withQueryParams([ 'page' => $page, ]), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(200, $response->getStatusCode()); } @@ -55,11 +55,11 @@ class ListShortcodesActionTest extends TestCase $this->service->listShortUrls($page, null, [], null)->willThrow(\Exception::class) ->shouldBeCalledTimes(1); - $response = $this->action->__invoke( + $response = $this->action->process( ServerRequestFactory::fromGlobals()->withQueryParams([ 'page' => $page, ]), - new Response() + TestUtils::createDelegateMock()->reveal() ); $this->assertEquals(500, $response->getStatusCode()); } diff --git a/module/Rest/test/Action/ResolveUrlActionTest.php b/module/Rest/test/Action/ResolveUrlActionTest.php index 4631eef1..5b6a8355 100644 --- a/module/Rest/test/Action/ResolveUrlActionTest.php +++ b/module/Rest/test/Action/ResolveUrlActionTest.php @@ -7,7 +7,7 @@ use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\ResolveUrlAction; use Shlinkio\Shlink\Rest\Util\RestUtils; -use Zend\Diactoros\Response; +use ShlinkioTest\Shlink\Common\Util\TestUtils; use Zend\Diactoros\ServerRequestFactory; use Zend\I18n\Translator\Translator; @@ -38,7 +38,7 @@ class ResolveUrlActionTest extends TestCase ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(404, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_ARGUMENT_ERROR) > 0); } @@ -53,7 +53,7 @@ class ResolveUrlActionTest extends TestCase ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(200, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), 'http://domain.com/foo/bar') > 0); } @@ -68,7 +68,7 @@ class ResolveUrlActionTest extends TestCase ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(400, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::INVALID_SHORTCODE_ERROR) > 0); } @@ -83,7 +83,7 @@ class ResolveUrlActionTest extends TestCase ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); - $response = $this->action->__invoke($request, new Response()); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(500, $response->getStatusCode()); $this->assertTrue(strpos($response->getBody()->getContents(), RestUtils::UNKNOWN_ERROR) > 0); } From a2c4eebec882d351e3b11638744165a0ddb88ab4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2017 10:09:30 +0100 Subject: [PATCH 16/16] Updated CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e29d3607..96fca031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ ## CHANGELOG +### 1.4.0 + +**Enhancements:** + +* [89: Update to expressive 2](https://github.com/shlinkio/shlink/issues/89) + ### 1.3.1 **Tasks**