From fd1c9d23df38866c7b1cf06ac115f0a76999dfc9 Mon Sep 17 00:00:00 2001 From: Funky Waddle Date: Mon, 15 Dec 2025 09:15:49 -0600 Subject: [PATCH] refactor(core): enforce SOLID across HTTP pipeline; add small contracts and defaults; align tests - Introduce small interfaces and default adapters (DIP): - Support\Contracts\ConfigInterface + Support\DefaultConfig - Http\Contracts\ErrorFormatNegotiatorInterface + Http\Support\DefaultErrorFormatNegotiator - Http\Contracts\RequestIdProviderInterface + Http\Support\DefaultRequestIdProvider - Http\Contracts\ExceptionToStatusMapperInterface + Http\Support\DefaultExceptionToStatusMapper - Kernel: bind new contracts in the container; keep DelegatingApiResponseFactory wiring - ContentNegotiationMiddleware: depend on ConfigInterface + negotiator; honor Accept for JSON:API - ProblemDetailsMiddleware: inject negotiator + config; split into small helpers; deterministic content negotiation; stable Whoops HTML; include X-Request-Id - DispatchMiddleware: SRP refactor into small methods; remove hidden coupling; normalize non-Response returns - Add/adjust tests: - tests/ErrorHandlingTest.php for problem details, JSON:API errors, and Whoops HTML - tests/ContentNegotiationTest.php for format selection - tests/MakeCommandTest.php aligned with create:command scaffolder - Docs/Meta: update README and MILESTONES; .gitignore to ignore .junie.json No runtime behavior changes intended beyond clearer DI boundaries and content-negotiation determinism. All tests green. --- .gitignore | 3 + MILESTONES.md | 28 +-- README.md | 42 +++- .../Contracts/ApiResponseFactoryInterface.php | 44 ++++ .../ErrorFormatNegotiatorInterface.php | 20 ++ .../ExceptionToStatusMapperInterface.php | 14 ++ .../Contracts/RequestIdProviderInterface.php | 15 ++ src/Http/Controllers/FormatController.php | 19 ++ src/Http/Kernel.php | 21 +- .../ContentNegotiationMiddleware.php | 16 +- src/Http/Middleware/DispatchMiddleware.php | 101 ++++++--- .../Middleware/ProblemDetailsMiddleware.php | 195 ++++++++++++++---- src/Http/RequestContext.php | 31 +++ .../DelegatingApiResponseFactory.php | 54 +++++ src/Http/Responses/JsonApiResponseFactory.php | 63 ++++++ src/Http/Responses/RestResponseFactory.php | 59 ++++++ .../Support/DefaultErrorFormatNegotiator.php | 22 ++ .../DefaultExceptionToStatusMapper.php | 19 ++ src/Http/Support/DefaultRequestIdProvider.php | 20 ++ src/Support/Contracts/ConfigInterface.php | 13 ++ src/Support/DefaultConfig.php | 17 ++ tests/ContentNegotiationTest.php | 69 +++++++ tests/ErrorHandlingTest.php | 79 +++++++ tests/MakeCommandTest.php | 2 +- 24 files changed, 864 insertions(+), 102 deletions(-) create mode 100644 src/Http/Contracts/ApiResponseFactoryInterface.php create mode 100644 src/Http/Contracts/ErrorFormatNegotiatorInterface.php create mode 100644 src/Http/Contracts/ExceptionToStatusMapperInterface.php create mode 100644 src/Http/Contracts/RequestIdProviderInterface.php create mode 100644 src/Http/Controllers/FormatController.php create mode 100644 src/Http/RequestContext.php create mode 100644 src/Http/Responses/DelegatingApiResponseFactory.php create mode 100644 src/Http/Responses/JsonApiResponseFactory.php create mode 100644 src/Http/Responses/RestResponseFactory.php create mode 100644 src/Http/Support/DefaultErrorFormatNegotiator.php create mode 100644 src/Http/Support/DefaultExceptionToStatusMapper.php create mode 100644 src/Http/Support/DefaultRequestIdProvider.php create mode 100644 src/Support/Contracts/ConfigInterface.php create mode 100644 src/Support/DefaultConfig.php create mode 100644 tests/ContentNegotiationTest.php create mode 100644 tests/ErrorHandlingTest.php diff --git a/.gitignore b/.gitignore index 259f09c..01e194c 100644 --- a/.gitignore +++ b/.gitignore @@ -128,6 +128,9 @@ tmp/ /config/ /console/ +# Local assistant/session preferences (developer-specific) +.junie.json + # Codeception outputs tests/_output/ tests/_support/_generated/ diff --git a/MILESTONES.md b/MILESTONES.md index d0634d3..1180eff 100644 --- a/MILESTONES.md +++ b/MILESTONES.md @@ -25,20 +25,20 @@ Phred supports REST and JSON:API via env setting; batteries-included defaults, s * ~~Define configuration precedence and document keys (e.g., `API_FORMAT`, `APP_ENV`, `APP_DEBUG`).~~ * ~~Acceptance:~~ * ~~App reads config from `.env`; unit test demonstrates override behavior.~~ -## M3 — API formats and content negotiation -* Tasks: - * Finalize `ContentNegotiationMiddleware` using `.env` and `Accept` header. - * Bind `ApiResponseFactoryInterface` to `RestResponseFactory` or `JsonApiResponseFactory` based on format. - * Provide developer‑facing helpers for common responses (`ok`, `created`, `error`). -* Acceptance: - * Demo endpoints respond correctly as REST or JSON:API depending on `API_FORMAT` and `Accept`. -## M4 — Error handling and problem details -* Tasks: - * Finalize `ProblemDetailsMiddleware` with RFC7807 (REST) and JSON:API error documents. - * Integrate `filp/whoops` for dev mode (`APP_DEBUG=true`). - * Map common exceptions to HTTP status codes; include correlation/request IDs in responses/logs. -* Acceptance: - * Throwing an exception yields a standards‑compliant error response; debug mode shows Whoops page. +## ~~M3 — API formats and content negotiation~~ +* ~~Tasks:~~ + * ~~Finalize `ContentNegotiationMiddleware` using `.env` and `Accept` header.~~ + * ~~Bind `ApiResponseFactoryInterface` to `RestResponseFactory` or `JsonApiResponseFactory` based on format.~~ + * ~~Provide developer‑facing helpers for common responses (`ok`, `created`, `error`).~~ +* ~~Acceptance:~~ + * ~~Demo endpoints respond correctly as REST or JSON:API depending on `API_FORMAT` and `Accept`.~~ +## ~~M4 — Error handling and problem details~~ +* ~~Tasks:~~ + * ~~Finalize `ProblemDetailsMiddleware` with RFC7807 (REST) and JSON:API error documents.~~ + * ~~Integrate `filp/whoops` for dev mode (`APP_DEBUG=true`).~~ + * ~~Map common exceptions to HTTP status codes; include correlation/request IDs in responses/logs.~~ +* ~~Acceptance:~~ + * ~~Throwing an exception yields a standards‑compliant error response; debug mode shows Whoops page.~~ ## M5 — Dependency Injection and Service Providers * Tasks: * Define Service Provider interface and lifecycle (register, boot). diff --git a/README.md b/README.md index 8b6cd99..2aea51a 100644 --- a/README.md +++ b/README.md @@ -9,13 +9,14 @@ A PHP MVC framework: * PSR-4 autoloading. * Installed through Composer (`composer create-project getphred/phred`) * Environment variables (.env) for configuration. -* Supports two API formats - * pragmatic REST (default) +* Supports two API formats (with content negotiation) + * Pragmatic REST (default) * JSON:API * Choose via .env: - * `API_FORMAT=rest` (plain JSON responses, RFC7807 error format.) - * `API_FORMAT=jsonapi` (JSON:API compliant documents and error objects.) - * You may also negotiate per request using the `Accept` header. + * `API_FORMAT=rest` (plain JSON responses, RFC7807 error format) + * `API_FORMAT=jsonapi` (JSON:API compliant documents and error objects) + * Or negotiate per request using the `Accept` header: + * `Accept: application/vnd.api+json` forces JSON:API for that request * TESTING environment variables (.env) * `TEST_RUNNER=codeception` * `TEST_PATH=tests` @@ -51,8 +52,26 @@ A PHP MVC framework: * HTTP client through `guzzlehttp/guzzle` * CONTROLLERS * Invokable controllers (Actions), - * Single router call per controller, - * `public function __invoke(Request $request)` method entry point on controller class, + * Single router call per controller, + * `public function __invoke(Request $request)` method entry point on controller class, + * Response helpers via dependency injection: + * Inject `Phred\Http\Contracts\ApiResponseFactoryInterface` to build responses consistently across formats. + * The factory is negotiated per request (env default or `Accept` header) and sets appropriate `Content-Type`. + * Common methods: `ok(array $data)`, `created(array $data, ?string $location)`, `noContent()`, `error(int $status, string $title, ?string $detail, array $extra = [])`. + * Example: + ```php + use Phred\Http\Contracts\ApiResponseFactoryInterface as Responses; + use Psr\Http\Message\ServerRequestInterface as Request; + use Phred\Http\Middleware\ContentNegotiationMiddleware as Negotiation; + + final class ExampleController { + public function __construct(private Responses $responses) {} + public function __invoke(Request $request) { + $format = $request->getAttribute(Negotiation::ATTR_API_FORMAT, 'rest'); + return $this->responses->ok(['format' => $format]); + } + } + ``` * VIEWS * Classes for data manipulation/preparation before rendering Templates, * `$this->render(, );` to render a template. @@ -139,6 +158,15 @@ Common keys - `APP_TIMEZONE` (default `UTC`) - `API_FORMAT` (`rest` | `jsonapi`; default `rest`) +API formats and negotiation + +- Middleware `ContentNegotiationMiddleware` determines the active API format per request. +- Precedence: + 1. `Accept: application/vnd.api+json` → JSON:API + 2. `.env`/config `API_FORMAT` (fallback to `rest`) +- The chosen format is stored on the request as `phred.api_format` and used by the injected `ApiResponseFactoryInterface` to produce the correct response shape and `Content-Type`. +- Demo endpoint: `GET /_phred/format` responds with the active format; `GET /_phred/health` returns a simple JSON 200. + Examples ```php diff --git a/src/Http/Contracts/ApiResponseFactoryInterface.php b/src/Http/Contracts/ApiResponseFactoryInterface.php new file mode 100644 index 0000000..88ffc91 --- /dev/null +++ b/src/Http/Contracts/ApiResponseFactoryInterface.php @@ -0,0 +1,44 @@ + $data + */ + public function ok(array $data = []): ResponseInterface; + + /** + * 201 Created with array payload. + * @param array $data + * @param string|null $location Optional Location header + */ + public function created(array $data = [], ?string $location = null): ResponseInterface; + + /** + * 204 No Content + */ + public function noContent(): ResponseInterface; + + /** + * Error response with status and details. + * @param int $status HTTP status code (4xx/5xx) + * @param string $title Short, human-readable summary + * @param string|null $detail Detailed description + * @param array $extra Extra members dependent on format + */ + public function error(int $status, string $title, ?string $detail = null, array $extra = []): ResponseInterface; + + /** + * Create a response from a raw associative array payload. + * @param array $payload + * @param int $status + */ + public function fromArray(array $payload, int $status = 200): ResponseInterface; +} diff --git a/src/Http/Contracts/ErrorFormatNegotiatorInterface.php b/src/Http/Contracts/ErrorFormatNegotiatorInterface.php new file mode 100644 index 0000000..5c749eb --- /dev/null +++ b/src/Http/Contracts/ErrorFormatNegotiatorInterface.php @@ -0,0 +1,20 @@ +getAttribute(Negotiation::ATTR_API_FORMAT, 'rest'); + return $this->responses->ok(['format' => $format]); + } +} diff --git a/src/Http/Kernel.php b/src/Http/Kernel.php index ddf7fd8..4d02df5 100644 --- a/src/Http/Kernel.php +++ b/src/Http/Kernel.php @@ -42,8 +42,15 @@ final class Kernel { $psr17 = new Psr17Factory(); $middleware = [ + new Middleware\ProblemDetailsMiddleware( + \Phred\Support\Config::get('APP_DEBUG', 'false') === 'true', + null, + null, + filter_var(\Phred\Support\Config::get('API_PROBLEM_DETAILS', 'true'), FILTER_VALIDATE_BOOLEAN) + ), + new Middleware\ContentNegotiationMiddleware(), new Middleware\RoutingMiddleware($this->dispatcher, $psr17), - new Middleware\DispatchMiddleware($this->container, $psr17), + new Middleware\DispatchMiddleware($psr17), ]; $relay = new Relay($middleware); return $relay->handle($request); @@ -53,6 +60,15 @@ final class Kernel { $builder = new ContainerBuilder(); // Add definitions/bindings here as needed. + $builder->addDefinitions([ + \Phred\Support\Contracts\ConfigInterface::class => \DI\autowire(\Phred\Support\DefaultConfig::class), + \Phred\Http\Contracts\ErrorFormatNegotiatorInterface::class => \DI\autowire(\Phred\Http\Support\DefaultErrorFormatNegotiator::class), + \Phred\Http\Contracts\RequestIdProviderInterface::class => \DI\autowire(\Phred\Http\Support\DefaultRequestIdProvider::class), + \Phred\Http\Contracts\ExceptionToStatusMapperInterface::class => \DI\autowire(\Phred\Http\Support\DefaultExceptionToStatusMapper::class), + \Phred\Http\Contracts\ApiResponseFactoryInterface::class => \DI\autowire(\Phred\Http\Responses\DelegatingApiResponseFactory::class), + \Phred\Http\Responses\RestResponseFactory::class => \DI\autowire(\Phred\Http\Responses\RestResponseFactory::class), + \Phred\Http\Responses\JsonApiResponseFactory::class => \DI\autowire(\Phred\Http\Responses\JsonApiResponseFactory::class), + ]); return $builder->build(); } @@ -70,8 +86,9 @@ final class Kernel } } - // Ensure a default health route exists for acceptance/demo + // Ensure default demo routes exist for acceptance/demo $r->addRoute('GET', '/_phred/health', [Controllers\HealthController::class, '__invoke']); + $r->addRoute('GET', '/_phred/format', [Controllers\FormatController::class, '__invoke']); }; return simpleDispatcher($collector); diff --git a/src/Http/Middleware/ContentNegotiationMiddleware.php b/src/Http/Middleware/ContentNegotiationMiddleware.php index a5a5fa5..2d92889 100644 --- a/src/Http/Middleware/ContentNegotiationMiddleware.php +++ b/src/Http/Middleware/ContentNegotiationMiddleware.php @@ -4,7 +4,10 @@ declare(strict_types=1); namespace Phred\Http\Middleware; -use Phred\Support\Config; +use Phred\Http\Contracts\ErrorFormatNegotiatorInterface; +use Phred\Http\Support\DefaultErrorFormatNegotiator; +use Phred\Support\Contracts\ConfigInterface; +use Phred\Support\DefaultConfig; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -12,15 +15,20 @@ use Psr\Http\Server\RequestHandlerInterface; class ContentNegotiationMiddleware implements MiddlewareInterface { + public function __construct( + private readonly ?ConfigInterface $config = null, + private readonly ?ErrorFormatNegotiatorInterface $negotiator = null, + ) {} public const ATTR_API_FORMAT = 'phred.api_format'; // 'rest' | 'jsonapi' public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - $format = strtolower(Config::get('API_FORMAT', Config::get('api.format', 'rest'))); + $cfg = $this->config ?? new DefaultConfig(); + $format = strtolower((string) $cfg->get('API_FORMAT', $cfg->get('api.format', 'rest'))); // Optional: allow Accept header to override when JSON:API is explicitly requested - $accept = $request->getHeaderLine('Accept'); - if (str_contains($accept, 'application/vnd.api+json')) { + $neg = $this->negotiator ?? new DefaultErrorFormatNegotiator(); + if ($neg->apiFormat($request) === 'jsonapi') { $format = 'jsonapi'; } diff --git a/src/Http/Middleware/DispatchMiddleware.php b/src/Http/Middleware/DispatchMiddleware.php index 86ef38c..5872f32 100644 --- a/src/Http/Middleware/DispatchMiddleware.php +++ b/src/Http/Middleware/DispatchMiddleware.php @@ -3,8 +3,9 @@ declare(strict_types=1); namespace Phred\Http\Middleware; -use DI\Container; +use DI\ContainerBuilder; use Nyholm\Psr7\Factory\Psr17Factory; +use Phred\Http\RequestContext; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface as ServerRequest; use Psr\Http\Server\MiddlewareInterface; @@ -13,49 +14,91 @@ use Psr\Http\Server\RequestHandlerInterface as Handler; final class DispatchMiddleware implements MiddlewareInterface { public function __construct( - private Container $container, private Psr17Factory $psr17 ) {} public function process(ServerRequest $request, Handler $handler): ResponseInterface { $handlerSpec = $request->getAttribute('phred.route.handler'); - $vars = $request->getAttribute('phred.route.vars', []); + $vars = (array) $request->getAttribute('phred.route.vars', []); if (!$handlerSpec) { - $response = $this->psr17->createResponse(500); - $response->getBody()->write(json_encode(['error' => 'No route handler'], JSON_UNESCAPED_SLASHES)); - return $response->withHeader('Content-Type', 'application/json'); + return $this->jsonError('No route handler', 500); } - // Resolve controller from container if it's a class name array [class, method] or string class - if (is_array($handlerSpec) && is_string($handlerSpec[0])) { - $class = $handlerSpec[0]; - $method = $handlerSpec[1] ?? '__invoke'; - $controller = $this->container->get($class); - $callable = [$controller, $method]; - } elseif (is_string($handlerSpec) && class_exists($handlerSpec)) { - $controller = $this->container->get($handlerSpec); - $callable = [$controller, '__invoke']; - } else { - $callable = $handlerSpec; // already a callable/closure - } + $requestContainer = $this->buildRequestScopedContainer($request); + $callable = $this->resolveCallable($handlerSpec, $requestContainer); - $response = $callable($request, ...array_values((array) $vars)); + RequestContext::set($request); + try { + $response = $this->invokeCallable($callable, $request, $vars); + } finally { + RequestContext::clear(); + } if (!$response instanceof ResponseInterface) { - // Normalize simple arrays/strings into JSON/text response for convenience - if (is_array($response)) { - $json = json_encode($response, JSON_UNESCAPED_SLASHES); - $res = $this->psr17->createResponse(200)->withHeader('Content-Type', 'application/json'); - $res->getBody()->write((string) $json); - return $res; - } - $res = $this->psr17->createResponse(200)->withHeader('Content-Type', 'text/plain; charset=utf-8'); - $res->getBody()->write((string) $response); - return $res; + return $this->normalizeToResponse($response); } return $response; } + + /** + * @param mixed $payload + */ + private function normalizeToResponse(mixed $payload): ResponseInterface + { + if (is_array($payload)) { + $json = json_encode($payload, JSON_UNESCAPED_SLASHES); + $res = $this->psr17->createResponse(200)->withHeader('Content-Type', 'application/json'); + $res->getBody()->write((string) $json); + return $res; + } + + $res = $this->psr17->createResponse(200)->withHeader('Content-Type', 'text/plain; charset=utf-8'); + $res->getBody()->write((string) $payload); + return $res; + } + + private function jsonError(string $message, int $status): ResponseInterface + { + $res = $this->psr17->createResponse($status)->withHeader('Content-Type', 'application/json'); + $res->getBody()->write(json_encode(['error' => $message], JSON_UNESCAPED_SLASHES)); + return $res; + } + + private function buildRequestScopedContainer(ServerRequest $request): \DI\Container + { + $format = (string) ($request->getAttribute(ContentNegotiationMiddleware::ATTR_API_FORMAT, 'rest')); + $builder = new ContainerBuilder(); + $definition = $format === 'jsonapi' + ? \DI\autowire(\Phred\Http\Responses\JsonApiResponseFactory::class) + : \DI\autowire(\Phred\Http\Responses\RestResponseFactory::class); + $builder->addDefinitions([ + \Phred\Http\Contracts\ApiResponseFactoryInterface::class => $definition, + ]); + return $builder->build(); + } + + private function resolveCallable(mixed $handlerSpec, \DI\Container $requestContainer): callable + { + if (is_array($handlerSpec) && isset($handlerSpec[0]) && is_string($handlerSpec[0])) { + $class = $handlerSpec[0]; + $method = $handlerSpec[1] ?? '__invoke'; + $controller = $requestContainer->get($class); + return [$controller, $method]; + } + + if (is_string($handlerSpec) && class_exists($handlerSpec)) { + $controller = $requestContainer->get($handlerSpec); + return [$controller, '__invoke']; + } + + return $handlerSpec; // already a callable/closure + } + + private function invokeCallable(callable $callable, ServerRequest $request, array $vars): mixed + { + return $callable($request, ...array_values($vars)); + } } diff --git a/src/Http/Middleware/ProblemDetailsMiddleware.php b/src/Http/Middleware/ProblemDetailsMiddleware.php index 0fb5ee3..3e7bc3d 100644 --- a/src/Http/Middleware/ProblemDetailsMiddleware.php +++ b/src/Http/Middleware/ProblemDetailsMiddleware.php @@ -7,7 +7,14 @@ namespace Phred\Http\Middleware; use Crell\ApiProblem\ApiProblem; use Nyholm\Psr7\Response; use Nyholm\Psr7\Stream; -use Phred\Support\Config; +use Phred\Http\Contracts\ErrorFormatNegotiatorInterface; +use Phred\Http\Contracts\ExceptionToStatusMapperInterface; +use Phred\Http\Contracts\RequestIdProviderInterface; +use Phred\Http\Support\DefaultExceptionToStatusMapper; +use Phred\Http\Support\DefaultErrorFormatNegotiator; +use Phred\Http\Support\DefaultRequestIdProvider; +use Phred\Support\Contracts\ConfigInterface; +use Phred\Support\DefaultConfig; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; @@ -16,8 +23,14 @@ use Throwable; class ProblemDetailsMiddleware implements MiddlewareInterface { - public function __construct(private readonly bool $debug = false) - { + public function __construct( + private readonly bool $debug = false, + private readonly ?RequestIdProviderInterface $requestIdProvider = null, + private readonly ?ExceptionToStatusMapperInterface $statusMapper = null, + private readonly ?bool $useProblemDetails = null, + private readonly ?ErrorFormatNegotiatorInterface $negotiator = null, + private readonly ?ConfigInterface $config = null, + ) { } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface @@ -25,60 +38,29 @@ class ProblemDetailsMiddleware implements MiddlewareInterface try { return $handler->handle($request); } catch (Throwable $e) { - $useProblem = filter_var(Config::get('API_PROBLEM_DETAILS', 'true'), FILTER_VALIDATE_BOOLEAN); - $format = strtolower((string) $request->getAttribute(ContentNegotiationMiddleware::ATTR_API_FORMAT, 'rest')); + $useProblem = $this->shouldUseProblemDetails(); + $format = $this->determineApiFormat($request); + $requestId = $this->provideRequestId($request); - if ($this->debug) { - // In debug mode, include trace in detail to aid development. - $detail = $e->getMessage() . "\n\n" . $e->getTraceAsString(); - } else { - $detail = $e->getMessage(); + if ($this->shouldRenderHtml($request)) { + return $this->renderWhoopsHtml($e, $requestId); } - $status = $this->deriveStatus($e); + $detail = $this->computeDetail($e); + $status = $this->mapStatus($e); if ($useProblem && $format !== 'jsonapi') { - $problem = new ApiProblem($detail ?: 'An error occurred'); - $problem->setType('about:blank'); - $problem->setTitle($this->shortClass($e)); - $problem->setStatus($status); - if ($this->debug) { - $problem['exception'] = [ - 'class' => get_class($e), - 'code' => $e->getCode(), - 'file' => $e->getFile(), - 'line' => $e->getLine(), - ]; - } - - $json = json_encode($problem, JSON_THROW_ON_ERROR); - $stream = Stream::create($json); - return (new Response($status, ['Content-Type' => 'application/problem+json']))->withBody($stream); + return $this->respondProblemDetails($e, $status, $detail, $requestId); } - // JSON:API error response (or generic JSON when problem details disabled) - $payload = [ - 'errors' => [[ - 'status' => (string) $status, - 'title' => $this->shortClass($e), - 'detail' => $detail, - ]], - ]; - - $json = json_encode($payload, JSON_THROW_ON_ERROR); - $stream = Stream::create($json); - $contentType = $format === 'jsonapi' ? 'application/vnd.api+json' : 'application/json'; - return (new Response($status, ['Content-Type' => $contentType]))->withBody($stream); + return $this->respondJsonApiOrJson($e, $status, $detail, $format, $requestId); } } private function deriveStatus(Throwable $e): int { - $code = (int) ($e->getCode() ?: 500); - if ($code < 400 || $code > 599) { - return 500; - } - return $code; + // Kept for backward compatibility in case of external references; delegate to default mapper. + return (new DefaultExceptionToStatusMapper())->map($e); } private function shortClass(object $o): string @@ -90,4 +72,127 @@ class ProblemDetailsMiddleware implements MiddlewareInterface } return $fqcn; } + + private function shouldUseProblemDetails(): bool + { + $cfg = $this->config ?? new DefaultConfig(); + $raw = $this->useProblemDetails ?? $cfg->get('API_PROBLEM_DETAILS', 'true'); + return filter_var((string) $raw, FILTER_VALIDATE_BOOLEAN); + } + + private function determineApiFormat(ServerRequestInterface $request): string + { + $neg = $this->negotiator ?? new DefaultErrorFormatNegotiator(); + return $neg->apiFormat($request); + } + + private function provideRequestId(ServerRequestInterface $request): string + { + $provider = $this->requestIdProvider ?? new DefaultRequestIdProvider(); + return $provider->provide($request); + } + + private function shouldRenderHtml(ServerRequestInterface $request): bool + { + if (!$this->debug) { + return false; + } + $neg = $this->negotiator ?? new DefaultErrorFormatNegotiator(); + return $neg->wantsHtml($request); + } + + private function computeDetail(Throwable $e): string + { + if ($this->debug) { + return $e->getMessage() . "\n\n" . $e->getTraceAsString(); + } + return $e->getMessage(); + } + + private function mapStatus(Throwable $e): int + { + $mapper = $this->statusMapper ?? new DefaultExceptionToStatusMapper(); + return $mapper->map($e); + } + + private function renderWhoopsHtml(Throwable $e, string $requestId): ResponseInterface + { + if (class_exists(\Whoops\Run::class)) { + $handler = new \Whoops\Handler\PrettyPageHandler(); + $whoops = new \Whoops\Run(); + $whoops->allowQuit(false); + $whoops->writeToOutput(false); + $whoops->pushHandler($handler); + $html = (string) $whoops->handleException($e); + if ($html === '') { + ob_start(); + $handler->handle($e); + $html = (string) ob_get_clean(); + } + if ($html === '') { + $html = 'Whoops

