From d93d7978d4874bc36d9ceb1030ac55546060f076 Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 29 Mar 2023 10:13:38 +0100 Subject: [PATCH 1/6] Add PSR-17 Support Replaces all callable parameters and properties with `StreamFactoryInterface` and `ResponseFactoryInterface` where appropriate and removes the callable compatability layer. Closes #29 Signed-off-by: George Steel --- psalm-baseline.xml | 14 +-- src/Middleware/ImplicitHeadMiddleware.php | 22 +--- .../ImplicitHeadMiddlewareFactory.php | 13 +-- src/Middleware/ImplicitOptionsMiddleware.php | 22 +--- .../ImplicitOptionsMiddlewareFactory.php | 19 +-- src/Middleware/MethodNotAllowedMiddleware.php | 20 +--- .../MethodNotAllowedMiddlewareFactory.php | 17 ++- src/Middleware/Psr17ResponseFactoryTrait.php | 81 ------------- .../CallableResponseFactoryDecorator.php | 36 ------ ...AbstractImplicitMethodsIntegrationTest.php | 27 ++--- src/Test/ImplicitMethodsIntegrationTest.php | 14 --- src/Test/ResponseFactory.php | 24 ++++ .../ImplicitHeadMiddlewareFactoryTest.php | 12 +- .../Middleware/ImplicitHeadMiddlewareTest.php | 4 +- .../ImplicitOptionsMiddlewareFactoryTest.php | 20 +--- .../ImplicitOptionsMiddlewareTest.php | 41 ++----- .../MethodNotAllowedMiddlewareFactoryTest.php | 110 +++--------------- .../MethodNotAllowedMiddlewareTest.php | 10 +- .../CallableResponseFactoryDecoratorTest.php | 48 -------- test/ServiceManagerIntegrationTest.php | 9 +- 20 files changed, 124 insertions(+), 439 deletions(-) delete mode 100644 src/Middleware/Psr17ResponseFactoryTrait.php delete mode 100644 src/Response/CallableResponseFactoryDecorator.php delete mode 100644 src/Test/ImplicitMethodsIntegrationTest.php create mode 100644 src/Test/ResponseFactory.php delete mode 100644 test/Response/CallableResponseFactoryDecoratorTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index cb8cfab..4266fab 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,18 +1,8 @@ - - - - $container->get(StreamInterface::class) - - + - $this->services[$id] + services[$id]]]> - - - new CallableResponseFactoryDecorator(fn (): ResponseInterface => $this->response) - - diff --git a/src/Middleware/ImplicitHeadMiddleware.php b/src/Middleware/ImplicitHeadMiddleware.php index f82bdf3..4b09bf7 100644 --- a/src/Middleware/ImplicitHeadMiddleware.php +++ b/src/Middleware/ImplicitHeadMiddleware.php @@ -9,7 +9,7 @@ use Mezzio\Router\RouterInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Message\StreamInterface; +use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; @@ -44,19 +44,10 @@ class ImplicitHeadMiddleware implements MiddlewareInterface { public const FORWARDED_HTTP_METHOD_ATTRIBUTE = 'forwarded_http_method'; - /** @var callable(): StreamInterface */ - private $streamFactory; - - /** - * @param callable(): StreamInterface $streamFactory A factory capable of returning an empty - * StreamInterface instance to inject in a response. - */ - public function __construct(private RouterInterface $router, callable $streamFactory) - { - // Factory is wrapped in closure in order to enforce return type safety. - $this->streamFactory = function () use ($streamFactory): StreamInterface { - return $streamFactory(); - }; + public function __construct( + private readonly RouterInterface $router, + private readonly StreamFactoryInterface $streamFactory, + ) { } /** @@ -99,7 +90,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface ->withMethod(RequestMethod::METHOD_GET) ); - $body = ($this->streamFactory)(); - return $response->withBody($body); + return $response->withBody($this->streamFactory->createStream()); } } diff --git a/src/Middleware/ImplicitHeadMiddlewareFactory.php b/src/Middleware/ImplicitHeadMiddlewareFactory.php index ec88dc6..941b687 100644 --- a/src/Middleware/ImplicitHeadMiddlewareFactory.php +++ b/src/Middleware/ImplicitHeadMiddlewareFactory.php @@ -7,7 +7,7 @@ use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\RouterInterface; use Psr\Container\ContainerInterface; -use Psr\Http\Message\StreamInterface; +use Psr\Http\Message\StreamFactoryInterface; /** * Create and return an ImplicitHeadMiddleware instance. @@ -16,7 +16,7 @@ * * - Mezzio\Router\RouterInterface, which should resolve to an * instance of that interface. - * - Psr\Http\Message\StreamInterface, which should resolve to a callable + * - Psr\Http\Message\StreamFactoryInterface, which should resolve to a factory * that will produce an empty Psr\Http\Message\StreamInterface instance. * * @final @@ -24,8 +24,7 @@ class ImplicitHeadMiddlewareFactory { /** - * @throws MissingDependencyException If either the Mezzio\Router\RouterInterface - * or Psr\Http\Message\StreamInterface services are missing. + * @throws MissingDependencyException If either the RouterInterface or StreamFactoryInterface services are missing. */ public function __invoke(ContainerInterface $container): ImplicitHeadMiddleware { @@ -36,16 +35,16 @@ public function __invoke(ContainerInterface $container): ImplicitHeadMiddleware ); } - if (! $container->has(StreamInterface::class)) { + if (! $container->has(StreamFactoryInterface::class)) { throw MissingDependencyException::dependencyForService( - StreamInterface::class, + StreamFactoryInterface::class, ImplicitHeadMiddleware::class ); } return new ImplicitHeadMiddleware( $container->get(RouterInterface::class), - $container->get(StreamInterface::class) + $container->get(StreamFactoryInterface::class) ); } } diff --git a/src/Middleware/ImplicitOptionsMiddleware.php b/src/Middleware/ImplicitOptionsMiddleware.php index 20ce511..239a2f8 100644 --- a/src/Middleware/ImplicitOptionsMiddleware.php +++ b/src/Middleware/ImplicitOptionsMiddleware.php @@ -5,7 +5,6 @@ namespace Mezzio\Router\Middleware; use Fig\Http\Message\RequestMethodInterface as RequestMethod; -use Mezzio\Router\Response\CallableResponseFactoryDecorator; use Mezzio\Router\RouteResult; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; @@ -16,7 +15,6 @@ use function assert; use function implode; use function is_array; -use function is_callable; /** * Handle implicit OPTIONS requests. @@ -45,26 +43,8 @@ */ class ImplicitOptionsMiddleware implements MiddlewareInterface { - /** @var ResponseFactoryInterface */ - private $responseFactory; - - /** - * @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory A factory capable of returning an - * empty ResponseInterface instance to return for implicit OPTIONS - * requests. - */ - public function __construct($responseFactory) + public function __construct(private readonly ResponseFactoryInterface $responseFactory) { - if (is_callable($responseFactory)) { - // Factories are wrapped in a closure in order to enforce return type safety. - $responseFactory = new CallableResponseFactoryDecorator( - static function () use ($responseFactory): ResponseInterface { - return $responseFactory(); - } - ); - } - - $this->responseFactory = $responseFactory; } /** diff --git a/src/Middleware/ImplicitOptionsMiddlewareFactory.php b/src/Middleware/ImplicitOptionsMiddlewareFactory.php index bc3f07e..6435ad9 100644 --- a/src/Middleware/ImplicitOptionsMiddlewareFactory.php +++ b/src/Middleware/ImplicitOptionsMiddlewareFactory.php @@ -6,29 +6,34 @@ use Mezzio\Router\Exception\MissingDependencyException; use Psr\Container\ContainerInterface; +use Psr\Http\Message\ResponseFactoryInterface; /** * Create and return an ImplicitOptionsMiddleware instance. * * This factory depends on one other service: * - * - Psr\Http\Message\ResponseInterface, which should resolve to a callable - * that will produce an empty Psr\Http\Message\ResponseInterface instance. + * - \Psr\Http\Message\ResponseFactoryInterface, which should resolve to a factory + * that will produce an empty \Psr\Http\Message\ResponseInterface instance. * * @final */ class ImplicitOptionsMiddlewareFactory { - use Psr17ResponseFactoryTrait; - /** - * @throws MissingDependencyException If the Psr\Http\Message\ResponseInterface - * service is missing. + * @throws MissingDependencyException If the ResponseFactoryInterface service is missing. */ public function __invoke(ContainerInterface $container): ImplicitOptionsMiddleware { + if (! $container->has(ResponseFactoryInterface::class)) { + throw MissingDependencyException::dependencyForService( + ResponseFactoryInterface::class, + ImplicitOptionsMiddleware::class + ); + } + return new ImplicitOptionsMiddleware( - $this->detectResponseFactory($container) + $container->get(ResponseFactoryInterface::class), ); } } diff --git a/src/Middleware/MethodNotAllowedMiddleware.php b/src/Middleware/MethodNotAllowedMiddleware.php index b0eb86e..9f9eaae 100644 --- a/src/Middleware/MethodNotAllowedMiddleware.php +++ b/src/Middleware/MethodNotAllowedMiddleware.php @@ -5,7 +5,6 @@ namespace Mezzio\Router\Middleware; use Fig\Http\Message\StatusCodeInterface as StatusCode; -use Mezzio\Router\Response\CallableResponseFactoryDecorator; use Mezzio\Router\RouteResult; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; @@ -16,7 +15,6 @@ use function assert; use function implode; use function is_array; -use function is_callable; /** * Emit a 405 Method Not Allowed response @@ -33,24 +31,8 @@ */ class MethodNotAllowedMiddleware implements MiddlewareInterface { - /** @var ResponseFactoryInterface */ - private $responseFactory; - - /** - * @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory - */ - public function __construct($responseFactory) + public function __construct(private readonly ResponseFactoryInterface $responseFactory) { - if (is_callable($responseFactory)) { - // Factories are wrapped in a closure in order to enforce return type safety. - $responseFactory = new CallableResponseFactoryDecorator( - function () use ($responseFactory): ResponseInterface { - return $responseFactory(); - } - ); - } - - $this->responseFactory = $responseFactory; } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface diff --git a/src/Middleware/MethodNotAllowedMiddlewareFactory.php b/src/Middleware/MethodNotAllowedMiddlewareFactory.php index 2548630..a031fcc 100644 --- a/src/Middleware/MethodNotAllowedMiddlewareFactory.php +++ b/src/Middleware/MethodNotAllowedMiddlewareFactory.php @@ -6,29 +6,34 @@ use Mezzio\Router\Exception\MissingDependencyException; use Psr\Container\ContainerInterface; +use Psr\Http\Message\ResponseFactoryInterface; /** * Create and return a MethodNotAllowedMiddleware instance. * * This factory depends on one other service: * - * - Psr\Http\Message\ResponseInterface, which should resolve to a callable + * - \Psr\Http\Message\ResponseFactoryInterface, which should resolve to a factory * that will produce an empty Psr\Http\Message\ResponseInterface instance. * * @final */ class MethodNotAllowedMiddlewareFactory { - use Psr17ResponseFactoryTrait; - /** - * @throws MissingDependencyException If the Psr\Http\Message\ResponseInterface - * service is missing. + * @throws MissingDependencyException If the ResponseFactoryInterface service is missing. */ public function __invoke(ContainerInterface $container): MethodNotAllowedMiddleware { + if (! $container->has(ResponseFactoryInterface::class)) { + throw MissingDependencyException::dependencyForService( + ResponseFactoryInterface::class, + MethodNotAllowedMiddleware::class + ); + } + return new MethodNotAllowedMiddleware( - $this->detectResponseFactory($container) + $container->get(ResponseFactoryInterface::class), ); } } diff --git a/src/Middleware/Psr17ResponseFactoryTrait.php b/src/Middleware/Psr17ResponseFactoryTrait.php deleted file mode 100644 index 3ede604..0000000 --- a/src/Middleware/Psr17ResponseFactoryTrait.php +++ /dev/null @@ -1,81 +0,0 @@ -has(ResponseFactoryInterface::class); - - if (! $psr17FactoryAvailable) { - return $this->createResponseFactoryFromDeprecatedCallable($container); - } - - if ($this->doesConfigurationProvidesDedicatedResponseFactory($container)) { - return $this->createResponseFactoryFromDeprecatedCallable($container); - } - - $responseFactory = $container->get(ResponseFactoryInterface::class); - Assert::isInstanceOf($responseFactory, ResponseFactoryInterface::class); - return $responseFactory; - } - - private function createResponseFactoryFromDeprecatedCallable( - ContainerInterface $container - ): ResponseFactoryInterface { - if (! $container->has(ResponseInterface::class)) { - throw MissingDependencyException::dependencyForService( - ResponseInterface::class, - MethodNotAllowedMiddleware::class - ); - } - - /** @var callable():ResponseInterface $responseFactory */ - $responseFactory = $container->get(ResponseInterface::class); - - return new CallableResponseFactoryDecorator($responseFactory); - } - - private function doesConfigurationProvidesDedicatedResponseFactory(ContainerInterface $container): bool - { - if (! $container->has('config')) { - return false; - } - - $config = $container->get('config'); - Assert::isArrayAccessible($config); - $dependencies = $config['dependencies'] ?? []; - Assert::isMap($dependencies); - - $delegators = $dependencies['delegators'] ?? []; - $aliases = $dependencies['aliases'] ?? []; - Assert::isArrayAccessible($delegators); - Assert::isArrayAccessible($aliases); - - if (isset($delegators[ResponseInterface::class]) || isset($aliases[ResponseInterface::class])) { - // Even tho, aliases could point to a different service, we assume that there is a dedicated factory - // available. The alias resolving is not worth it. - return true; - } - - /** @psalm-suppress MixedAssignment */ - $deprecatedResponseFactory = $dependencies['factories'][ResponseInterface::class] ?? null; - - return $deprecatedResponseFactory !== null - && $deprecatedResponseFactory !== 'Mezzio\Container\ResponseFactoryFactory'; - } -} diff --git a/src/Response/CallableResponseFactoryDecorator.php b/src/Response/CallableResponseFactoryDecorator.php deleted file mode 100644 index 1059ce3..0000000 --- a/src/Response/CallableResponseFactoryDecorator.php +++ /dev/null @@ -1,36 +0,0 @@ -responseFactory = $responseFactory; - } - - public function createResponse(int $code = 200, string $reasonPhrase = ''): ResponseInterface - { - return $this->getResponseFromCallable()->withStatus($code, $reasonPhrase); - } - - public function getResponseFromCallable(): ResponseInterface - { - return ($this->responseFactory)(); - } -} diff --git a/src/Test/AbstractImplicitMethodsIntegrationTest.php b/src/Test/AbstractImplicitMethodsIntegrationTest.php index 3f9e98e..b7247a0 100644 --- a/src/Test/AbstractImplicitMethodsIntegrationTest.php +++ b/src/Test/AbstractImplicitMethodsIntegrationTest.php @@ -9,7 +9,7 @@ use Generator; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; -use Laminas\Diactoros\Stream; +use Laminas\Diactoros\StreamFactory; use Laminas\Stratigility\MiddlewarePipe; use Mezzio\Router\Middleware\DispatchMiddleware; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; @@ -22,6 +22,7 @@ use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -46,9 +47,7 @@ abstract public function getRouter(): RouterInterface; public function getImplicitOptionsMiddleware(?ResponseInterface $response = null): ImplicitOptionsMiddleware { return new ImplicitOptionsMiddleware( - function () use ($response): ResponseInterface { - return $response ?? new Response(); - } + new ResponseFactory(), ); } @@ -56,19 +55,17 @@ public function getImplicitHeadMiddleware(RouterInterface $router): ImplicitHead { return new ImplicitHeadMiddleware( $router, - function () { - return new Stream('php://temp', 'rw'); - } + new StreamFactory(), ); } - /** - * @return callable(): never - */ - public function createInvalidResponseFactory(): callable + public function createInvalidResponseFactory(): ResponseFactoryInterface { - return static function (): ResponseInterface { - self::fail('Response generated when it should not have been'); + return new class implements ResponseFactoryInterface { + public function createResponse(int $code = 200, string $reasonPhrase = ''): ResponseInterface + { + TestCase::fail('Response generated when it should not have been'); + } }; } @@ -238,9 +235,7 @@ public function testWithoutImplicitMiddleware(string $requestMethod, array $allo $pipeline = new MiddlewarePipe(); $pipeline->pipe(new RouteMiddleware($router)); - $pipeline->pipe(new MethodNotAllowedMiddleware(static function () use ($finalResponse): ResponseInterface { - return $finalResponse; - })); + $pipeline->pipe(new MethodNotAllowedMiddleware(new ResponseFactory($finalResponse))); $finalHandler = $this->createMock(RequestHandlerInterface::class); $finalHandler diff --git a/src/Test/ImplicitMethodsIntegrationTest.php b/src/Test/ImplicitMethodsIntegrationTest.php deleted file mode 100644 index 4af253b..0000000 --- a/src/Test/ImplicitMethodsIntegrationTest.php +++ /dev/null @@ -1,14 +0,0 @@ -defaultResponse + ? $this->defaultResponse->withStatus($code, $reasonPhrase) + : (new Response())->withStatus($code, $reasonPhrase); + } +} diff --git a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php index d710ce1..4573c17 100644 --- a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php +++ b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php @@ -4,6 +4,7 @@ namespace MezzioTest\Router\Middleware; +use Laminas\Diactoros\StreamFactory; use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\Middleware\ImplicitHeadMiddlewareFactory; use Mezzio\Router\RouterInterface; @@ -11,7 +12,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; -use Psr\Http\Message\StreamInterface; +use Psr\Http\Message\StreamFactoryInterface; use function in_array; @@ -50,7 +51,7 @@ public function testFactoryRaisesExceptionIfStreamFactoryServiceIsMissing(): voi ->expects(self::exactly(2)) ->method('has') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); + self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); return true; })) ->willReturnOnConsecutiveCalls(true, false); @@ -63,14 +64,13 @@ public function testFactoryRaisesExceptionIfStreamFactoryServiceIsMissing(): voi public function testFactoryProducesImplicitHeadMiddlewareWhenAllDependenciesPresent(): void { $router = $this->createMock(RouterInterface::class); - $streamFactory = static function (): void { - }; + $streamFactory = new StreamFactory(); $this->container ->expects(self::exactly(2)) ->method('has') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); + self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); return true; })) ->willReturn(true); @@ -79,7 +79,7 @@ public function testFactoryProducesImplicitHeadMiddlewareWhenAllDependenciesPres ->expects(self::exactly(2)) ->method('get') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); + self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); return true; })) ->willReturnOnConsecutiveCalls($router, $streamFactory); diff --git a/test/Middleware/ImplicitHeadMiddlewareTest.php b/test/Middleware/ImplicitHeadMiddlewareTest.php index a13afda..5e39dc7 100644 --- a/test/Middleware/ImplicitHeadMiddlewareTest.php +++ b/test/Middleware/ImplicitHeadMiddlewareTest.php @@ -7,6 +7,7 @@ use Fig\Http\Message\RequestMethodInterface as RequestMethod; use Laminas\Diactoros\Response\TextResponse; use Laminas\Diactoros\ServerRequest; +use Laminas\Diactoros\StreamFactory; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; @@ -15,7 +16,6 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Message\StreamInterface; use Psr\Http\Server\MiddlewareInterface; /** @covers \Mezzio\Router\Middleware\ImplicitHeadMiddleware */ @@ -33,7 +33,7 @@ protected function setUp(): void $this->router = $this->createMock(RouterInterface::class); $this->middleware = new ImplicitHeadMiddleware( $this->router, - fn (): StreamInterface => $this->createMock(StreamInterface::class), + new StreamFactory(), ); } diff --git a/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php b/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php index 0126d05..4a6c89c 100644 --- a/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php +++ b/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php @@ -6,12 +6,12 @@ use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\Middleware\ImplicitOptionsMiddlewareFactory; +use Mezzio\Router\Test\ResponseFactory; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseFactoryInterface; -use Psr\Http\Message\ResponseInterface; #[CoversClass(ImplicitOptionsMiddlewareFactory::class)] final class ImplicitOptionsMiddlewareFactoryTest extends TestCase @@ -38,25 +38,17 @@ public function testFactoryRaisesExceptionIfResponseFactoryServiceIsMissing(): v public function testFactoryProducesImplicitOptionsMiddlewareWhenAllDependenciesPresent(): void { - $factory = static function (): void { - }; - $this->container + ->expects(self::once()) ->method('has') - ->with(self::callback(static function ($arg): bool { - self::assertContains($arg, [ - ResponseFactoryInterface::class, - ResponseInterface::class, - ]); - - return true; - }))->willReturnOnConsecutiveCalls(false, true); + ->with(ResponseFactoryInterface::class) + ->willReturn(true); $this->container ->expects(self::once()) ->method('get') - ->with(ResponseInterface::class) - ->willReturn($factory); + ->with(ResponseFactoryInterface::class) + ->willReturn(new ResponseFactory()); ($this->factory)($this->container); } diff --git a/test/Middleware/ImplicitOptionsMiddlewareTest.php b/test/Middleware/ImplicitOptionsMiddlewareTest.php index 7ec06c3..1b5e02f 100644 --- a/test/Middleware/ImplicitOptionsMiddlewareTest.php +++ b/test/Middleware/ImplicitOptionsMiddlewareTest.php @@ -5,33 +5,29 @@ namespace MezzioTest\Router\Middleware; use Fig\Http\Message\RequestMethodInterface as RequestMethod; +use Laminas\Diactoros\Response; +use Laminas\Diactoros\ServerRequest; use Mezzio\Router\Middleware\ImplicitOptionsMiddleware; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; -use PHPUnit\Framework\MockObject\MockObject; +use Mezzio\Router\Test\ResponseFactory; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use function implode; - /** @covers \Mezzio\Router\Middleware\ImplicitOptionsMiddleware */ final class ImplicitOptionsMiddlewareTest extends TestCase { - /** @var ResponseInterface&MockObject */ private ResponseInterface $response; - private ImplicitOptionsMiddleware $middleware; protected function setUp(): void { parent::setUp(); - $this->response = $this->createMock(ResponseInterface::class); - $responseFactory = fn (): ResponseInterface => $this->response; - - $this->middleware = new ImplicitOptionsMiddleware($responseFactory); + $this->response = new Response(); + $this->middleware = new ImplicitOptionsMiddleware(new ResponseFactory($this->response)); } public function testNonOptionsRequestInvokesHandler(): void @@ -117,34 +113,21 @@ public function testInjectsAllowHeaderInResponseProvidedToConstructorDuringOptio $result = RouteResult::fromRouteFailure($allowedMethods); - $request = $this->createMock(ServerRequestInterface::class); - $request - ->method('getMethod') - ->willReturn(RequestMethod::METHOD_OPTIONS); - - $request - ->method('getAttribute') - ->with(RouteResult::class) - ->willReturn($result); + $request = (new ServerRequest()) + ->withMethod(RequestMethod::METHOD_OPTIONS) + ->withAttribute(RouteResult::class, $result); $handler = $this->createMock(RequestHandlerInterface::class); $handler ->expects(self::never()) ->method('handle'); - $this->response - ->method('withStatus') - ->willReturnSelf(); - - $this->response - ->expects(self::once()) - ->method('withHeader') - ->with('Allow', implode(',', $allowedMethods)) - ->willReturnSelf(); + self::assertFalse($this->response->hasHeader('Allow')); - $result = $this->middleware->process($request, $handler); + $response = $this->middleware->process($request, $handler); - self::assertSame($this->response, $result); + self::assertNotSame($this->response, $response); + self::assertTrue($response->hasHeader('Allow')); } public function testReturnsResultOfHandlerWhenRouteNotFound(): void diff --git a/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php b/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php index a8005e2..7814cee 100644 --- a/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php +++ b/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php @@ -4,26 +4,20 @@ namespace MezzioTest\Router\Middleware; -use Generator; use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\Middleware\MethodNotAllowedMiddlewareFactory; -use Mezzio\Router\Response\CallableResponseFactoryDecorator; +use Mezzio\Router\Test\ResponseFactory; use MezzioTest\Router\InMemoryContainer; use PHPUnit\Framework\Attributes\CoversClass; -use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseFactoryInterface; -use Psr\Http\Message\ResponseInterface; - -use function in_array; #[CoversClass(MethodNotAllowedMiddlewareFactory::class)] final class MethodNotAllowedMiddlewareFactoryTest extends TestCase { private static ResponseFactoryInterface&MockObject $responseFactoryMock; - private static ResponseInterface&MockObject $responseMock; private ContainerInterface&MockObject $container; private MethodNotAllowedMiddlewareFactory $factory; @@ -35,59 +29,15 @@ protected function setUp(): void $this->factory = new MethodNotAllowedMiddlewareFactory(); self::$responseFactoryMock = $this->createMock(ResponseFactoryInterface::class); - self::$responseMock = $this->createMock(ResponseInterface::class); - } - - /** - * @psalm-return Generator}> - */ - public static function configurationsWithOverriddenResponseInterfaceFactory(): Generator - { - yield 'default' => [ - [ - 'dependencies' => [ - 'factories' => [ - ResponseInterface::class => function (): ResponseInterface { - return self::$responseMock; - }, - ], - ], - ], - ]; - - yield 'aliased' => [ - [ - 'dependencies' => [ - 'aliases' => [ - ResponseInterface::class => 'CustomResponseInterface', - ], - ], - ], - ]; - - yield 'delegated' => [ - [ - 'dependencies' => [ - 'delegators' => [ - ResponseInterface::class => [ - fn (): ResponseInterface => self::$responseMock, - ], - ], - ], - ], - ]; } public function testFactoryRaisesExceptionIfResponseFactoryServiceIsMissing(): void { $this->container + ->expects(self::once()) ->method('has') - ->with(self::callback(static function ($arg): bool { - return in_array($arg, [ - ResponseFactoryInterface::class, - ResponseInterface::class, - ], true); - }))->willReturn(false); + ->with(ResponseFactoryInterface::class) + ->willReturn(false); $this->expectException(MissingDependencyException::class); @@ -96,60 +46,28 @@ public function testFactoryRaisesExceptionIfResponseFactoryServiceIsMissing(): v public function testFactoryProducesMethodNotAllowedMiddlewareWhenAllDependenciesPresent(): void { - $factory = static function (): void { - }; - $this->container - ->expects(self::exactly(2)) + ->expects(self::once()) ->method('has') - ->with(self::callback(static function ($arg): bool { - return in_array($arg, [ - ResponseFactoryInterface::class, - ResponseInterface::class, - ], true); - })) - ->willReturn(false, true); + ->with(ResponseFactoryInterface::class) + ->willReturn(true); $this->container ->expects(self::once()) ->method('get') - ->with(ResponseInterface::class) - ->willReturn($factory); + ->with(ResponseFactoryInterface::class) + ->willReturn(new ResponseFactory()); ($this->factory)($this->container); } - public function testWillUseResponseFactoryInterfaceFromContainerWhenApplicationFactoryIsNotOverridden(): void + public function testWillUseResponseFactoryInterfaceFromContainer(): void { - $container = new InMemoryContainer(); - $container->set('config', [ - 'dependencies' => [ - 'factories' => [ - ResponseInterface::class => 'Mezzio\Container\ResponseFactoryFactory', - ], - ], - ]); - $container->set(ResponseFactoryInterface::class, self::$responseFactoryMock); + $responseFactory = new ResponseFactory(); + $container = new InMemoryContainer(); + $container->set(ResponseFactoryInterface::class, $responseFactory); $middleware = ($this->factory)($container); - self::assertSame(self::$responseFactoryMock, $middleware->getResponseFactory()); - } - - /** @param array $config */ - #[DataProvider('configurationsWithOverriddenResponseInterfaceFactory')] - public function testWontUseResponseFactoryInterfaceFromContainerWhenApplicationFactoryIsOverriden( - array $config - ): void { - $container = new InMemoryContainer(); - $container->set('config', $config); - $container->set(ResponseFactoryInterface::class, self::$responseFactoryMock); - $container->set(ResponseInterface::class, static fn (): ResponseInterface => self::$responseMock); - - $middleware = ($this->factory)($container); - $responseFactoryFromGenerator = $middleware->getResponseFactory(); - - self::assertNotSame(self::$responseFactoryMock, $responseFactoryFromGenerator); - self::assertInstanceOf(CallableResponseFactoryDecorator::class, $responseFactoryFromGenerator); - self::assertSame(self::$responseMock, $responseFactoryFromGenerator->getResponseFromCallable()); + self::assertSame($responseFactory, $middleware->getResponseFactory()); } } diff --git a/test/Middleware/MethodNotAllowedMiddlewareTest.php b/test/Middleware/MethodNotAllowedMiddlewareTest.php index e420c1c..c98c6d5 100644 --- a/test/Middleware/MethodNotAllowedMiddlewareTest.php +++ b/test/Middleware/MethodNotAllowedMiddlewareTest.php @@ -8,6 +8,7 @@ use Mezzio\Router\Middleware\MethodNotAllowedMiddleware; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; +use Mezzio\Router\Test\ResponseFactory; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; @@ -32,12 +33,11 @@ protected function setUp(): void { parent::setUp(); - $this->handler = $this->createMock(RequestHandlerInterface::class); - $this->request = $this->createMock(ServerRequestInterface::class); - $this->response = $this->createMock(ResponseInterface::class); - $responseFactory = fn (): ResponseInterface => $this->response; + $this->handler = $this->createMock(RequestHandlerInterface::class); + $this->request = $this->createMock(ServerRequestInterface::class); + $this->response = $this->createMock(ResponseInterface::class); - $this->middleware = new MethodNotAllowedMiddleware($responseFactory); + $this->middleware = new MethodNotAllowedMiddleware(new ResponseFactory($this->response)); } public function testDelegatesToHandlerIfNoRouteResultPresentInRequest(): void diff --git a/test/Response/CallableResponseFactoryDecoratorTest.php b/test/Response/CallableResponseFactoryDecoratorTest.php deleted file mode 100644 index 7c1319a..0000000 --- a/test/Response/CallableResponseFactoryDecoratorTest.php +++ /dev/null @@ -1,48 +0,0 @@ -response = $this->createMock(ResponseInterface::class); - $this->factory = new CallableResponseFactoryDecorator(fn (): ResponseInterface => $this->response); - } - - public function testWillPassStatusCodeAndPhraseToCallable(): void - { - $this->response - ->expects(self::once()) - ->method('withStatus') - ->with(500, 'Foo') - ->willReturnSelf(); - - $this->factory->createResponse(500, 'Foo'); - } - - public function testWillReturnSameResponseInstance(): void - { - $this->response - ->expects(self::once()) - ->method('withStatus') - ->willReturnSelf(); - - self::assertSame($this->response, $this->factory->createResponse()); - } -} diff --git a/test/ServiceManagerIntegrationTest.php b/test/ServiceManagerIntegrationTest.php index 2d99b7b..af13515 100644 --- a/test/ServiceManagerIntegrationTest.php +++ b/test/ServiceManagerIntegrationTest.php @@ -4,6 +4,7 @@ namespace MezzioTest\Router; +use Laminas\Diactoros\StreamFactory; use Laminas\ServiceManager\ConfigInterface; use Laminas\ServiceManager\ServiceManager; use Mezzio\Router; @@ -11,7 +12,7 @@ use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseFactoryInterface; -use Psr\Http\Message\StreamInterface; +use Psr\Http\Message\StreamFactoryInterface; use function array_merge_recursive; @@ -30,13 +31,13 @@ protected function setUp(): void [ 'factories' => [ ResponseFactoryInterface::class => function (): ResponseFactoryInterface { - return $this->createMock(ResponseFactoryInterface::class); + return new Router\Test\ResponseFactory(); }, Router\RouterInterface::class => function (): Router\RouterInterface { return $this->createMock(Router\RouterInterface::class); }, - StreamInterface::class => function (): callable { - return fn (): StreamInterface => $this->createMock(StreamInterface::class); + StreamFactoryInterface::class => function (): StreamFactoryInterface { + return new StreamFactory(); }, ], ], From 46bbd4adb020e92a166b683f3e36cdbad779493c Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 29 Mar 2023 11:42:30 +0100 Subject: [PATCH 2/6] Revert "Add PSR-17 Support" This reverts commit c12739633f358b7b3229109373c605c2d8df9c5b. Signed-off-by: George Steel --- psalm-baseline.xml | 14 ++- src/Middleware/ImplicitHeadMiddleware.php | 22 +++- .../ImplicitHeadMiddlewareFactory.php | 13 ++- src/Middleware/ImplicitOptionsMiddleware.php | 22 +++- .../ImplicitOptionsMiddlewareFactory.php | 19 ++- src/Middleware/MethodNotAllowedMiddleware.php | 20 +++- .../MethodNotAllowedMiddlewareFactory.php | 17 +-- src/Middleware/Psr17ResponseFactoryTrait.php | 81 +++++++++++++ .../CallableResponseFactoryDecorator.php | 36 ++++++ ...AbstractImplicitMethodsIntegrationTest.php | 27 +++-- src/Test/ImplicitMethodsIntegrationTest.php | 14 +++ src/Test/ResponseFactory.php | 24 ---- .../ImplicitHeadMiddlewareFactoryTest.php | 12 +- .../Middleware/ImplicitHeadMiddlewareTest.php | 4 +- .../ImplicitOptionsMiddlewareFactoryTest.php | 20 +++- .../ImplicitOptionsMiddlewareTest.php | 41 +++++-- .../MethodNotAllowedMiddlewareFactoryTest.php | 110 +++++++++++++++--- .../MethodNotAllowedMiddlewareTest.php | 10 +- .../CallableResponseFactoryDecoratorTest.php | 48 ++++++++ test/ServiceManagerIntegrationTest.php | 9 +- 20 files changed, 439 insertions(+), 124 deletions(-) create mode 100644 src/Middleware/Psr17ResponseFactoryTrait.php create mode 100644 src/Response/CallableResponseFactoryDecorator.php create mode 100644 src/Test/ImplicitMethodsIntegrationTest.php delete mode 100644 src/Test/ResponseFactory.php create mode 100644 test/Response/CallableResponseFactoryDecoratorTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 4266fab..cb8cfab 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,8 +1,18 @@ - + + + + $container->get(StreamInterface::class) + + - services[$id]]]> + $this->services[$id] + + + new CallableResponseFactoryDecorator(fn (): ResponseInterface => $this->response) + + diff --git a/src/Middleware/ImplicitHeadMiddleware.php b/src/Middleware/ImplicitHeadMiddleware.php index 4b09bf7..f82bdf3 100644 --- a/src/Middleware/ImplicitHeadMiddleware.php +++ b/src/Middleware/ImplicitHeadMiddleware.php @@ -9,7 +9,7 @@ use Mezzio\Router\RouterInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Message\StreamFactoryInterface; +use Psr\Http\Message\StreamInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; @@ -44,10 +44,19 @@ class ImplicitHeadMiddleware implements MiddlewareInterface { public const FORWARDED_HTTP_METHOD_ATTRIBUTE = 'forwarded_http_method'; - public function __construct( - private readonly RouterInterface $router, - private readonly StreamFactoryInterface $streamFactory, - ) { + /** @var callable(): StreamInterface */ + private $streamFactory; + + /** + * @param callable(): StreamInterface $streamFactory A factory capable of returning an empty + * StreamInterface instance to inject in a response. + */ + public function __construct(private RouterInterface $router, callable $streamFactory) + { + // Factory is wrapped in closure in order to enforce return type safety. + $this->streamFactory = function () use ($streamFactory): StreamInterface { + return $streamFactory(); + }; } /** @@ -90,6 +99,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface ->withMethod(RequestMethod::METHOD_GET) ); - return $response->withBody($this->streamFactory->createStream()); + $body = ($this->streamFactory)(); + return $response->withBody($body); } } diff --git a/src/Middleware/ImplicitHeadMiddlewareFactory.php b/src/Middleware/ImplicitHeadMiddlewareFactory.php index 941b687..ec88dc6 100644 --- a/src/Middleware/ImplicitHeadMiddlewareFactory.php +++ b/src/Middleware/ImplicitHeadMiddlewareFactory.php @@ -7,7 +7,7 @@ use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\RouterInterface; use Psr\Container\ContainerInterface; -use Psr\Http\Message\StreamFactoryInterface; +use Psr\Http\Message\StreamInterface; /** * Create and return an ImplicitHeadMiddleware instance. @@ -16,7 +16,7 @@ * * - Mezzio\Router\RouterInterface, which should resolve to an * instance of that interface. - * - Psr\Http\Message\StreamFactoryInterface, which should resolve to a factory + * - Psr\Http\Message\StreamInterface, which should resolve to a callable * that will produce an empty Psr\Http\Message\StreamInterface instance. * * @final @@ -24,7 +24,8 @@ class ImplicitHeadMiddlewareFactory { /** - * @throws MissingDependencyException If either the RouterInterface or StreamFactoryInterface services are missing. + * @throws MissingDependencyException If either the Mezzio\Router\RouterInterface + * or Psr\Http\Message\StreamInterface services are missing. */ public function __invoke(ContainerInterface $container): ImplicitHeadMiddleware { @@ -35,16 +36,16 @@ public function __invoke(ContainerInterface $container): ImplicitHeadMiddleware ); } - if (! $container->has(StreamFactoryInterface::class)) { + if (! $container->has(StreamInterface::class)) { throw MissingDependencyException::dependencyForService( - StreamFactoryInterface::class, + StreamInterface::class, ImplicitHeadMiddleware::class ); } return new ImplicitHeadMiddleware( $container->get(RouterInterface::class), - $container->get(StreamFactoryInterface::class) + $container->get(StreamInterface::class) ); } } diff --git a/src/Middleware/ImplicitOptionsMiddleware.php b/src/Middleware/ImplicitOptionsMiddleware.php index 239a2f8..20ce511 100644 --- a/src/Middleware/ImplicitOptionsMiddleware.php +++ b/src/Middleware/ImplicitOptionsMiddleware.php @@ -5,6 +5,7 @@ namespace Mezzio\Router\Middleware; use Fig\Http\Message\RequestMethodInterface as RequestMethod; +use Mezzio\Router\Response\CallableResponseFactoryDecorator; use Mezzio\Router\RouteResult; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; @@ -15,6 +16,7 @@ use function assert; use function implode; use function is_array; +use function is_callable; /** * Handle implicit OPTIONS requests. @@ -43,8 +45,26 @@ */ class ImplicitOptionsMiddleware implements MiddlewareInterface { - public function __construct(private readonly ResponseFactoryInterface $responseFactory) + /** @var ResponseFactoryInterface */ + private $responseFactory; + + /** + * @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory A factory capable of returning an + * empty ResponseInterface instance to return for implicit OPTIONS + * requests. + */ + public function __construct($responseFactory) { + if (is_callable($responseFactory)) { + // Factories are wrapped in a closure in order to enforce return type safety. + $responseFactory = new CallableResponseFactoryDecorator( + static function () use ($responseFactory): ResponseInterface { + return $responseFactory(); + } + ); + } + + $this->responseFactory = $responseFactory; } /** diff --git a/src/Middleware/ImplicitOptionsMiddlewareFactory.php b/src/Middleware/ImplicitOptionsMiddlewareFactory.php index 6435ad9..bc3f07e 100644 --- a/src/Middleware/ImplicitOptionsMiddlewareFactory.php +++ b/src/Middleware/ImplicitOptionsMiddlewareFactory.php @@ -6,34 +6,29 @@ use Mezzio\Router\Exception\MissingDependencyException; use Psr\Container\ContainerInterface; -use Psr\Http\Message\ResponseFactoryInterface; /** * Create and return an ImplicitOptionsMiddleware instance. * * This factory depends on one other service: * - * - \Psr\Http\Message\ResponseFactoryInterface, which should resolve to a factory - * that will produce an empty \Psr\Http\Message\ResponseInterface instance. + * - Psr\Http\Message\ResponseInterface, which should resolve to a callable + * that will produce an empty Psr\Http\Message\ResponseInterface instance. * * @final */ class ImplicitOptionsMiddlewareFactory { + use Psr17ResponseFactoryTrait; + /** - * @throws MissingDependencyException If the ResponseFactoryInterface service is missing. + * @throws MissingDependencyException If the Psr\Http\Message\ResponseInterface + * service is missing. */ public function __invoke(ContainerInterface $container): ImplicitOptionsMiddleware { - if (! $container->has(ResponseFactoryInterface::class)) { - throw MissingDependencyException::dependencyForService( - ResponseFactoryInterface::class, - ImplicitOptionsMiddleware::class - ); - } - return new ImplicitOptionsMiddleware( - $container->get(ResponseFactoryInterface::class), + $this->detectResponseFactory($container) ); } } diff --git a/src/Middleware/MethodNotAllowedMiddleware.php b/src/Middleware/MethodNotAllowedMiddleware.php index 9f9eaae..b0eb86e 100644 --- a/src/Middleware/MethodNotAllowedMiddleware.php +++ b/src/Middleware/MethodNotAllowedMiddleware.php @@ -5,6 +5,7 @@ namespace Mezzio\Router\Middleware; use Fig\Http\Message\StatusCodeInterface as StatusCode; +use Mezzio\Router\Response\CallableResponseFactoryDecorator; use Mezzio\Router\RouteResult; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; @@ -15,6 +16,7 @@ use function assert; use function implode; use function is_array; +use function is_callable; /** * Emit a 405 Method Not Allowed response @@ -31,8 +33,24 @@ */ class MethodNotAllowedMiddleware implements MiddlewareInterface { - public function __construct(private readonly ResponseFactoryInterface $responseFactory) + /** @var ResponseFactoryInterface */ + private $responseFactory; + + /** + * @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory + */ + public function __construct($responseFactory) { + if (is_callable($responseFactory)) { + // Factories are wrapped in a closure in order to enforce return type safety. + $responseFactory = new CallableResponseFactoryDecorator( + function () use ($responseFactory): ResponseInterface { + return $responseFactory(); + } + ); + } + + $this->responseFactory = $responseFactory; } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface diff --git a/src/Middleware/MethodNotAllowedMiddlewareFactory.php b/src/Middleware/MethodNotAllowedMiddlewareFactory.php index a031fcc..2548630 100644 --- a/src/Middleware/MethodNotAllowedMiddlewareFactory.php +++ b/src/Middleware/MethodNotAllowedMiddlewareFactory.php @@ -6,34 +6,29 @@ use Mezzio\Router\Exception\MissingDependencyException; use Psr\Container\ContainerInterface; -use Psr\Http\Message\ResponseFactoryInterface; /** * Create and return a MethodNotAllowedMiddleware instance. * * This factory depends on one other service: * - * - \Psr\Http\Message\ResponseFactoryInterface, which should resolve to a factory + * - Psr\Http\Message\ResponseInterface, which should resolve to a callable * that will produce an empty Psr\Http\Message\ResponseInterface instance. * * @final */ class MethodNotAllowedMiddlewareFactory { + use Psr17ResponseFactoryTrait; + /** - * @throws MissingDependencyException If the ResponseFactoryInterface service is missing. + * @throws MissingDependencyException If the Psr\Http\Message\ResponseInterface + * service is missing. */ public function __invoke(ContainerInterface $container): MethodNotAllowedMiddleware { - if (! $container->has(ResponseFactoryInterface::class)) { - throw MissingDependencyException::dependencyForService( - ResponseFactoryInterface::class, - MethodNotAllowedMiddleware::class - ); - } - return new MethodNotAllowedMiddleware( - $container->get(ResponseFactoryInterface::class), + $this->detectResponseFactory($container) ); } } diff --git a/src/Middleware/Psr17ResponseFactoryTrait.php b/src/Middleware/Psr17ResponseFactoryTrait.php new file mode 100644 index 0000000..3ede604 --- /dev/null +++ b/src/Middleware/Psr17ResponseFactoryTrait.php @@ -0,0 +1,81 @@ +has(ResponseFactoryInterface::class); + + if (! $psr17FactoryAvailable) { + return $this->createResponseFactoryFromDeprecatedCallable($container); + } + + if ($this->doesConfigurationProvidesDedicatedResponseFactory($container)) { + return $this->createResponseFactoryFromDeprecatedCallable($container); + } + + $responseFactory = $container->get(ResponseFactoryInterface::class); + Assert::isInstanceOf($responseFactory, ResponseFactoryInterface::class); + return $responseFactory; + } + + private function createResponseFactoryFromDeprecatedCallable( + ContainerInterface $container + ): ResponseFactoryInterface { + if (! $container->has(ResponseInterface::class)) { + throw MissingDependencyException::dependencyForService( + ResponseInterface::class, + MethodNotAllowedMiddleware::class + ); + } + + /** @var callable():ResponseInterface $responseFactory */ + $responseFactory = $container->get(ResponseInterface::class); + + return new CallableResponseFactoryDecorator($responseFactory); + } + + private function doesConfigurationProvidesDedicatedResponseFactory(ContainerInterface $container): bool + { + if (! $container->has('config')) { + return false; + } + + $config = $container->get('config'); + Assert::isArrayAccessible($config); + $dependencies = $config['dependencies'] ?? []; + Assert::isMap($dependencies); + + $delegators = $dependencies['delegators'] ?? []; + $aliases = $dependencies['aliases'] ?? []; + Assert::isArrayAccessible($delegators); + Assert::isArrayAccessible($aliases); + + if (isset($delegators[ResponseInterface::class]) || isset($aliases[ResponseInterface::class])) { + // Even tho, aliases could point to a different service, we assume that there is a dedicated factory + // available. The alias resolving is not worth it. + return true; + } + + /** @psalm-suppress MixedAssignment */ + $deprecatedResponseFactory = $dependencies['factories'][ResponseInterface::class] ?? null; + + return $deprecatedResponseFactory !== null + && $deprecatedResponseFactory !== 'Mezzio\Container\ResponseFactoryFactory'; + } +} diff --git a/src/Response/CallableResponseFactoryDecorator.php b/src/Response/CallableResponseFactoryDecorator.php new file mode 100644 index 0000000..1059ce3 --- /dev/null +++ b/src/Response/CallableResponseFactoryDecorator.php @@ -0,0 +1,36 @@ +responseFactory = $responseFactory; + } + + public function createResponse(int $code = 200, string $reasonPhrase = ''): ResponseInterface + { + return $this->getResponseFromCallable()->withStatus($code, $reasonPhrase); + } + + public function getResponseFromCallable(): ResponseInterface + { + return ($this->responseFactory)(); + } +} diff --git a/src/Test/AbstractImplicitMethodsIntegrationTest.php b/src/Test/AbstractImplicitMethodsIntegrationTest.php index b7247a0..3f9e98e 100644 --- a/src/Test/AbstractImplicitMethodsIntegrationTest.php +++ b/src/Test/AbstractImplicitMethodsIntegrationTest.php @@ -9,7 +9,7 @@ use Generator; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequest; -use Laminas\Diactoros\StreamFactory; +use Laminas\Diactoros\Stream; use Laminas\Stratigility\MiddlewarePipe; use Mezzio\Router\Middleware\DispatchMiddleware; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; @@ -22,7 +22,6 @@ use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; -use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -47,7 +46,9 @@ abstract public function getRouter(): RouterInterface; public function getImplicitOptionsMiddleware(?ResponseInterface $response = null): ImplicitOptionsMiddleware { return new ImplicitOptionsMiddleware( - new ResponseFactory(), + function () use ($response): ResponseInterface { + return $response ?? new Response(); + } ); } @@ -55,17 +56,19 @@ public function getImplicitHeadMiddleware(RouterInterface $router): ImplicitHead { return new ImplicitHeadMiddleware( $router, - new StreamFactory(), + function () { + return new Stream('php://temp', 'rw'); + } ); } - public function createInvalidResponseFactory(): ResponseFactoryInterface + /** + * @return callable(): never + */ + public function createInvalidResponseFactory(): callable { - return new class implements ResponseFactoryInterface { - public function createResponse(int $code = 200, string $reasonPhrase = ''): ResponseInterface - { - TestCase::fail('Response generated when it should not have been'); - } + return static function (): ResponseInterface { + self::fail('Response generated when it should not have been'); }; } @@ -235,7 +238,9 @@ public function testWithoutImplicitMiddleware(string $requestMethod, array $allo $pipeline = new MiddlewarePipe(); $pipeline->pipe(new RouteMiddleware($router)); - $pipeline->pipe(new MethodNotAllowedMiddleware(new ResponseFactory($finalResponse))); + $pipeline->pipe(new MethodNotAllowedMiddleware(static function () use ($finalResponse): ResponseInterface { + return $finalResponse; + })); $finalHandler = $this->createMock(RequestHandlerInterface::class); $finalHandler diff --git a/src/Test/ImplicitMethodsIntegrationTest.php b/src/Test/ImplicitMethodsIntegrationTest.php new file mode 100644 index 0000000..4af253b --- /dev/null +++ b/src/Test/ImplicitMethodsIntegrationTest.php @@ -0,0 +1,14 @@ +defaultResponse - ? $this->defaultResponse->withStatus($code, $reasonPhrase) - : (new Response())->withStatus($code, $reasonPhrase); - } -} diff --git a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php index 4573c17..d710ce1 100644 --- a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php +++ b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php @@ -4,7 +4,6 @@ namespace MezzioTest\Router\Middleware; -use Laminas\Diactoros\StreamFactory; use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\Middleware\ImplicitHeadMiddlewareFactory; use Mezzio\Router\RouterInterface; @@ -12,7 +11,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; -use Psr\Http\Message\StreamFactoryInterface; +use Psr\Http\Message\StreamInterface; use function in_array; @@ -51,7 +50,7 @@ public function testFactoryRaisesExceptionIfStreamFactoryServiceIsMissing(): voi ->expects(self::exactly(2)) ->method('has') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); + self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); return true; })) ->willReturnOnConsecutiveCalls(true, false); @@ -64,13 +63,14 @@ public function testFactoryRaisesExceptionIfStreamFactoryServiceIsMissing(): voi public function testFactoryProducesImplicitHeadMiddlewareWhenAllDependenciesPresent(): void { $router = $this->createMock(RouterInterface::class); - $streamFactory = new StreamFactory(); + $streamFactory = static function (): void { + }; $this->container ->expects(self::exactly(2)) ->method('has') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); + self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); return true; })) ->willReturn(true); @@ -79,7 +79,7 @@ public function testFactoryProducesImplicitHeadMiddlewareWhenAllDependenciesPres ->expects(self::exactly(2)) ->method('get') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); + self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); return true; })) ->willReturnOnConsecutiveCalls($router, $streamFactory); diff --git a/test/Middleware/ImplicitHeadMiddlewareTest.php b/test/Middleware/ImplicitHeadMiddlewareTest.php index 5e39dc7..a13afda 100644 --- a/test/Middleware/ImplicitHeadMiddlewareTest.php +++ b/test/Middleware/ImplicitHeadMiddlewareTest.php @@ -7,7 +7,6 @@ use Fig\Http\Message\RequestMethodInterface as RequestMethod; use Laminas\Diactoros\Response\TextResponse; use Laminas\Diactoros\ServerRequest; -use Laminas\Diactoros\StreamFactory; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; @@ -16,6 +15,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\StreamInterface; use Psr\Http\Server\MiddlewareInterface; /** @covers \Mezzio\Router\Middleware\ImplicitHeadMiddleware */ @@ -33,7 +33,7 @@ protected function setUp(): void $this->router = $this->createMock(RouterInterface::class); $this->middleware = new ImplicitHeadMiddleware( $this->router, - new StreamFactory(), + fn (): StreamInterface => $this->createMock(StreamInterface::class), ); } diff --git a/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php b/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php index 4a6c89c..0126d05 100644 --- a/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php +++ b/test/Middleware/ImplicitOptionsMiddlewareFactoryTest.php @@ -6,12 +6,12 @@ use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\Middleware\ImplicitOptionsMiddlewareFactory; -use Mezzio\Router\Test\ResponseFactory; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseFactoryInterface; +use Psr\Http\Message\ResponseInterface; #[CoversClass(ImplicitOptionsMiddlewareFactory::class)] final class ImplicitOptionsMiddlewareFactoryTest extends TestCase @@ -38,17 +38,25 @@ public function testFactoryRaisesExceptionIfResponseFactoryServiceIsMissing(): v public function testFactoryProducesImplicitOptionsMiddlewareWhenAllDependenciesPresent(): void { + $factory = static function (): void { + }; + $this->container - ->expects(self::once()) ->method('has') - ->with(ResponseFactoryInterface::class) - ->willReturn(true); + ->with(self::callback(static function ($arg): bool { + self::assertContains($arg, [ + ResponseFactoryInterface::class, + ResponseInterface::class, + ]); + + return true; + }))->willReturnOnConsecutiveCalls(false, true); $this->container ->expects(self::once()) ->method('get') - ->with(ResponseFactoryInterface::class) - ->willReturn(new ResponseFactory()); + ->with(ResponseInterface::class) + ->willReturn($factory); ($this->factory)($this->container); } diff --git a/test/Middleware/ImplicitOptionsMiddlewareTest.php b/test/Middleware/ImplicitOptionsMiddlewareTest.php index 1b5e02f..7ec06c3 100644 --- a/test/Middleware/ImplicitOptionsMiddlewareTest.php +++ b/test/Middleware/ImplicitOptionsMiddlewareTest.php @@ -5,29 +5,33 @@ namespace MezzioTest\Router\Middleware; use Fig\Http\Message\RequestMethodInterface as RequestMethod; -use Laminas\Diactoros\Response; -use Laminas\Diactoros\ServerRequest; use Mezzio\Router\Middleware\ImplicitOptionsMiddleware; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; -use Mezzio\Router\Test\ResponseFactory; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; +use function implode; + /** @covers \Mezzio\Router\Middleware\ImplicitOptionsMiddleware */ final class ImplicitOptionsMiddlewareTest extends TestCase { + /** @var ResponseInterface&MockObject */ private ResponseInterface $response; + private ImplicitOptionsMiddleware $middleware; protected function setUp(): void { parent::setUp(); - $this->response = new Response(); - $this->middleware = new ImplicitOptionsMiddleware(new ResponseFactory($this->response)); + $this->response = $this->createMock(ResponseInterface::class); + $responseFactory = fn (): ResponseInterface => $this->response; + + $this->middleware = new ImplicitOptionsMiddleware($responseFactory); } public function testNonOptionsRequestInvokesHandler(): void @@ -113,21 +117,34 @@ public function testInjectsAllowHeaderInResponseProvidedToConstructorDuringOptio $result = RouteResult::fromRouteFailure($allowedMethods); - $request = (new ServerRequest()) - ->withMethod(RequestMethod::METHOD_OPTIONS) - ->withAttribute(RouteResult::class, $result); + $request = $this->createMock(ServerRequestInterface::class); + $request + ->method('getMethod') + ->willReturn(RequestMethod::METHOD_OPTIONS); + + $request + ->method('getAttribute') + ->with(RouteResult::class) + ->willReturn($result); $handler = $this->createMock(RequestHandlerInterface::class); $handler ->expects(self::never()) ->method('handle'); - self::assertFalse($this->response->hasHeader('Allow')); + $this->response + ->method('withStatus') + ->willReturnSelf(); + + $this->response + ->expects(self::once()) + ->method('withHeader') + ->with('Allow', implode(',', $allowedMethods)) + ->willReturnSelf(); - $response = $this->middleware->process($request, $handler); + $result = $this->middleware->process($request, $handler); - self::assertNotSame($this->response, $response); - self::assertTrue($response->hasHeader('Allow')); + self::assertSame($this->response, $result); } public function testReturnsResultOfHandlerWhenRouteNotFound(): void diff --git a/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php b/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php index 7814cee..a8005e2 100644 --- a/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php +++ b/test/Middleware/MethodNotAllowedMiddlewareFactoryTest.php @@ -4,20 +4,26 @@ namespace MezzioTest\Router\Middleware; +use Generator; use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\Middleware\MethodNotAllowedMiddlewareFactory; -use Mezzio\Router\Test\ResponseFactory; +use Mezzio\Router\Response\CallableResponseFactoryDecorator; use MezzioTest\Router\InMemoryContainer; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseFactoryInterface; +use Psr\Http\Message\ResponseInterface; + +use function in_array; #[CoversClass(MethodNotAllowedMiddlewareFactory::class)] final class MethodNotAllowedMiddlewareFactoryTest extends TestCase { private static ResponseFactoryInterface&MockObject $responseFactoryMock; + private static ResponseInterface&MockObject $responseMock; private ContainerInterface&MockObject $container; private MethodNotAllowedMiddlewareFactory $factory; @@ -29,15 +35,59 @@ protected function setUp(): void $this->factory = new MethodNotAllowedMiddlewareFactory(); self::$responseFactoryMock = $this->createMock(ResponseFactoryInterface::class); + self::$responseMock = $this->createMock(ResponseInterface::class); + } + + /** + * @psalm-return Generator}> + */ + public static function configurationsWithOverriddenResponseInterfaceFactory(): Generator + { + yield 'default' => [ + [ + 'dependencies' => [ + 'factories' => [ + ResponseInterface::class => function (): ResponseInterface { + return self::$responseMock; + }, + ], + ], + ], + ]; + + yield 'aliased' => [ + [ + 'dependencies' => [ + 'aliases' => [ + ResponseInterface::class => 'CustomResponseInterface', + ], + ], + ], + ]; + + yield 'delegated' => [ + [ + 'dependencies' => [ + 'delegators' => [ + ResponseInterface::class => [ + fn (): ResponseInterface => self::$responseMock, + ], + ], + ], + ], + ]; } public function testFactoryRaisesExceptionIfResponseFactoryServiceIsMissing(): void { $this->container - ->expects(self::once()) ->method('has') - ->with(ResponseFactoryInterface::class) - ->willReturn(false); + ->with(self::callback(static function ($arg): bool { + return in_array($arg, [ + ResponseFactoryInterface::class, + ResponseInterface::class, + ], true); + }))->willReturn(false); $this->expectException(MissingDependencyException::class); @@ -46,28 +96,60 @@ public function testFactoryRaisesExceptionIfResponseFactoryServiceIsMissing(): v public function testFactoryProducesMethodNotAllowedMiddlewareWhenAllDependenciesPresent(): void { + $factory = static function (): void { + }; + $this->container - ->expects(self::once()) + ->expects(self::exactly(2)) ->method('has') - ->with(ResponseFactoryInterface::class) - ->willReturn(true); + ->with(self::callback(static function ($arg): bool { + return in_array($arg, [ + ResponseFactoryInterface::class, + ResponseInterface::class, + ], true); + })) + ->willReturn(false, true); $this->container ->expects(self::once()) ->method('get') - ->with(ResponseFactoryInterface::class) - ->willReturn(new ResponseFactory()); + ->with(ResponseInterface::class) + ->willReturn($factory); ($this->factory)($this->container); } - public function testWillUseResponseFactoryInterfaceFromContainer(): void + public function testWillUseResponseFactoryInterfaceFromContainerWhenApplicationFactoryIsNotOverridden(): void { - $responseFactory = new ResponseFactory(); - $container = new InMemoryContainer(); - $container->set(ResponseFactoryInterface::class, $responseFactory); + $container = new InMemoryContainer(); + $container->set('config', [ + 'dependencies' => [ + 'factories' => [ + ResponseInterface::class => 'Mezzio\Container\ResponseFactoryFactory', + ], + ], + ]); + $container->set(ResponseFactoryInterface::class, self::$responseFactoryMock); $middleware = ($this->factory)($container); - self::assertSame($responseFactory, $middleware->getResponseFactory()); + self::assertSame(self::$responseFactoryMock, $middleware->getResponseFactory()); + } + + /** @param array $config */ + #[DataProvider('configurationsWithOverriddenResponseInterfaceFactory')] + public function testWontUseResponseFactoryInterfaceFromContainerWhenApplicationFactoryIsOverriden( + array $config + ): void { + $container = new InMemoryContainer(); + $container->set('config', $config); + $container->set(ResponseFactoryInterface::class, self::$responseFactoryMock); + $container->set(ResponseInterface::class, static fn (): ResponseInterface => self::$responseMock); + + $middleware = ($this->factory)($container); + $responseFactoryFromGenerator = $middleware->getResponseFactory(); + + self::assertNotSame(self::$responseFactoryMock, $responseFactoryFromGenerator); + self::assertInstanceOf(CallableResponseFactoryDecorator::class, $responseFactoryFromGenerator); + self::assertSame(self::$responseMock, $responseFactoryFromGenerator->getResponseFromCallable()); } } diff --git a/test/Middleware/MethodNotAllowedMiddlewareTest.php b/test/Middleware/MethodNotAllowedMiddlewareTest.php index c98c6d5..e420c1c 100644 --- a/test/Middleware/MethodNotAllowedMiddlewareTest.php +++ b/test/Middleware/MethodNotAllowedMiddlewareTest.php @@ -8,7 +8,6 @@ use Mezzio\Router\Middleware\MethodNotAllowedMiddleware; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; -use Mezzio\Router\Test\ResponseFactory; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; @@ -33,11 +32,12 @@ protected function setUp(): void { parent::setUp(); - $this->handler = $this->createMock(RequestHandlerInterface::class); - $this->request = $this->createMock(ServerRequestInterface::class); - $this->response = $this->createMock(ResponseInterface::class); + $this->handler = $this->createMock(RequestHandlerInterface::class); + $this->request = $this->createMock(ServerRequestInterface::class); + $this->response = $this->createMock(ResponseInterface::class); + $responseFactory = fn (): ResponseInterface => $this->response; - $this->middleware = new MethodNotAllowedMiddleware(new ResponseFactory($this->response)); + $this->middleware = new MethodNotAllowedMiddleware($responseFactory); } public function testDelegatesToHandlerIfNoRouteResultPresentInRequest(): void diff --git a/test/Response/CallableResponseFactoryDecoratorTest.php b/test/Response/CallableResponseFactoryDecoratorTest.php new file mode 100644 index 0000000..7c1319a --- /dev/null +++ b/test/Response/CallableResponseFactoryDecoratorTest.php @@ -0,0 +1,48 @@ +response = $this->createMock(ResponseInterface::class); + $this->factory = new CallableResponseFactoryDecorator(fn (): ResponseInterface => $this->response); + } + + public function testWillPassStatusCodeAndPhraseToCallable(): void + { + $this->response + ->expects(self::once()) + ->method('withStatus') + ->with(500, 'Foo') + ->willReturnSelf(); + + $this->factory->createResponse(500, 'Foo'); + } + + public function testWillReturnSameResponseInstance(): void + { + $this->response + ->expects(self::once()) + ->method('withStatus') + ->willReturnSelf(); + + self::assertSame($this->response, $this->factory->createResponse()); + } +} diff --git a/test/ServiceManagerIntegrationTest.php b/test/ServiceManagerIntegrationTest.php index af13515..2d99b7b 100644 --- a/test/ServiceManagerIntegrationTest.php +++ b/test/ServiceManagerIntegrationTest.php @@ -4,7 +4,6 @@ namespace MezzioTest\Router; -use Laminas\Diactoros\StreamFactory; use Laminas\ServiceManager\ConfigInterface; use Laminas\ServiceManager\ServiceManager; use Mezzio\Router; @@ -12,7 +11,7 @@ use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseFactoryInterface; -use Psr\Http\Message\StreamFactoryInterface; +use Psr\Http\Message\StreamInterface; use function array_merge_recursive; @@ -31,13 +30,13 @@ protected function setUp(): void [ 'factories' => [ ResponseFactoryInterface::class => function (): ResponseFactoryInterface { - return new Router\Test\ResponseFactory(); + return $this->createMock(ResponseFactoryInterface::class); }, Router\RouterInterface::class => function (): Router\RouterInterface { return $this->createMock(Router\RouterInterface::class); }, - StreamFactoryInterface::class => function (): StreamFactoryInterface { - return new StreamFactory(); + StreamInterface::class => function (): callable { + return fn (): StreamInterface => $this->createMock(StreamInterface::class); }, ], ], From b7a1522a8f7a48ad368c51f10f50253bc92fa6ea Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 29 Mar 2023 12:06:25 +0100 Subject: [PATCH 3/6] Add support for `StreamFactoryInterface` and type-hint on `ResponseFactoryInterface` where appropriate Signed-off-by: George Steel --- psalm-baseline.xml | 8 ++-- src/Middleware/ImplicitHeadMiddleware.php | 28 +++++++---- .../ImplicitHeadMiddlewareFactory.php | 16 +++++-- src/Middleware/ImplicitOptionsMiddleware.php | 5 +- src/Middleware/MethodNotAllowedMiddleware.php | 5 +- src/Middleware/Psr17ResponseFactoryTrait.php | 2 +- .../CallableResponseFactoryDecorator.php | 2 +- .../ImplicitHeadMiddlewareFactoryTest.php | 48 ++++++++++++++++--- 8 files changed, 84 insertions(+), 30 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index cb8cfab..007745e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,18 +1,18 @@ - + - $container->get(StreamInterface::class) + get(StreamInterface::class)]]> - $this->services[$id] + services[$id]]]> - new CallableResponseFactoryDecorator(fn (): ResponseInterface => $this->response) + $this->response)]]> diff --git a/src/Middleware/ImplicitHeadMiddleware.php b/src/Middleware/ImplicitHeadMiddleware.php index f82bdf3..8691e33 100644 --- a/src/Middleware/ImplicitHeadMiddleware.php +++ b/src/Middleware/ImplicitHeadMiddleware.php @@ -9,10 +9,13 @@ use Mezzio\Router\RouterInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use function is_callable; + /** * Handle implicit HEAD requests. * @@ -44,19 +47,25 @@ class ImplicitHeadMiddleware implements MiddlewareInterface { public const FORWARDED_HTTP_METHOD_ATTRIBUTE = 'forwarded_http_method'; - /** @var callable(): StreamInterface */ + /** @var callable(): StreamInterface|StreamFactoryInterface */ private $streamFactory; /** - * @param callable(): StreamInterface $streamFactory A factory capable of returning an empty + * @param callable(): StreamInterface|StreamFactoryInterface $streamFactory A factory capable of returning an empty * StreamInterface instance to inject in a response. */ - public function __construct(private RouterInterface $router, callable $streamFactory) + public function __construct(private RouterInterface $router, callable|StreamFactoryInterface $streamFactory) { - // Factory is wrapped in closure in order to enforce return type safety. - $this->streamFactory = function () use ($streamFactory): StreamInterface { - return $streamFactory(); - }; + if (is_callable($streamFactory)) { + // Factory is wrapped in closure in order to enforce return type safety. + $this->streamFactory = function () use ($streamFactory): StreamInterface { + return $streamFactory(); + }; + + return; + } + + $this->streamFactory = $streamFactory; } /** @@ -99,7 +108,10 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface ->withMethod(RequestMethod::METHOD_GET) ); - $body = ($this->streamFactory)(); + $body = $this->streamFactory instanceof StreamFactoryInterface + ? $this->streamFactory->createStream() + : ($this->streamFactory)(); + return $response->withBody($body); } } diff --git a/src/Middleware/ImplicitHeadMiddlewareFactory.php b/src/Middleware/ImplicitHeadMiddlewareFactory.php index ec88dc6..9e5fde0 100644 --- a/src/Middleware/ImplicitHeadMiddlewareFactory.php +++ b/src/Middleware/ImplicitHeadMiddlewareFactory.php @@ -7,6 +7,7 @@ use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\RouterInterface; use Psr\Container\ContainerInterface; +use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; /** @@ -14,10 +15,9 @@ * * This factory depends on two other services: * - * - Mezzio\Router\RouterInterface, which should resolve to an - * instance of that interface. - * - Psr\Http\Message\StreamInterface, which should resolve to a callable - * that will produce an empty Psr\Http\Message\StreamInterface instance. + * - Mezzio\Router\RouterInterface, which should resolve to an instance of that interface. + * - Either Psr\Http\Message\StreamFactoryInterface or Psr\Http\Message\StreamInterface, which should resolve to a + * callable that will produce an empty Psr\Http\Message\StreamInterface instance. * * @final */ @@ -36,6 +36,14 @@ public function __invoke(ContainerInterface $container): ImplicitHeadMiddleware ); } + // Prefer StreamFactoryInterface if present over StreamInterface callable + if ($container->has(StreamFactoryInterface::class)) { + return new ImplicitHeadMiddleware( + $container->get(RouterInterface::class), + $container->get(StreamFactoryInterface::class) + ); + } + if (! $container->has(StreamInterface::class)) { throw MissingDependencyException::dependencyForService( StreamInterface::class, diff --git a/src/Middleware/ImplicitOptionsMiddleware.php b/src/Middleware/ImplicitOptionsMiddleware.php index 20ce511..51f29cc 100644 --- a/src/Middleware/ImplicitOptionsMiddleware.php +++ b/src/Middleware/ImplicitOptionsMiddleware.php @@ -45,15 +45,14 @@ */ class ImplicitOptionsMiddleware implements MiddlewareInterface { - /** @var ResponseFactoryInterface */ - private $responseFactory; + private ResponseFactoryInterface $responseFactory; /** * @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory A factory capable of returning an * empty ResponseInterface instance to return for implicit OPTIONS * requests. */ - public function __construct($responseFactory) + public function __construct(callable|ResponseFactoryInterface $responseFactory) { if (is_callable($responseFactory)) { // Factories are wrapped in a closure in order to enforce return type safety. diff --git a/src/Middleware/MethodNotAllowedMiddleware.php b/src/Middleware/MethodNotAllowedMiddleware.php index b0eb86e..e812450 100644 --- a/src/Middleware/MethodNotAllowedMiddleware.php +++ b/src/Middleware/MethodNotAllowedMiddleware.php @@ -33,13 +33,12 @@ */ class MethodNotAllowedMiddleware implements MiddlewareInterface { - /** @var ResponseFactoryInterface */ - private $responseFactory; + private ResponseFactoryInterface $responseFactory; /** * @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory */ - public function __construct($responseFactory) + public function __construct(callable|ResponseFactoryInterface $responseFactory) { if (is_callable($responseFactory)) { // Factories are wrapped in a closure in order to enforce return type safety. diff --git a/src/Middleware/Psr17ResponseFactoryTrait.php b/src/Middleware/Psr17ResponseFactoryTrait.php index 3ede604..0cd504c 100644 --- a/src/Middleware/Psr17ResponseFactoryTrait.php +++ b/src/Middleware/Psr17ResponseFactoryTrait.php @@ -13,7 +13,7 @@ /** * @internal - * @deprecated Will be removed with v4.0.0 + * @deprecated Will be removed with v5.0.0 */ trait Psr17ResponseFactoryTrait { diff --git a/src/Response/CallableResponseFactoryDecorator.php b/src/Response/CallableResponseFactoryDecorator.php index 1059ce3..39e5dfc 100644 --- a/src/Response/CallableResponseFactoryDecorator.php +++ b/src/Response/CallableResponseFactoryDecorator.php @@ -9,7 +9,7 @@ /** * @internal - * @deprecated Will be removed with v4.0.0 + * @deprecated Will be removed with v5.0.0 */ final class CallableResponseFactoryDecorator implements ResponseFactoryInterface { diff --git a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php index d710ce1..aa35090 100644 --- a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php +++ b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php @@ -4,6 +4,7 @@ namespace MezzioTest\Router\Middleware; +use Laminas\Diactoros\StreamFactory; use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\Middleware\ImplicitHeadMiddlewareFactory; use Mezzio\Router\RouterInterface; @@ -11,6 +12,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; +use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; use function in_array; @@ -47,39 +49,73 @@ public function testFactoryRaisesExceptionIfRouterInterfaceServiceIsMissing(): v public function testFactoryRaisesExceptionIfStreamFactoryServiceIsMissing(): void { $this->container - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('has') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); + self::assertTrue(in_array($arg, [ + RouterInterface::class, + StreamFactoryInterface::class, + StreamInterface::class, + ])); return true; })) - ->willReturnOnConsecutiveCalls(true, false); + ->willReturnOnConsecutiveCalls(true, false, false); $this->expectException(MissingDependencyException::class); ($this->factory)($this->container); } - public function testFactoryProducesImplicitHeadMiddlewareWhenAllDependenciesPresent(): void + public function testFactoryProducesImplicitHeadMiddlewareWithCallableStreamFactory(): void { $router = $this->createMock(RouterInterface::class); $streamFactory = static function (): void { }; $this->container - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('has') + ->with(self::callback(function (string $arg): bool { + self::assertTrue(in_array($arg, [ + RouterInterface::class, + StreamFactoryInterface::class, + StreamInterface::class, + ])); + return true; + })) + ->willReturnOnConsecutiveCalls(true, false, true); + + $this->container + ->expects(self::exactly(2)) + ->method('get') ->with(self::callback(function (string $arg): bool { self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); return true; })) + ->willReturnOnConsecutiveCalls($router, $streamFactory); + + ($this->factory)($this->container); + } + + public function testFactoryProducesImplicitHeadMiddlewareWithStreamFactoryInterface(): void + { + $router = $this->createMock(RouterInterface::class); + $streamFactory = new StreamFactory(); + + $this->container + ->expects(self::exactly(2)) + ->method('has') + ->with(self::callback(function (string $arg): bool { + self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); + return true; + })) ->willReturn(true); $this->container ->expects(self::exactly(2)) ->method('get') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamInterface::class])); + self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); return true; })) ->willReturnOnConsecutiveCalls($router, $streamFactory); From 0b17fbb8d1cfc0844944152ace5b4908a3f6e965 Mon Sep 17 00:00:00 2001 From: George Steel Date: Wed, 29 Mar 2023 12:12:42 +0100 Subject: [PATCH 4/6] Revert change in removal target version Signed-off-by: George Steel --- src/Middleware/Psr17ResponseFactoryTrait.php | 2 +- src/Response/CallableResponseFactoryDecorator.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Middleware/Psr17ResponseFactoryTrait.php b/src/Middleware/Psr17ResponseFactoryTrait.php index 0cd504c..3ede604 100644 --- a/src/Middleware/Psr17ResponseFactoryTrait.php +++ b/src/Middleware/Psr17ResponseFactoryTrait.php @@ -13,7 +13,7 @@ /** * @internal - * @deprecated Will be removed with v5.0.0 + * @deprecated Will be removed with v4.0.0 */ trait Psr17ResponseFactoryTrait { diff --git a/src/Response/CallableResponseFactoryDecorator.php b/src/Response/CallableResponseFactoryDecorator.php index 39e5dfc..1059ce3 100644 --- a/src/Response/CallableResponseFactoryDecorator.php +++ b/src/Response/CallableResponseFactoryDecorator.php @@ -9,7 +9,7 @@ /** * @internal - * @deprecated Will be removed with v5.0.0 + * @deprecated Will be removed with v4.0.0 */ final class CallableResponseFactoryDecorator implements ResponseFactoryInterface { From 08ed660c99c01e014cdab0bf6aab9801295a7acf Mon Sep 17 00:00:00 2001 From: George Steel Date: Fri, 31 Mar 2023 10:10:58 +0100 Subject: [PATCH 5/6] Add a decorator for callable stream factory arguments to ImplicitHeadMiddleware The decorator enables a StreamFactoryInterface type internally whilst preserving BC with callable stream factories. Signed-off-by: George Steel --- psalm-baseline.xml | 5 ++ src/Middleware/ImplicitHeadMiddleware.php | 22 +++---- src/Stream/CallableStreamFactoryDecorator.php | 43 +++++++++++++ .../CallableStreamFactoryDecoratorTest.php | 61 +++++++++++++++++++ 4 files changed, 116 insertions(+), 15 deletions(-) create mode 100644 src/Stream/CallableStreamFactoryDecorator.php create mode 100644 test/Stream/CallableStreamFactoryDecoratorTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 007745e..b3550a7 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,10 @@ + + + new CallableStreamFactoryDecorator($streamFactory) + + get(StreamInterface::class)]]> diff --git a/src/Middleware/ImplicitHeadMiddleware.php b/src/Middleware/ImplicitHeadMiddleware.php index 8691e33..6be238c 100644 --- a/src/Middleware/ImplicitHeadMiddleware.php +++ b/src/Middleware/ImplicitHeadMiddleware.php @@ -7,6 +7,7 @@ use Fig\Http\Message\RequestMethodInterface as RequestMethod; use Mezzio\Router\RouteResult; use Mezzio\Router\RouterInterface; +use Mezzio\Router\Stream\CallableStreamFactoryDecorator; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\StreamFactoryInterface; @@ -47,22 +48,17 @@ class ImplicitHeadMiddleware implements MiddlewareInterface { public const FORWARDED_HTTP_METHOD_ATTRIBUTE = 'forwarded_http_method'; - /** @var callable(): StreamInterface|StreamFactoryInterface */ - private $streamFactory; + private StreamFactoryInterface $streamFactory; /** - * @param callable(): StreamInterface|StreamFactoryInterface $streamFactory A factory capable of returning an empty - * StreamInterface instance to inject in a response. + * @param (callable(): StreamInterface)|StreamFactoryInterface $streamFactory A factory capable of returning + * an empty StreamInterface instance to + * inject in a response. */ public function __construct(private RouterInterface $router, callable|StreamFactoryInterface $streamFactory) { if (is_callable($streamFactory)) { - // Factory is wrapped in closure in order to enforce return type safety. - $this->streamFactory = function () use ($streamFactory): StreamInterface { - return $streamFactory(); - }; - - return; + $streamFactory = new CallableStreamFactoryDecorator($streamFactory); } $this->streamFactory = $streamFactory; @@ -108,10 +104,6 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface ->withMethod(RequestMethod::METHOD_GET) ); - $body = $this->streamFactory instanceof StreamFactoryInterface - ? $this->streamFactory->createStream() - : ($this->streamFactory)(); - - return $response->withBody($body); + return $response->withBody($this->streamFactory->createStream()); } } diff --git a/src/Stream/CallableStreamFactoryDecorator.php b/src/Stream/CallableStreamFactoryDecorator.php new file mode 100644 index 0000000..4db838f --- /dev/null +++ b/src/Stream/CallableStreamFactoryDecorator.php @@ -0,0 +1,43 @@ +streamFactory = $streamFactory; + } + + /** @inheritDoc */ + public function createStream(string $content = ''): StreamInterface + { + return ($this->streamFactory)(); + } + + /** @inheritDoc */ + public function createStreamFromFile(string $filename, string $mode = 'r'): StreamInterface + { + throw new RuntimeException('This method will not be implemented'); + } + + /** @inheritDoc */ + public function createStreamFromResource($resource): StreamInterface + { + throw new RuntimeException('This method will not be implemented'); + } +} diff --git a/test/Stream/CallableStreamFactoryDecoratorTest.php b/test/Stream/CallableStreamFactoryDecoratorTest.php new file mode 100644 index 0000000..674fc2c --- /dev/null +++ b/test/Stream/CallableStreamFactoryDecoratorTest.php @@ -0,0 +1,61 @@ +stream = (new StreamFactory())->createStream(); + + $this->decorator = new CallableStreamFactoryDecorator(fn (): StreamInterface => $this->stream); + } + + public function testThatCreateStreamWillProduceStream(): void + { + self::assertSame($this->stream, $this->decorator->createStream()); + } + + public function testThatTheStreamDoesNotReceiveContentArgument(): void + { + $result = $this->decorator->createStream('some content'); + + self::assertSame('', $result->getContents()); + } + + public function testCreateStreamFromFileIsNotImplemented(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('This method will not be implemented'); + + $this->decorator->createStreamFromFile('/foo'); + } + + public function testCreateStreamFromResourceIsNotImplemented(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('This method will not be implemented'); + + $this->decorator->createStreamFromResource(fopen(__FILE__, 'r')); + } +} From 72d74a0e08123fa2af49cd3736943649d2ef1113 Mon Sep 17 00:00:00 2001 From: George Steel Date: Fri, 31 Mar 2023 10:28:10 +0100 Subject: [PATCH 6/6] Amend factory to preserve existing 3.x series behaviour by preferring the deprecated callable Signed-off-by: George Steel --- psalm-baseline.xml | 6 +++ .../ImplicitHeadMiddlewareFactory.php | 38 +++++++++++++------ .../ImplicitHeadMiddlewareFactoryTest.php | 10 +++-- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index b3550a7..82aa0a8 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6,6 +6,12 @@ + + get(StreamInterface::class))]]> + + + detectStreamFactory + get(StreamInterface::class)]]> diff --git a/src/Middleware/ImplicitHeadMiddlewareFactory.php b/src/Middleware/ImplicitHeadMiddlewareFactory.php index 9e5fde0..55fede3 100644 --- a/src/Middleware/ImplicitHeadMiddlewareFactory.php +++ b/src/Middleware/ImplicitHeadMiddlewareFactory.php @@ -6,6 +6,7 @@ use Mezzio\Router\Exception\MissingDependencyException; use Mezzio\Router\RouterInterface; +use Mezzio\Router\Stream\CallableStreamFactoryDecorator; use Psr\Container\ContainerInterface; use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; @@ -36,24 +37,37 @@ public function __invoke(ContainerInterface $container): ImplicitHeadMiddleware ); } - // Prefer StreamFactoryInterface if present over StreamInterface callable - if ($container->has(StreamFactoryInterface::class)) { - return new ImplicitHeadMiddleware( - $container->get(RouterInterface::class), - $container->get(StreamFactoryInterface::class) - ); - } + return new ImplicitHeadMiddleware( + $container->get(RouterInterface::class), + $this->detectStreamFactory($container), + ); + } - if (! $container->has(StreamInterface::class)) { + /** + * BC Preserving StreamFactoryInterface Retrieval + * + * Preserves existing behaviour in the 3.x series by fetching a `StreamInterface` callable and wrapping it in a + * decorator that implements StreamFactoryInterface. If `StreamInterface` callable is unavailable, attempt to + * fetch a `StreamFactoryInterface`, throwing a MissingDependencyException if neither are found. + * + * @deprecated Will be removed in version 4.0.0 + */ + private function detectStreamFactory(ContainerInterface $container): StreamFactoryInterface + { + $hasStreamFactory = $container->has(StreamFactoryInterface::class); + $hasDeprecatedCallable = $container->has(StreamInterface::class); + + if (! $hasStreamFactory && ! $hasDeprecatedCallable) { throw MissingDependencyException::dependencyForService( StreamInterface::class, ImplicitHeadMiddleware::class ); } - return new ImplicitHeadMiddleware( - $container->get(RouterInterface::class), - $container->get(StreamInterface::class) - ); + if ($hasDeprecatedCallable) { + return new CallableStreamFactoryDecorator($container->get(StreamInterface::class)); + } + + return $container->get(StreamFactoryInterface::class); } } diff --git a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php index aa35090..1a22289 100644 --- a/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php +++ b/test/Middleware/ImplicitHeadMiddlewareFactoryTest.php @@ -103,13 +103,17 @@ public function testFactoryProducesImplicitHeadMiddlewareWithStreamFactoryInterf $streamFactory = new StreamFactory(); $this->container - ->expects(self::exactly(2)) + ->expects(self::exactly(3)) ->method('has') ->with(self::callback(function (string $arg): bool { - self::assertTrue(in_array($arg, [RouterInterface::class, StreamFactoryInterface::class])); + self::assertTrue(in_array($arg, [ + RouterInterface::class, + StreamFactoryInterface::class, + StreamInterface::class, + ])); return true; })) - ->willReturn(true); + ->willReturn(true, true, false); $this->container ->expects(self::exactly(2))