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** diff --git a/composer.json b/composer.json index f012cab4..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", - "mtymek/expressive-config-manager": "^0.4", - "acelaya/zsm-annotated-services": "^0.2.0", - "acelaya/ze-content-based-error-handler": "^1.0", + "zendframework/zend-config-aggregator": "^0.1", + "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", @@ -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/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/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/data/proxies/.gitignore b/data/proxies/.gitignore old mode 100644 new mode 100755 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 @@ [ - 'pre-routing' => [ - 'middleware' => [ - Middleware\LocaleMiddleware::class, - ], - 'priority' => 5, - ], - ], - -]; 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/ConfigProviderTest.php b/module/Common/test/ConfigProviderTest.php index 864ab648..05b5da49 100644 --- a/module/Common/test/ConfigProviderTest.php +++ b/module/Common/test/ConfigProviderTest.php @@ -1,7 +1,7 @@ configProvider->__invoke(); - $this->assertArrayHasKey('middleware_pipeline', $config); $this->assertArrayHasKey('dependencies', $config); $this->assertArrayHasKey('twig', $config); } diff --git a/module/Common/test/Factory/CacheFactoryTest.php b/module/Common/test/Factory/CacheFactoryTest.php index 0d50aab6..9a095ad6 100644 --- a/module/Common/test/Factory/CacheFactoryTest.php +++ b/module/Common/test/Factory/CacheFactoryTest.php @@ -6,7 +6,7 @@ use Doctrine\Common\Cache\ArrayCache; use Doctrine\Common\Cache\FilesystemCache; use Doctrine\Common\Cache\MemcachedCache; use Doctrine\Common\Cache\RedisCache; -use PHPUnit_Framework_TestCase as TestCase; +use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Factory\CacheFactory; use Shlinkio\Shlink\Core\Options\AppOptions; use Zend\ServiceManager\ServiceManager; diff --git a/module/Common/test/Factory/EntityManagerFactoryTest.php b/module/Common/test/Factory/EntityManagerFactoryTest.php index 2bad3c38..6a9b5c01 100644 --- a/module/Common/test/Factory/EntityManagerFactoryTest.php +++ b/module/Common/test/Factory/EntityManagerFactoryTest.php @@ -2,7 +2,7 @@ namespace ShlinkioTest\Shlink\Common\Factory; use Doctrine\ORM\EntityManager; -use PHPUnit_Framework_TestCase as TestCase; +use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Factory\EntityManagerFactory; use Zend\ServiceManager\ServiceManager; diff --git a/module/Common/test/Factory/LoggerFactoryTest.php b/module/Common/test/Factory/LoggerFactoryTest.php index bf292a1f..9c6acab5 100644 --- a/module/Common/test/Factory/LoggerFactoryTest.php +++ b/module/Common/test/Factory/LoggerFactoryTest.php @@ -2,7 +2,7 @@ namespace ShlinkioTest\Shlink\Common\Factory; use Monolog\Logger; -use PHPUnit_Framework_TestCase as TestCase; +use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\Factory\LoggerFactory; use Zend\ServiceManager\ServiceManager; diff --git a/module/Common/test/Factory/TranslatorFactoryTest.php b/module/Common/test/Factory/TranslatorFactoryTest.php index e70f820b..b227685b 100644 --- a/module/Common/test/Factory/TranslatorFactoryTest.php +++ b/module/Common/test/Factory/TranslatorFactoryTest.php @@ -1,7 +1,7 @@ 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/Paginator/PaginableRepositoryAdapterTest.php b/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php index 196cf0c9..99a25bcc 100644 --- a/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php +++ b/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php @@ -1,7 +1,7 @@ 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; + } +} diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index 4c21501c..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,46 +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'); - } catch (PreviewGenerationException $e) { - return $out($request, $response->withStatus(500), 'Preview generation error'); + return $delegate->process($request); } } } 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/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/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/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index 1ef49307..fa7a84c0 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -1,14 +1,15 @@ urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); + $delegate = $this->prophesize(DelegateInterface::class); + $delegate->process(Argument::cetera())->shouldBeCalledTimes(1); - $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()); } /** @@ -63,9 +61,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,40 +73,18 @@ class PreviewActionTest extends TestCase /** * @test */ - public function invalidShortcodeExceptionReturnsNotFound() + public function invalidShortCodeExceptionFallsBackToNextMiddleware() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) ->shouldBeCalledTimes(1); + $delegate = $this->prophesize(DelegateInterface::class); - $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()); - } - - /** - * @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()); + $delegate->process(Argument::any())->shouldHaveBeenCalledTimes(1); } } diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index d4924724..ea71d855 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -1,7 +1,8 @@ urlShortener->shortCodeToUrl($shortCode)->willReturn(null)->shouldBeCalledTimes(1); + $delegate = $this->prophesize(DelegateInterface::class); - $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 = $this->prophesize(DelegateInterface::class); - $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 = $this->prophesize(DelegateInterface::class); - $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); } } diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 434189fc..6e629772 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -1,14 +1,14 @@ 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 +53,32 @@ class RedirectActionTest extends TestCase /** * @test */ - public function nextErrorMiddlewareIsInvokedIfLongUrlIsNotFound() + public function nextMiddlewareIsInvokedIfLongUrlIsNotFound() { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(null) ->shouldBeCalledTimes(1); + $delegate = $this->prophesize(DelegateInterface::class); $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 = $this->prophesize(DelegateInterface::class); $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); } } diff --git a/module/Core/test/ConfigProviderTest.php b/module/Core/test/ConfigProviderTest.php index 3af8a48a..ce9dea72 100644 --- a/module/Core/test/ConfigProviderTest.php +++ b/module/Core/test/ConfigProviderTest.php @@ -1,7 +1,7 @@ middleware->__invoke( - ServerRequestFactory::fromGlobals(), - new Response(), - function ($req, $resp) use (&$isCalled) { - $isCalled = true; - return $resp; - } - ); - $this->assertTrue($isCalled); + $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')); } /** @@ -51,19 +51,17 @@ class QrCodeCacheMiddlewareTest extends TestCase $isCalled = false; $uri = (new Uri())->withPath('/foo'); $this->cache->save('/foo', ['body' => 'the body', 'content-type' => 'image/png']); + $delegate = $this->prophesize(DelegateInterface::class); - $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); } } diff --git a/module/Core/test/Options/AppOptionsFactoryTest.php b/module/Core/test/Options/AppOptionsFactoryTest.php index a3d9ac1a..7d1b503b 100644 --- a/module/Core/test/Options/AppOptionsFactoryTest.php +++ b/module/Core/test/Options/AppOptionsFactoryTest.php @@ -1,7 +1,7 @@ [ 'plugins' => [ 'invokables' => [ - 'application/json' => JsonErrorHandler::class, + 'application/json' => JsonErrorResponseGenerator::class, ], 'aliases' => [ 'application/x-json' => 'application/json', 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/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/src/ErrorHandler/JsonErrorHandler.php b/module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php similarity index 53% rename from module/Rest/src/ErrorHandler/JsonErrorHandler.php rename to module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.php index 79667c73..0f26957c 100644 --- a/module/Rest/src/ErrorHandler/JsonErrorHandler.php +++ b/module/Rest/src/ErrorHandler/JsonErrorResponseGenerator.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/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/src/Middleware/CheckAuthenticationMiddleware.php b/module/Rest/src/Middleware/CheckAuthenticationMiddleware.php index 5b18ea31..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,20 +112,13 @@ 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); } 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(); } @@ -156,6 +135,6 @@ class CheckAuthenticationMiddleware implements MiddlewareInterface ), self::AUTHORIZATION_HEADER ), - ], 401); + ], self::STATUS_UNAUTHORIZED); } } 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/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index 240b60a6..2a929495 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -1,13 +1,13 @@ 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 c3b9e3ff..3b2eec10 100644 --- a/module/Rest/test/Action/CreateShortcodeActionTest.php +++ b/module/Rest/test/Action/CreateShortcodeActionTest.php @@ -1,14 +1,14 @@ 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 b0813519..518925ab 100644 --- a/module/Rest/test/Action/EditTagsActionTest.php +++ b/module/Rest/test/Action/EditTagsActionTest.php @@ -1,13 +1,13 @@ 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 7e222269..ee356d1e 100644 --- a/module/Rest/test/Action/GetVisitsActionTest.php +++ b/module/Rest/test/Action/GetVisitsActionTest.php @@ -1,14 +1,14 @@ 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 d17740be..1fee9b28 100644 --- a/module/Rest/test/Action/ListShortcodesActionTest.php +++ b/module/Rest/test/Action/ListShortcodesActionTest.php @@ -1,17 +1,17 @@ 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 07db655a..5b6a8355 100644 --- a/module/Rest/test/Action/ResolveUrlActionTest.php +++ b/module/Rest/test/Action/ResolveUrlActionTest.php @@ -1,13 +1,13 @@ 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); } diff --git a/module/Rest/test/Authentication/JWTServiceTest.php b/module/Rest/test/Authentication/JWTServiceTest.php index ede0b6c6..66f90f25 100644 --- a/module/Rest/test/Authentication/JWTServiceTest.php +++ b/module/Rest/test/Authentication/JWTServiceTest.php @@ -2,7 +2,7 @@ namespace ShlinkioTest\Shlink\Rest\Authentication; use Firebase\JWT\JWT; -use PHPUnit_Framework_TestCase as TestCase; +use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Rest\Authentication\JWTService; use Shlinkio\Shlink\Rest\Entity\ApiKey; diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 0270183f..a512e31a 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -1,7 +1,7 @@ configProvider->__invoke(); $this->assertArrayHasKey('error_handler', $config); - $this->assertArrayHasKey('middleware_pipeline', $config); $this->assertArrayHasKey('routes', $config); $this->assertArrayHasKey('dependencies', $config); $this->assertArrayHasKey('translator', $config); diff --git a/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php b/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php deleted file mode 100644 index ea1eee80..00000000 --- a/module/Rest/test/ErrorHandler/JsonErrorHandlerTest.php +++ /dev/null @@ -1,79 +0,0 @@ -errorHandler = new JsonErrorHandler(); - } - - /** - * @test - */ - public function noMatchedRouteReturnsNotFoundResponse() - { - $response = $this->errorHandler->__invoke(ServerRequestFactory::fromGlobals(), new Response()); - $this->assertInstanceOf(Response\JsonResponse::class, $response); - $this->assertEquals(404, $response->getStatusCode()); - } - - /** - * @test - */ - public function matchedRouteWithErrorReturnsMethodNotAllowedResponse() - { - $response = $this->errorHandler->__invoke( - ServerRequestFactory::fromGlobals(), - (new Response())->withStatus(405), - 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/ErrorHandler/JsonErrorResponseGeneratorTest.php b/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php new file mode 100644 index 00000000..164ce6eb --- /dev/null +++ b/module/Rest/test/ErrorHandler/JsonErrorResponseGeneratorTest.php @@ -0,0 +1,44 @@ +errorHandler = new JsonErrorResponseGenerator(); + } + + /** + * @test + */ + public function noErrorStatusReturnsInternalServerError() + { + $response = $this->errorHandler->__invoke(null, ServerRequestFactory::fromGlobals(), new Response()); + $this->assertInstanceOf(Response\JsonResponse::class, $response); + $this->assertEquals(500, $response->getStatusCode()); + } + + /** + * @test + */ + public function errorStatusReturnsThatStatus() + { + $response = $this->errorHandler->__invoke( + null, + ServerRequestFactory::fromGlobals(), + (new Response())->withStatus(405) + ); + $this->assertInstanceOf(Response\JsonResponse::class, $response); + $this->assertEquals(405, $response->getStatusCode()); + } +} diff --git a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php index 17c3057b..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); } } diff --git a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php index c36c0d02..65523f62 100644 --- a/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/CheckAuthenticationMiddlewareTest.php @@ -1,12 +1,16 @@ 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::fromRouteMatch('rest-authenticate', 'foo', []) + 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::fromRouteMatch('bar', 'foo', []) + 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); } /** @@ -85,9 +82,9 @@ 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()); + $response = $this->middleware->process($request, TestUtils::createDelegateMock()->reveal()); $this->assertEquals(401, $response->getStatusCode()); } @@ -99,10 +96,11 @@ 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()); + $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); } @@ -115,10 +113,11 @@ 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()); + $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 @@ -133,33 +132,33 @@ 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); - $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( 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); - $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()); } } diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index 780de834..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(); diff --git a/module/Rest/test/Middleware/PathVersionMiddlewareTest.php b/module/Rest/test/Middleware/PathVersionMiddlewareTest.php index 25dc629f..7957e450 100644 --- a/module/Rest/test/Middleware/PathVersionMiddlewareTest.php +++ b/module/Rest/test/Middleware/PathVersionMiddlewareTest.php @@ -1,7 +1,7 @@