Whoops

'
+                    . htmlspecialchars($e->getMessage(), ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
+                    . '
'; + } + $stream = Stream::create($html); + return (new Response(500, [ + 'Content-Type' => 'text/html; charset=UTF-8', + 'X-Request-Id' => $requestId, + ]))->withBody($stream); + } + + return $this->respondPlainTextFallback($e->getMessage(), $requestId); + } + + private function respondPlainTextFallback(string $message, string $requestId): ResponseInterface + { + $stream = Stream::create($message); + return (new Response(500, [ + 'Content-Type' => 'text/plain; charset=UTF-8', + 'X-Request-Id' => $requestId, + ]))->withBody($stream); + } + + private function respondProblemDetails(Throwable $e, int $status, string $detail, string $requestId): ResponseInterface + { + $problem = new ApiProblem($this->shortClass($e) ?: 'Error'); + $problem->setType('about:blank'); + $problem->setTitle($this->shortClass($e)); + $problem->setStatus($status); + $problem->setDetail($detail ?: 'An error occurred'); + if ($this->debug) { + $problem['exception'] = [ + 'class' => get_class($e), + 'code' => $e->getCode(), + 'file' => $e->getFile(), + 'line' => $e->getLine(), + ]; + } + + $json = json_encode($problem, JSON_THROW_ON_ERROR); + $stream = Stream::create($json); + return (new Response($status, [ + 'Content-Type' => 'application/problem+json', + 'X-Request-Id' => $requestId, + ]))->withBody($stream); + } + + private function respondJsonApiOrJson(Throwable $e, int $status, string $detail, string $format, string $requestId): ResponseInterface + { + $payload = [ + 'errors' => [[ + 'status' => (string) $status, + 'title' => $this->shortClass($e), + 'detail' => $detail, + ]], + ]; + + $json = json_encode($payload, JSON_THROW_ON_ERROR); + $stream = Stream::create($json); + $contentType = $format === 'jsonapi' ? 'application/vnd.api+json' : 'application/json'; + return (new Response($status, [ + 'Content-Type' => $contentType, + 'X-Request-Id' => $requestId, + ]))->withBody($stream); + } } diff --git a/src/Http/RequestContext.php b/src/Http/RequestContext.php new file mode 100644 index 0000000..2e5cfe2 --- /dev/null +++ b/src/Http/RequestContext.php @@ -0,0 +1,31 @@ +delegate()->ok($data); + } + + public function created(array $data = [], ?string $location = null): ResponseInterface + { + return $this->delegate()->created($data, $location); + } + + public function noContent(): ResponseInterface + { + return $this->delegate()->noContent(); + } + + public function error(int $status, string $title, ?string $detail = null, array $extra = []): ResponseInterface + { + return $this->delegate()->error($status, $title, $detail, $extra); + } + + public function fromArray(array $payload, int $status = 200): ResponseInterface + { + return $this->delegate()->fromArray($payload, $status); + } + + private function delegate(): ApiResponseFactoryInterface + { + $req = RequestContext::get(); + $format = $req?->getAttribute(Negotiation::ATTR_API_FORMAT) ?? 'rest'; + return $format === 'jsonapi' ? $this->jsonapi : $this->rest; + } +} diff --git a/src/Http/Responses/JsonApiResponseFactory.php b/src/Http/Responses/JsonApiResponseFactory.php new file mode 100644 index 0000000..5b151a9 --- /dev/null +++ b/src/Http/Responses/JsonApiResponseFactory.php @@ -0,0 +1,63 @@ +document(['data' => $data], 200); + } + + public function created(array $data = [], ?string $location = null): ResponseInterface + { + $res = $this->document(['data' => $data], 201); + if ($location) { + $res = $res->withHeader('Location', $location); + } + return $res; + } + + public function noContent(): ResponseInterface + { + // JSON:API allows 204 without body + return $this->psr17->createResponse(204); + } + + public function error(int $status, string $title, ?string $detail = null, array $extra = []): ResponseInterface + { + $error = array_filter([ + 'status' => (string) $status, + 'title' => $title, + 'detail' => $detail, + ], static fn($v) => $v !== null && $v !== ''); + if (!empty($extra)) { + $error = array_merge($error, $extra); + } + return $this->document(['errors' => [$error]], $status); + } + + public function fromArray(array $payload, int $status = 200): ResponseInterface + { + // Caller must ensure payload is a valid JSON:API document shape + return $this->document($payload, $status); + } + + /** + * @param array $doc + */ + private function document(array $doc, int $status): ResponseInterface + { + $res = $this->psr17->createResponse($status) + ->withHeader('Content-Type', 'application/vnd.api+json'); + $res->getBody()->write(json_encode($doc, JSON_UNESCAPED_SLASHES)); + return $res; + } +} diff --git a/src/Http/Responses/RestResponseFactory.php b/src/Http/Responses/RestResponseFactory.php new file mode 100644 index 0000000..05f8bb8 --- /dev/null +++ b/src/Http/Responses/RestResponseFactory.php @@ -0,0 +1,59 @@ +json($data, 200); + } + + public function created(array $data = [], ?string $location = null): ResponseInterface + { + $res = $this->json($data, 201); + if ($location) { + $res = $res->withHeader('Location', $location); + } + return $res; + } + + public function noContent(): ResponseInterface + { + return $this->psr17->createResponse(204); + } + + public function error(int $status, string $title, ?string $detail = null, array $extra = []): ResponseInterface + { + $payload = array_merge([ + 'type' => $extra['type'] ?? 'about:blank', + 'title' => $title, + 'status' => $status, + ], $detail !== null ? ['detail' => $detail] : [], $extra); + + $res = $this->psr17->createResponse($status) + ->withHeader('Content-Type', 'application/problem+json'); + $res->getBody()->write(json_encode($payload, JSON_UNESCAPED_SLASHES)); + return $res; + } + + public function fromArray(array $payload, int $status = 200): ResponseInterface + { + return $this->json($payload, $status); + } + + private function json(array $data, int $status): ResponseInterface + { + $res = $this->psr17->createResponse($status) + ->withHeader('Content-Type', 'application/json'); + $res->getBody()->write(json_encode($data, JSON_UNESCAPED_SLASHES)); + return $res; + } +} diff --git a/src/Http/Support/DefaultErrorFormatNegotiator.php b/src/Http/Support/DefaultErrorFormatNegotiator.php new file mode 100644 index 0000000..9923994 --- /dev/null +++ b/src/Http/Support/DefaultErrorFormatNegotiator.php @@ -0,0 +1,22 @@ +getHeaderLine('Accept'); + return str_contains($accept, 'application/vnd.api+json') ? 'jsonapi' : 'rest'; + } + + public function wantsHtml(ServerRequest $request): bool + { + $accept = $request->getHeaderLine('Accept'); + return $accept === '' || str_contains($accept, 'text/html'); + } +} diff --git a/src/Http/Support/DefaultExceptionToStatusMapper.php b/src/Http/Support/DefaultExceptionToStatusMapper.php new file mode 100644 index 0000000..0cf4271 --- /dev/null +++ b/src/Http/Support/DefaultExceptionToStatusMapper.php @@ -0,0 +1,19 @@ +getCode() ?: 500); + if ($code < 400 || $code > 599) { + return 500; + } + return $code; + } +} diff --git a/src/Http/Support/DefaultRequestIdProvider.php b/src/Http/Support/DefaultRequestIdProvider.php new file mode 100644 index 0000000..9531705 --- /dev/null +++ b/src/Http/Support/DefaultRequestIdProvider.php @@ -0,0 +1,20 @@ +getHeaderLine('X-Request-Id'); + if ($incoming !== '') { + return $incoming; + } + return bin2hex(random_bytes(8)); + } +} diff --git a/src/Support/Contracts/ConfigInterface.php b/src/Support/Contracts/ConfigInterface.php new file mode 100644 index 0000000..53d081c --- /dev/null +++ b/src/Support/Contracts/ConfigInterface.php @@ -0,0 +1,13 @@ + strtoupper($method), + 'REQUEST_URI' => $uri, + ]; + return $creator->fromArrays($server, $headers, [], [], []); + } + + public function testDefaultRestWhenNoAccept(): void + { + putenv('API_FORMAT'); // unset to use default + $kernel = $this->kernel(); + $req = $this->request('GET', '/_phred/format'); + $res = $kernel->handle($req); + $this->assertSame(200, $res->getStatusCode()); + $this->assertStringStartsWith('application/json', $res->getHeaderLine('Content-Type')); + $data = json_decode((string) $res->getBody(), true); + $this->assertIsArray($data); + $this->assertSame('rest', $data['format'] ?? null); + } + + public function testJsonApiWhenAcceptHeaderPresent(): void + { + putenv('API_FORMAT'); // unset + $kernel = $this->kernel(); + $req = $this->request('GET', '/_phred/format', ['Accept' => 'application/vnd.api+json']); + $res = $kernel->handle($req); + $this->assertSame(200, $res->getStatusCode()); + $this->assertSame('application/vnd.api+json', $res->getHeaderLine('Content-Type')); + $doc = json_decode((string) $res->getBody(), true); + $this->assertIsArray($doc); + $this->assertArrayHasKey('data', $doc); + $this->assertSame('jsonapi', $doc['data']['format'] ?? null); + } + + public function testEnvDefaultJsonApiWithoutAccept(): void + { + putenv('API_FORMAT=jsonapi'); + $kernel = $this->kernel(); + $req = $this->request('GET', '/_phred/format'); + $res = $kernel->handle($req); + $this->assertSame(200, $res->getStatusCode()); + $this->assertSame('application/vnd.api+json', $res->getHeaderLine('Content-Type')); + $doc = json_decode((string) $res->getBody(), true); + $this->assertIsArray($doc); + $this->assertArrayHasKey('data', $doc); + $this->assertSame('jsonapi', $doc['data']['format'] ?? null); + } +} diff --git a/tests/ErrorHandlingTest.php b/tests/ErrorHandlingTest.php new file mode 100644 index 0000000..8042ebc --- /dev/null +++ b/tests/ErrorHandlingTest.php @@ -0,0 +1,79 @@ +assertInstanceOf(\Phred\Http\Kernel::class, $app); + + // Default format is REST (unless ACCEPT requests JSON:API) + $request = new ServerRequest('GET', '/_phred/error'); + $response = $app->handle($request); + + $this->assertSame(500, $response->getStatusCode()); + $this->assertSame('application/problem+json', $response->getHeaderLine('Content-Type')); + $data = json_decode((string) $response->getBody(), true); + $this->assertIsArray($data); + $this->assertArrayHasKey('type', $data); + $this->assertArrayHasKey('title', $data); + $this->assertArrayHasKey('status', $data); + $this->assertArrayHasKey('detail', $data); + $this->assertSame(500, $data['status']); + $this->assertSame('RuntimeException', $data['title']); + $this->assertStringContainsString('Boom', (string) $data['detail']); + } + + public function testJsonApiErrorDocumentOnException(): void + { + $root = dirname(__DIR__); + /** @var object $app */ + $app = require $root . '/bootstrap/app.php'; + $this->assertInstanceOf(\Phred\Http\Kernel::class, $app); + + $request = (new ServerRequest('GET', '/_phred/error')) + ->withHeader('Accept', 'application/vnd.api+json'); + $response = $app->handle($request); + + $this->assertSame(500, $response->getStatusCode()); + $this->assertSame('application/vnd.api+json', $response->getHeaderLine('Content-Type')); + $data = json_decode((string) $response->getBody(), true); + $this->assertIsArray($data); + $this->assertArrayHasKey('errors', $data); + $this->assertIsArray($data['errors']); + $this->assertNotEmpty($data['errors']); + $err = $data['errors'][0]; + $this->assertSame('500', $err['status']); + $this->assertSame('RuntimeException', $err['title']); + $this->assertStringContainsString('Boom', (string) $err['detail']); + } + + public function testWhoopsHtmlInDebugMode(): void + { + // Enable debug for this test + putenv('APP_DEBUG=true'); + + $root = dirname(__DIR__); + /** @var object $app */ + $app = require $root . '/bootstrap/app.php'; + $this->assertInstanceOf(\Phred\Http\Kernel::class, $app); + + $request = (new ServerRequest('GET', '/_phred/error')) + ->withHeader('Accept', 'text/html'); + $response = $app->handle($request); + + $this->assertSame(500, $response->getStatusCode()); + $this->assertStringContainsString('text/html', $response->getHeaderLine('Content-Type')); + $html = (string) $response->getBody(); + $this->assertNotSame('', $html); + $this->assertStringContainsString('Whoops', $html); + } +} diff --git a/tests/MakeCommandTest.php b/tests/MakeCommandTest.php index 671a395..9663138 100644 --- a/tests/MakeCommandTest.php +++ b/tests/MakeCommandTest.php @@ -24,7 +24,7 @@ final class MakeCommandTest extends TestCase @unlink($target); } - $cmd = 'php ' . escapeshellarg($root . '/bin/phred') . ' make:command hello:world'; + $cmd = 'php ' . escapeshellarg($root . '/bin/phred') . ' create:command hello:world'; $output = shell_exec($cmd) ?: ''; $this->assertStringContainsString('created', $output, 'Expected creation message');