diff --git a/src/Http/Traits/Response/Body.php b/src/Http/Traits/Response/Body.php index 3d21fcba1..e2ff5aad5 100644 --- a/src/Http/Traits/Response/Body.php +++ b/src/Http/Traits/Response/Body.php @@ -66,9 +66,10 @@ public function get(string $key, ?string $default = null) * Sets new key/value pair into response * @param mixed $value */ - public function set(string $key, $value): void + public function set(string $key, $value): self { $this->__response[$key] = $value; + return $this; } /** @@ -94,7 +95,7 @@ public function delete(string $key): void * Prepares the JSON response * @param array|null $data */ - public function json(?array $data = null, ?int $code = null): void + public function json(?array $data = null, ?int $code = null): self { $this->setContentType(ContentType::JSON); @@ -105,13 +106,15 @@ public function json(?array $data = null, ?int $code = null): void if ($data) { $this->__response = array_merge($this->__response, $data); } + + return $this; } /** * Prepares the JSONP response * @param array|null $data */ - public function jsonp(string $callback, ?array $data = null, ?int $code = null): void + public function jsonp(string $callback, ?array $data = null, ?int $code = null): self { $this->setContentType(ContentType::JSONP); @@ -124,6 +127,8 @@ public function jsonp(string $callback, ?array $data = null, ?int $code = null): if ($data) { $this->__response = array_merge($this->__response, $data); } + + return $this; } /** @@ -140,7 +145,7 @@ public function getJsonPData(array $data): string * @param array|null $data * @param string $root */ - public function xml(?array $data = null, $root = '', ?int $code = null): void + public function xml(?array $data = null, $root = '', ?int $code = null): self { $this->setContentType(ContentType::XML); @@ -153,12 +158,14 @@ public function xml(?array $data = null, $root = '', ?int $code = n if ($data) { $this->__response = array_merge($this->__response, $data); } + + return $this; } /** * Prepares the HTML content */ - public function html(string $html, ?int $code = null): void + public function html(string $html, ?int $code = null): self { $this->setContentType(ContentType::HTML); @@ -167,6 +174,8 @@ public function html(string $html, ?int $code = null): void } $this->__response[ReservedKeys::RENDERED_VIEW] = $html; + + return $this; } /** diff --git a/src/Http/Traits/Response/Header.php b/src/Http/Traits/Response/Header.php index d02c57cc1..c0394be0d 100644 --- a/src/Http/Traits/Response/Header.php +++ b/src/Http/Traits/Response/Header.php @@ -50,9 +50,10 @@ public function getHeader(string $key): ?string /** * Sets the response header */ - public function setHeader(string $key, string $value): void + public function setHeader(string $key, string $value): self { $this->__headers[$key] = $value; + return $this; } /** @@ -77,9 +78,9 @@ public function deleteHeader(string $key): void /** * Sets the content type */ - public function setContentType(string $contentType): void + public function setContentType(string $contentType): self { - $this->setHeader('Content-Type', $contentType); + return $this->setHeader('Content-Type', $contentType); } /** diff --git a/src/Http/Traits/Response/Status.php b/src/Http/Traits/Response/Status.php index a4c13b4ad..55a645cae 100644 --- a/src/Http/Traits/Response/Status.php +++ b/src/Http/Traits/Response/Status.php @@ -118,13 +118,15 @@ public function getText(int $code): string /** * Sets the status code */ - public function setStatusCode(int $code): void + public function setStatusCode(int $code): self { if (!isset($this->texts[$code])) { throw new InvalidArgumentException(sprintf('The HTTP status code "%s" is not valid.', $code)); } $this->__statusCode = $code; + + return $this; } /** diff --git a/src/Router/Enums/ExceptionMessages.php b/src/Router/Enums/ExceptionMessages.php index ffc7c4e59..63136c6b4 100644 --- a/src/Router/Enums/ExceptionMessages.php +++ b/src/Router/Enums/ExceptionMessages.php @@ -46,6 +46,8 @@ final class ExceptionMessages extends BaseExceptionMessages public const ACTION_NOT_FOUND_ON_CONTROLLER = 'Action `{%1}` not found on controller `{%2}`'; + public const INVALID_HANDLER_RESPONSE = 'Route handler `{%1}` must return `{%2}`'; + public const ROUTE_NOT_CLOSURE = 'Route is not a closure'; public const NAME_BEFORE_ROUTE_DEFINITION = 'Names can not be set before route definition'; diff --git a/src/Router/Exceptions/RouteException.php b/src/Router/Exceptions/RouteException.php index cd5b97c07..e893a41b5 100644 --- a/src/Router/Exceptions/RouteException.php +++ b/src/Router/Exceptions/RouteException.php @@ -116,6 +116,17 @@ public static function actionNotFound(string $action, string $controller): self ); } + public static function invalidHandlerResponse(string $handler, string $expectedType): self + { + return new self( + _message( + ExceptionMessages::INVALID_HANDLER_RESPONSE, + [$handler, $expectedType] + ), + E_ERROR + ); + } + public static function notClosure(): self { return new self( diff --git a/src/Router/RouteDispatcher.php b/src/Router/RouteDispatcher.php index 45038e549..e1ebd1e2d 100644 --- a/src/Router/RouteDispatcher.php +++ b/src/Router/RouteDispatcher.php @@ -19,10 +19,12 @@ use Quantum\Router\Exceptions\RouteException; use Quantum\Csrf\Exceptions\CsrfException; use Quantum\Di\Exceptions\DiException; +use Quantum\Http\Response; use Quantum\Http\Request; use ReflectionException; use Quantum\Csrf\Csrf; use Quantum\Di\Di; +use Closure; /** * Class RouteDispatcher @@ -34,7 +36,7 @@ final class RouteDispatcher * Dispatch a matched route. * @throws ReflectionException|CsrfException|DiException|RouteException */ - public function dispatch(MatchedRoute $matched, Request $request): void + public function dispatch(MatchedRoute $matched, Request $request): Response { $route = $matched->getRoute(); $params = $matched->getParams(); @@ -46,8 +48,10 @@ public function dispatch(MatchedRoute $matched, Request $request): void throw RouteException::closureHandlerMissing(); } - $this->invoke($closure, $params); - return; + return $this->requireResponse( + $this->invoke($closure, $params), + 'closure route' + ); } $callable = $this->resolveControllerCallable($route); @@ -57,10 +61,14 @@ public function dispatch(MatchedRoute $matched, Request $request): void $this->callHook($controller, '__before', $params); - /** @phpstan-ignore argument.type */ - $this->invoke($callable, $params); + $response = $this->requireResponse( + $this->invoke($callable, $params), + $route->getController() . '::' . $route->getAction() + ); $this->callHook($controller, '__after', $params); + + return $response; } /** @@ -88,15 +96,30 @@ private function resolveControllerCallable(Route $route): array /** * Invoke a callable with parameters resolved via DI autowiring. + * @param callable|array{0: object, 1: string} $callable * @param array $params - * @throws DiException|ReflectionException + * @return mixed */ - private function invoke(callable $callable, array $params): void + private function invoke($callable, array $params) { - call_user_func_array( - $callable, - Di::autowire($callable, $params) - ); + /** @var callable $callable */ + $closure = Closure::fromCallable($callable); + + return $closure(...Di::autowire($closure, $params)); + } + + /** + * Enforce a Response return contract for route handlers. + * @param mixed $result + * @throws RouteException + */ + private function requireResponse($result, string $handler): Response + { + if (!$result instanceof Response) { + throw RouteException::invalidHandlerResponse($handler, Response::class); + } + + return $result; } /** @@ -107,7 +130,6 @@ private function invoke(callable $callable, array $params): void private function callHook(object $controller, string $hook, array $params): void { if (method_exists($controller, $hook)) { - /** @phpstan-ignore argument.type */ $this->invoke([$controller, $hook], $params); } } diff --git a/tests/Unit/Http/Traits/Response/HttpResponseBodyTest.php b/tests/Unit/Http/Traits/Response/HttpResponseBodyTest.php index 829c503a1..1e6a3d129 100644 --- a/tests/Unit/Http/Traits/Response/HttpResponseBodyTest.php +++ b/tests/Unit/Http/Traits/Response/HttpResponseBodyTest.php @@ -24,7 +24,9 @@ public function testResponseSetHasGetAllDelete(): void $this->assertFalse($response->has('name')); - $response->set('name', 'John'); + $returned = $response->set('name', 'John'); + + $this->assertSame($response, $returned); $this->assertTrue($response->has('name')); @@ -49,7 +51,7 @@ public function testResponseJsonContent(): void $response->set('lastname', 'Doe'); - $response->json(); + $this->assertSame($response, $response->json()); $this->assertEquals('{"firstname":"John","lastname":"Doe"}', $response->getContent()); @@ -75,7 +77,7 @@ public function testResponseJsonP(): void $response->set('lastname', 'Doe'); - $response->jsonp('myfunc'); + $this->assertSame($response, $response->jsonp('myfunc')); $this->assertEquals('myfunc({"firstname":"John","lastname":"Doe"})', $response->getContent()); } @@ -88,7 +90,7 @@ public function testResponseXmlContent(): void $response->set('lastname', 'Doe'); - $response->xml(); + $this->assertSame($response, $response->xml()); $xml = "\n" . "\n" . @@ -196,7 +198,7 @@ public function testResponseHtmlContent(): void { $response = response(); - $response->html('
John Doe
'); + $this->assertSame($response, $response->html('
John Doe
')); $this->assertEquals('
John Doe
', $response->getContent()); diff --git a/tests/Unit/Http/Traits/Response/HttpResponseHeaderTest.php b/tests/Unit/Http/Traits/Response/HttpResponseHeaderTest.php index bb1cb0236..0b2eb6fd5 100644 --- a/tests/Unit/Http/Traits/Response/HttpResponseHeaderTest.php +++ b/tests/Unit/Http/Traits/Response/HttpResponseHeaderTest.php @@ -20,7 +20,9 @@ public function testResponseHeaderSetHasGetAllDelete(): void $this->assertFalse($response->hasHeader('X-Frame-Options')); - $response->setHeader('X-Frame-Options', 'deny'); + $returned = $response->setHeader('X-Frame-Options', 'deny'); + + $this->assertSame($response, $returned); $this->assertTrue($response->hasHeader('X-Frame-Options')); diff --git a/tests/Unit/Http/Traits/Response/HttpResponseStatusTest.php b/tests/Unit/Http/Traits/Response/HttpResponseStatusTest.php index a92919dba..0d1a92e1a 100644 --- a/tests/Unit/Http/Traits/Response/HttpResponseStatusTest.php +++ b/tests/Unit/Http/Traits/Response/HttpResponseStatusTest.php @@ -17,7 +17,9 @@ public function testResponseStatus(): void $this->assertEquals(200, $response->getStatusCode()); - $response->setStatusCode(301); + $returned = $response->setStatusCode(301); + + $this->assertSame($response, $returned); $this->assertEquals(301, $response->getStatusCode()); diff --git a/tests/Unit/Router/RouteDispatcherTest.php b/tests/Unit/Router/RouteDispatcherTest.php index 46a5a4a3f..665287b3e 100644 --- a/tests/Unit/Router/RouteDispatcherTest.php +++ b/tests/Unit/Router/RouteDispatcherTest.php @@ -11,6 +11,7 @@ use Quantum\Router\Route; use Quantum\Http\Request; use Quantum\Csrf\Csrf; +use Quantum\Di\Di; use Mockery; class RouteDispatcherTest extends AppTestCase @@ -23,12 +24,15 @@ public function tearDown(): void public function testDispatchExecutesClosureRoute(): void { - $called = false; - $receivedParam = null; + $expectedResponse = response(); - $closure = function (string $id) use (&$called, &$receivedParam): void { - $called = true; - $receivedParam = $id; + $closure = function (string $id, Response $response) use ($expectedResponse): Response { + $this->assertSame('123', $id); + $this->assertSame($expectedResponse, $response); + + return $response->json([ + 'id' => $id, + ]); }; $route = new Route(['GET'], '/test', null, null, $closure); @@ -41,12 +45,10 @@ public function testDispatchExecutesClosureRoute(): void $dispatcher = new RouteDispatcher(); $request = Mockery::mock(Request::class); - Mockery::mock(Response::class); - - $dispatcher->dispatch($matched, $request); + $response = $dispatcher->dispatch($matched, $request); - $this->assertTrue($called); - $this->assertSame('123', $receivedParam); + $this->assertSame($expectedResponse, $response); + $this->assertSame('{"id":"123"}', $response->getContent()); } public function testDispatchThrowsWhenClosureRouteHasNoClosure(): void @@ -70,9 +72,13 @@ public function testDispatchExecutesControllerActionWithParams(): void $controllerClass = new class () { public static ?string $received = null; - public function post(string $uuid): void + public function post(string $uuid, Response $response): Response { self::$received = $uuid; + + return $response->json([ + 'uuid' => $uuid, + ]); } }; @@ -83,15 +89,15 @@ public function post(string $uuid): void $dispatcher = new RouteDispatcher(); $request = Mockery::mock(Request::class); - Mockery::mock(Response::class); - $dispatcher->dispatch($matched, $request); + $response = $dispatcher->dispatch($matched, $request); $this->assertSame( 'abc-123', $controllerClass::$received, 'Controller action did not receive matched parameters' ); + $this->assertSame('{"uuid":"abc-123"}', $response->getContent()); } public function testDispatchCallsControllerHooksInCorrectOrder(): void @@ -104,9 +110,13 @@ public function __before(string $uuid): void self::$calls[] = 'before:' . $uuid; } - public function post(string $uuid): void + public function post(string $uuid, Response $response): Response { self::$calls[] = 'action:' . $uuid; + + return $response->json([ + 'uuid' => $uuid, + ]); } public function __after(string $uuid): void @@ -124,9 +134,7 @@ public function __after(string $uuid): void $request = Mockery::mock(Request::class); $request->shouldReceive('getMethod')->andReturn('POST'); - Mockery::mock(Response::class); - - $dispatcher->dispatch($matched, $request); + $response = $dispatcher->dispatch($matched, $request); $this->assertSame( [ @@ -137,6 +145,42 @@ public function __after(string $uuid): void $controllerClass::$calls, 'Controller hooks were not executed in the correct order' ); + $this->assertSame('{"uuid":"abc"}', $response->getContent()); + } + + public function testDispatchThrowsWhenClosureRouteDoesNotReturnResponse(): void + { + $this->expectException(RouteException::class); + + $route = new Route(['GET'], '/broken', null, null, function (): string { + return 'invalid'; + }); + + $matched = new MatchedRoute($route, []); + + $dispatcher = new RouteDispatcher(); + + $request = Mockery::mock(Request::class); + + $dispatcher->dispatch($matched, $request); + } + + public function testDispatchThrowsWhenControllerActionDoesNotReturnResponse(): void + { + $this->expectException(RouteException::class); + + $controllerClass = new class () { + public function post(): void + { + } + }; + + $route = new Route(['GET'], '/invalid', get_class($controllerClass), 'post'); + $matched = new MatchedRoute($route, []); + $dispatcher = new RouteDispatcher(); + $request = Mockery::mock(Request::class); + + $dispatcher->dispatch($matched, $request); } public function testDispatchThrowsWhenControllerActionDoesNotExist(): void @@ -182,7 +226,12 @@ public function submit(): void ->with(Csrf::TOKEN_KEY) ->andReturn(false); - Mockery::mock(Response::class); + $csrf = Mockery::mock(Csrf::class); + $csrf->shouldReceive('checkToken') + ->with($request) + ->andThrow(new CsrfException('Missing CSRF token')); + + Di::set(Csrf::class, $csrf); $dispatcher = new RouteDispatcher(); diff --git a/tests/_root/modules/Test/Controllers/TestController.php b/tests/_root/modules/Test/Controllers/TestController.php index bf5079b6a..ea88eca5a 100644 --- a/tests/_root/modules/Test/Controllers/TestController.php +++ b/tests/_root/modules/Test/Controllers/TestController.php @@ -2,10 +2,12 @@ namespace Quantum\Tests\_root\modules\Test\Controllers; +use Quantum\Http\Response; + class TestController { - public function tests(): void + public function tests(Response $response): Response { - // + return $response->html(''); } }