diff --git a/.agents/skills/README.md b/.agents/skills/README.md new file mode 100644 index 0000000..8a9bd15 --- /dev/null +++ b/.agents/skills/README.md @@ -0,0 +1,36 @@ +# Agent skills for phpro/http-tools + +Skills that teach a coding agent how to build integrations with this library. They live in +`.agents/skills/` (the cross-runtime location: Claude Code, Codex, Copilot CLI and Gemini CLI all +read it, some via a `.claude/skills` symlink which is git-ignored here). + +They are written for the **consumer** of this package — someone integrating a third-party API in +their own application — not for contributors to the library itself. + +## Skills + +| Skill | Use when | +|---|---| +| [`generate-http-api-call`](generate-http-api-call/SKILL.md) | Integrating a whole endpoint: request + response + handler + tests. Orchestrates the four below. | +| [`configure-http-client`](configure-http-client/SKILL.md) | Client and transport setup: base URI, auth, logging, plugins, preset choice. Once per API. | +| [`generate-http-request`](generate-http-request/SKILL.md) | Writing a `RequestInterface` model: URI templates, parameters, body. | +| [`generate-http-response`](generate-http-response/SKILL.md) | Turning a decoded payload into a strictly typed value object. | +| [`generate-http-request-handler`](generate-http-request-handler/SKILL.md) | The class that runs one call, and where error handling belongs. | +| [`test-http-integration`](test-http-integration/SKILL.md) | Mock client vs VCR cassettes, and what each layer should assert. | + +Each is usable on its own — ask for just a response model and you get just that skill. + +## The conventions they encode + +1. **One vertical slice per endpoint**, grouped per endpoint on disk — not `Model/`, `Request/`, `RequestHandler/` folders. +2. **A handler interface for every endpoint**, so consumers mock one call instead of an API client. +3. **Strictly typed response models, validated once** at the boundary. Never a stored raw array with `?? null` accessors. `psl/type` is the recommended validator, not a requirement. +4. **Client and transport configured once per API**, in a factory the tests reuse — so the tested plugin stack is the production one. +5. **Error handling at the layer that owns it**: plugin → transport decorator → handler, in that order of preference. +6. **Handler tests run through the real transport** against a recorded cassette; mock clients are for plugins, encoders and error paths. + +## Keeping them honest + +The examples use an imaginary "Crumbs Bakery" API (`App\Infrastructure\Bakery`, `GET /orders/{orderId}`) +throughout, so snippets compose across skills. Every library API they reference exists on this branch — +when the library changes, update the skills alongside it. diff --git a/.agents/skills/configure-http-client/SKILL.md b/.agents/skills/configure-http-client/SKILL.md new file mode 100644 index 0000000..166890e --- /dev/null +++ b/.agents/skills/configure-http-client/SKILL.md @@ -0,0 +1,214 @@ +--- +name: configure-http-client +description: Use when setting up or changing the HTTP client and transport for a phpro/http-tools integration — base URI, authentication headers, logging with sensitive data stripped, retries, plugin order, choosing a transport preset or encoder/decoder, or making the same wiring reusable from tests. Triggers on "configure the client", "add a plugin", "set the base url", "how do I log requests", "which preset should I use", "add auth to the API client". +--- + +# Configure an HTTP client and transport + +## Overview + +Every integration gets three small classes, written once and reused by **all** its request handlers *and its tests*: + +| Class | Responsibility | +|---|---| +| `…ClientConfig` | The variable inputs: base URI, credentials, logger. A readonly DTO. | +| `…ClientFactory` | Config → PSR-18 `ClientInterface`, via `ClientBuilder` plugins. | +| `…TransportFactory` | Client → `TransportInterface`, via a preset + any decorators. | + +**Core principle: the same factory the application uses must be usable from a test.** If a test has to rebuild the plugin stack by hand, the two will drift and your tests will pass against a client that doesn't exist in production. So: take the seams as constructor arguments (logger, recorder, base URI) rather than reaching for globals inside. + +## When to Use + +- Starting a new API integration (step 0 of `generate-http-api-call`). +- Adding authentication, logging, retries or a header to an existing integration. +- Choosing between `JsonPreset`, `RawPreset` and friends, or composing a custom encoder/decoder. + +## The three classes + +```php +addBaseUri($config->apiUri) + ->addHeaders(['X-Bakery-Key' => $config->apiKey]) + ->addLogger( + $config->logger, + FormatterBuilder::default() + ->withDebug($config->debug) + ->withMaxBodyLength(1000) + ->addDecorator(RemoveSensitiveHeadersFormatter::createDecorator([ + 'X-Bakery-Key', + ])) + ->build(), + ) + ->build(); + } +} +``` + +```php + + */ + public function create(?ClientInterface $client = null): TransportInterface + { + return new BakeryErrorHandlingTransport( + JsonPreset::create( + $client ?? BakeryClientFactory::create($this->config), + new TemplatedUriBuilder(), + ), + ); + } +} +``` + +The optional `?ClientInterface $client` argument is the seam that makes this factory usable from a test: production passes nothing, a test passes a mock or recording client and still gets the real preset and the real error-handling decorator. See `test-http-integration`. + +`ClientBuilder::default()` already installs `ErrorPlugin`, so 4xx/5xx throw `ClientErrorException`/`ServerErrorException`. Use the bare constructor `new ClientBuilder()` only when you deliberately want to inspect error responses yourself. + +## Plugin order matters + +`ClientBuilder` orders plugins by priority, highest first, so you don't have to think about array order: + +| Constant | Value | For | +|---|---|---| +| `ClientBuilder::PRIORITY_LEVEL_LOGGING` | 2000 | logging, recording — runs outermost, sees the final request | +| `ClientBuilder::PRIORITY_LEVEL_SECURITY` | 1000 | authentication | +| `ClientBuilder::PRIORITY_LEVEL_DEFAULT` | 0 | everything else | + +`addLogger()` and `addRecording()` default to logging priority; `addAuthentication()` defaults to security. Only pass an explicit `priority:` when you need something between the levels — for instance a plugin that must run *before* authentication signs the request. + +Getting this wrong is subtle: a logger at default priority logs the request *before* the auth plugin adds its header, so your logs and your cassettes won't match what went over the wire. + +## Builder methods + +| Method | Adds | +|---|---| +| `addBaseUri($uri, replaceHost: true)` | `BaseUriPlugin` — request models keep relative paths | +| `addHeaders(['X-Key' => '…'])` | `HeaderSetPlugin` — static headers, API keys | +| `addAuthentication($auth)` | `AuthenticationPlugin` — any `Http\Message\Authentication` (`BasicAuth`, `Bearer`, `Header`, `QueryParam`) | +| `addLogger($logger, $formatter)` | `LoggerPlugin` | +| `addRecording($namingStrategy, $recorder)` | VCR record + replay — for tests, see `test-http-integration` | +| `addPlugin($plugin, priority: …)` | any HTTPlug plugin | +| `addCallback(fn, priority: …)` | promotes a closure to a plugin, no class needed | +| `addPluginWithCurrentlyConfiguredClient(fn)` | a plugin that needs to call the API itself (OAuth token fetch) | +| `addDecorator(fn(ClientInterface): ClientInterface)` | wraps the client itself, not a plugin | + +Plugin catalogue, custom plugins, and the OAuth-token-refresh pattern: [references/plugins.md](references/plugins.md). + +**Before writing a plugin, check whether HTTPlug already has one** — retry, redirect, cookies, caching, decoding, history and more are [already available](http://docs.php-http.org/en/latest/plugins/). + +## Choosing a transport + +Presets pair an encoder with a decoder. Pick by what goes over the wire, not by what your models look like. + +| Preset | `TransportInterface<…>` | Use for | +|---|---|---| +| `JsonPreset` | `` | JSON APIs — the common case | +| `FormUrlencodedPreset` | `` | `application/x-www-form-urlencoded` submissions | +| `RawPreset` | `` | XML, CSV, plain text | +| `PsrPreset` | `` | you need headers/status in the handler | +| `BinaryDownloadPreset::withEmptyRequest` | `` | downloads | +| `BinaryDownloadPreset::withMultiPartRequest` | `` | uploads returning a file | + +Mixing encodings (JSON out, raw in; multipart out, JSON in) means composing your own — the full encoder/decoder matrix is in [references/transports.md](references/transports.md). + +Pair a preset with `new TemplatedUriBuilder()` unless your request models return finished URIs, in which case use `RawUriBuilder::createWithAutodiscoveredPsrFactories()`. + +## Transport decorators + +API-wide behaviour that isn't HTTP-level belongs in a decorator around the preset, composed once in the transport factory: error envelopes returned under `200`, `application/problem+json` translation, unwrapping a `{"data": …}` envelope before handlers see it. + +A decorator implements `TransportInterface` and delegates — see `generate-http-request-handler` for a worked example. Handlers stay unchanged, and every endpoint inherits the behaviour. + +## Symfony wiring + +```yaml +services: + App\Infrastructure\Bakery\BakeryClientConfig: + arguments: + $apiUri: '%env(APP_BAKERY_API_URI)%' + $apiKey: '%env(APP_BAKERY_API_KEY)%' + $logger: '@monolog.logger.bakery' + $debug: '%kernel.debug%' + + App\Infrastructure\Bakery\BakeryTransportFactory: ~ + + bakery.transport: + class: Phpro\HttpTools\Transport\TransportInterface + factory: ['@App\Infrastructure\Bakery\BakeryTransportFactory', 'create'] + + App\Infrastructure\Bakery\Order\: + resource: '../src/Infrastructure/Bakery/Order/*/*RequestHandler.php' + arguments: ['@bakery.transport'] +``` + +Credentials come from env vars, never from a constant or a committed config file. More on framework integration: `docs/framework/symfony.md`. + +## Common Mistakes + +| Mistake | Fix | +|---|---| +| Tests rebuild the plugin stack by hand | Give the factory an optional `?ClientInterface` and reuse it. | +| A second transport factory for the same API | One per API; the plugin stacks will otherwise drift. | +| Base URI hardcoded in request models' `uri()` | `addBaseUri()` on the client; keep `uri()` relative. | +| API key logged in plaintext | `RemoveSensitiveHeadersFormatter` / `RemoveSensitiveJsonKeysFormatter` / `RemoveSensitiveQueryStringsFormatter` decorators on the formatter. | +| Logger added at default priority | Leave it at `PRIORITY_LEVEL_LOGGING` so it sees the fully-built request. | +| `new ClientBuilder()` then surprised 404s don't throw | Use `ClientBuilder::default()` for `ErrorPlugin`. | +| Custom plugin for something HTTPlug ships | Check the plugin list first. | +| Retry logic in a handler | `RetryPlugin` on the client. | +| Client built inline inside a request handler | Handlers receive a `TransportInterface`, nothing else. | diff --git a/.agents/skills/configure-http-client/references/plugins.md b/.agents/skills/configure-http-client/references/plugins.md new file mode 100644 index 0000000..d58dfb5 --- /dev/null +++ b/.agents/skills/configure-http-client/references/plugins.md @@ -0,0 +1,187 @@ +# Plugins reference + +A plugin is HTTPlug middleware: it sees the PSR-7 request on the way out and the response on the way back. Everything cross-cutting belongs here — authentication, logging, retries, headers — never in a request handler. + +## Check before you build + +HTTPlug already ships most of what you need. [Full list](http://docs.php-http.org/en/latest/plugins/) — the ones that come up in practice: + +| Plugin | Namespace `Http\Client\Common\Plugin\` | Notes | +|---|---|---| +| `ErrorPlugin` | | 4xx → `ClientErrorException`, 5xx → `ServerErrorException`. In `ClientBuilder::default()`. | +| `BaseUriPlugin` | | Added by `addBaseUri()`. | +| `HeaderSetPlugin` | | Added by `addHeaders()`. Also `HeaderDefaultsPlugin`, `HeaderAppendPlugin`, `HeaderRemovePlugin`. | +| `AuthenticationPlugin` | | Added by `addAuthentication()`. | +| `RetryPlugin` | | `['retries' => 3]`. Retries on exceptions by default. | +| `RedirectPlugin` | | Follows 3xx. | +| `CookiePlugin` | | Needs a `CookieJar`. | +| `DecoderPlugin` | | Transparent gzip/deflate. | +| `HistoryPlugin` | | Records requests via a journal — handy in tests. | +| `ContentLengthPlugin`, `ContentTypePlugin` | | Fill in the obvious headers. | +| `CachePlugin` | | Needs a PSR-6 pool. | +| `LoggerPlugin` | `Http\Client\Common\Plugin\LoggerPlugin` | Added by `addLogger()`. | +| `RecordPlugin` / `ReplayPlugin` | `Http\Client\Plugin\Vcr\` | Added by `addRecording()`. See `test-http-integration`. | + +From this library: + +| Plugin | Purpose | +|---|---| +| `Phpro\HttpTools\Plugin\AcceptLanguagePlugin` | Sets `Accept-Language`. | +| `Phpro\HttpTools\Plugin\CallbackPlugin` | Promotes a closure into a plugin. | + +## Authentication + +`Http\Message\Authentication` implementations, passed to `addAuthentication()`: + +```php +use Http\Message\Authentication\BasicAuth; +use Http\Message\Authentication\Bearer; +use Http\Message\Authentication\Header; +use Http\Message\Authentication\QueryParam; +use Http\Message\Authentication\Chain; +use Http\Message\Authentication\Matching; +use Http\Message\Authentication\RequestConditional; +use Http\Message\Authentication\Wsse; + +ClientBuilder::default() + ->addAuthentication(new BasicAuth($config->username, $config->password)) + ->addAuthentication(new Bearer($config->token)) + ->addAuthentication(new Header('X-Bakery-Key', $config->apiKey)) + ->addAuthentication(new QueryParam(['api_key' => $config->apiKey])) + ->build(); +``` + +A single static header is equally fine via `addHeaders(['X-Bakery-Key' => …])`. Prefer `addAuthentication()` when the value is a credential — it lands at `PRIORITY_LEVEL_SECURITY`, which keeps it inside the logging plugin and makes the intent obvious. + +### Per-request credentials + +When the credential depends on runtime state (the current user, a tenant), build the plugin from a service rather than a config value: + +```php +final readonly class RemoteUserPluginFactory +{ + public static function create(CurrentUserLoader $currentUserLoader): Plugin\AuthenticationPlugin + { + return new Plugin\AuthenticationPlugin( + new Authentication\Header('X-Remote-User', $currentUserLoader->load()->username()), + ); + } +} +``` + +The factory is then also usable from a test with a stubbed loader — the whole reason it is a factory and not an inline `new`. + +### Tokens fetched from the API itself + +`addPluginWithCurrentlyConfiguredClient()` hands you the client as configured *so far*, so a token-fetch plugin can call the API without a circular dependency: + +```php +ClientBuilder::default() + ->addBaseUri($config->apiUri) + ->addPluginWithCurrentlyConfiguredClient( + static fn (ClientInterface $client): Plugin => new OAuthTokenPlugin( + new FetchTokenRequestHandler( + JsonPreset::create($client, new TemplatedUriBuilder()), + ), + $tokenCache, + ), + priority: ClientBuilder::PRIORITY_LEVEL_SECURITY, + ) + ->build(); +``` + +Cache the token. Without a cache this fetches one per request. + +## Custom plugins + +Only when nothing existing fits. For anything small, `addCallback()` avoids a class entirely: + +```php +$builder->addCallback( + static fn (RequestInterface $request, callable $next, callable $first): Promise + => $next($request->withHeader('X-Correlation-Id', $correlationId)), +); +``` + +The three arguments are HTTPlug's: `$next` continues down the chain, `$first` restarts it from the top (used by redirect and retry plugins). + +A full class when the logic warrants one: + +```php +withHeader('X-Tenant', $this->tenantContext->current()->id), + )->then(static function (ResponseInterface $response): ResponseInterface { + // Inspect or rewrite the response here if needed. + return $response; + }); + } +} +``` + +`handleRequest()` must return the promise from `$next()`/`$first()`. Returning a response directly, or forgetting to return at all, breaks the chain in ways that are hard to debug. + +## Priorities + +```php +ClientBuilder::PRIORITY_LEVEL_LOGGING // 2000 — outermost +ClientBuilder::PRIORITY_LEVEL_SECURITY // 1000 +ClientBuilder::PRIORITY_LEVEL_DEFAULT // 0 — innermost +``` + +Higher priority runs earlier on the way out, so it sees the request *before* lower-priority plugins modify it — and it is the last to see the response on the way back. + +Consequences worth internalising: + +- Logging and recording sit at 2000 so they capture the request as it will actually be sent, including auth headers (which the sensitive-header formatter then strips from the log). +- A plugin that must inspect the *signed* request needs a priority **below** security, not above. +- Between-level ordering is what the explicit `priority:` argument is for: `priority: 1500` runs after logging but before authentication. + +## Logging without leaking secrets + +`FormatterBuilder` composes the formatter that `LoggerPlugin` uses: + +```php +FormatterBuilder::default() + ->withDebug($config->debug) // false: one line per request; true: full headers + body + ->withMaxBodyLength(1000) + ->addDecorator(RemoveSensitiveHeadersFormatter::createDecorator([ + 'X-Bakery-Key', + 'Authorization', + ])) + ->addDecorator(RemoveSensitiveJsonKeysFormatter::createDecorator([ + 'password', + 'refreshToken', + ])) + ->addDecorator(RemoveSensitiveQueryStringsFormatter::createDecorator([ + 'api_key', + ])) + ->build(); +``` + +Add the decorator for every credential the integration handles, in whichever place it travels — header, JSON body, query string. Debug logging that dumps an API key into your log aggregator is a security incident, not a debugging convenience. + +Each decorator is also usable directly as a constructor-wrapping formatter (`new RemoveSensitiveHeadersFormatter($inner, [...])`); `createDecorator()` exists so it composes in the builder. + +## Decorators vs plugins + +`addDecorator(fn (ClientInterface $client): ClientInterface)` wraps the *client object*, not the request chain. Use it when you need to replace or wrap the client itself — a fiber-aware client, an in-memory fake, instrumentation that isn't request-shaped. For anything that reads or rewrites requests and responses, use a plugin. diff --git a/.agents/skills/configure-http-client/references/transports.md b/.agents/skills/configure-http-client/references/transports.md new file mode 100644 index 0000000..7a28d3f --- /dev/null +++ b/.agents/skills/configure-http-client/references/transports.md @@ -0,0 +1,105 @@ +# Transports, encoders and decoders + +A transport turns a request model into a PSR-7 request, sends it, and turns the response back into data. It is built from three independent choices: + +1. a **URI builder** — how `uri()` + `uriParameters()` become a URI, +2. an **encoder** — what the request body looks like on the wire, +3. a **decoder** — what the handler receives back. + +`TransportInterface` = `, DecoderInterface>`. The two halves are chosen separately, which is the point: JSON out and a binary file back is a perfectly ordinary combination. + +## URI builders + +| Builder | Behaviour | +|---|---| +| `new TemplatedUriBuilder()` | Expands RFC 6570 templates: `/orders/{id}`, `/orders{?status,page}`. **Default choice.** | +| `new TemplatedUriBuilder(['version' => 'v2'])` | Same, with default variables shared by every request (`/{version}/orders`). | +| `RawUriBuilder::createWithAutodiscoveredPsrFactories()` | Takes `uri()` verbatim. For finished URIs — pagination links returned by the API, for instance. | + +## Encoders + +| Encoder | `EncoderInterface<…>` | Wire format | +|---|---|---| +| `JsonEncoder` | `array\|null` | `application/json`; `null` → empty body | +| `FormUrlencodedEncoder` | `array\|null` | `application/x-www-form-urlencoded` | +| `RawEncoder` | `string` | body as-is, no content type | +| `EmptyBodyEncoder` | `null` | no body | +| `StreamEncoder` | `StreamInterface` | PSR-7 stream as body | +| `ResourceStreamEncoder` | `ResourceStream` | `phpro/resource-stream`, for large payloads | +| `MultiPartEncoder` | `MultiPart` | `multipart/form-data` via `symfony/mime` | +| `ContentTypeAwareEncoder` | `ContentTypeAwarePayload` | wraps another encoder and overrides `Content-Type` | + +## Decoders + +| Decoder | `DecoderInterface<…>` | Produces | +|---|---|---| +| `JsonDecoder` | `array` | decoded JSON; `[]` for an empty body | +| `FormUrlencodedDecoder` | `array` | parsed form body | +| `RawDecoder` | `string` | body as a string | +| `StreamDecoder` | `StreamInterface` | the PSR-7 stream, unread | +| `ResourceStreamDecoder` | `ResourceStream` | a resource stream | +| `ResponseDecoder` | `ResponseInterface` | the whole PSR-7 response | +| `BinaryFileDecoder` | `BinaryFile` | stream + size, mime type, filename, extension, hash | + +All of them have `::createWithAutodiscoveredPsrFactories()`, which is what you want unless you are injecting specific PSR-17 factories. + +Decoders that hand back a stream (`StreamDecoder`, `ResourceStreamDecoder`, `BinaryFileDecoder`) do **not** buffer the body. Don't read the stream twice, and close it when you're done — see `docs/files.md`. + +## Composing a custom transport + +When no preset matches: + +```php +use Phpro\HttpTools\Encoding\Json\JsonEncoder; +use Phpro\HttpTools\Encoding\Binary\BinaryFileDecoder; +use Phpro\HttpTools\Transport\EncodedTransportFactory; +use Phpro\HttpTools\Uri\TemplatedUriBuilder; + +// POST a JSON filter, get a generated PDF back. +$transport = EncodedTransportFactory::create( + $client, + new TemplatedUriBuilder(), + JsonEncoder::createWithAutodiscoveredPsrFactories(), + BinaryFileDecoder::createWithAutodiscoveredPsrFactories(), +); +// TransportInterface +``` + +If you compose the same combination twice, promote it to a small preset class of your own next to the transport factory. + +## Per-request content types + +`ContentTypeAwareEncoder` wraps another encoder so the request model decides the content type — for APIs that version through the media type: + +```php +$encoder = new ContentTypeAwareEncoder( + JsonEncoder::createWithAutodiscoveredPsrFactories(), +); +// RequestInterface> +// body(): new ContentTypeAwarePayload('application/vnd.bakery.v2+json', ['loaves' => 3]) +``` + +## SerializerTransport + +An alternative to encoder/decoder pairs: hand serialization to `symfony/serializer` (or any serializer behind `Phpro\HttpTools\Serializer\SerializerInterface`) and deserialize directly into typed objects. + +```php +use Phpro\HttpTools\Serializer\SymfonySerializer; +use Phpro\HttpTools\Transport\Presets\RawPreset; +use Phpro\HttpTools\Transport\Serializer\SerializerTransport; + +$transport = (new SerializerTransport( + new SymfonySerializer($symfonySerializer, 'json'), + RawPreset::create($client, new TemplatedUriBuilder()), +))->withOutputType(Order::class); +``` + +This replaces the response model's `::parse()` — the serializer builds the object. Worth it when the project already standardises on `symfony/serializer`; otherwise a `::type()` per model is more explicit about what the API is allowed to send. See `generate-http-response` under "Without psl/type". + +## Async + +Transports are synchronous by signature and fiber-transparent in practice. Give the client a fiber-based PSR-18 implementation and run handlers under `React\Async\parallel()` — no change to request models, response models, handlers or transports. See the async section of `README.md`. + +## SDK tools + +`Phpro\HttpTools\Sdk\HttpResource` plus the `Sdk\Rest\*Trait` classes compose a generic multi-endpoint client. It exists for cases where a handler per endpoint is genuinely overkill — a thin passthrough SDK, a spike. Request handlers remain the recommended approach, because they are what let consumers depend on one call instead of forty. See `docs/sdk.md`. diff --git a/.agents/skills/generate-http-api-call/SKILL.md b/.agents/skills/generate-http-api-call/SKILL.md new file mode 100644 index 0000000..a356f77 --- /dev/null +++ b/.agents/skills/generate-http-api-call/SKILL.md @@ -0,0 +1,207 @@ +--- +name: generate-http-api-call +description: Use when integrating a new third-party HTTP/REST/JSON API endpoint with phpro/http-tools — "add a call to endpoint X", "consume the /orders API", "wire up this API in our app" — and you need the whole slice (request model, response model, request handler, transport wiring, tests) rather than a single file. +--- + +# Generate an HTTP API call + +## Overview + +One endpoint = one vertical slice. A slice is four things, in this order: + +1. **Request model** — a `RequestInterface` value object: method, URI template, URI params, body. +2. **Response model** — a value object that *parses* the raw decoded payload via `psl/type`. +3. **Request handler** — `handle(Request): Response`, wrapping a `TransportInterface`. Plus an interface for it. +4. **Tests** — unit tests for parsing, an integration test for the handler against a recorded cassette. + +The transport and the PSR-18 client are configured **once per integration**, not per endpoint. + +**Core principle: never trust the API.** Raw arrays never escape the slice. The handler returns a typed model or throws. + +## When to Use + +- Adding any call to an external API in an app that uses `phpro/http-tools`. +- Extending an existing integration with another endpoint. + +**Not for:** building the client/transport itself (use `configure-http-client`), or generic multi-endpoint SDK wrappers (see `docs/sdk.md` in this repo — request handlers are the preferred approach). + +## Do it in order + +Each step is its own skill. Use them when you only need one piece. + +| Step | Skill | Produces | +|------|-------|----------| +| 0 | `configure-http-client` | `…ClientConfig`, `…ClientFactory`, `…TransportFactory` — **once per API**, skip if it exists | +| 1 | `generate-http-request` | `FetchOrderRequest` | +| 2 | `generate-http-response` | `Order`, `Customer`, `OrderStatus` | +| 3 | `generate-http-request-handler` | `FetchOrderRequestHandlerInterface` + `FetchOrderRequestHandler` | +| 4 | `test-http-integration` | unit + integration tests | + +Do not skip step 0's check: if `…TransportFactory` already exists, reuse it. Two transport factories for one API means the plugins drift apart. + +## Directory layout + +Group per API, then per resource, then per endpoint. Models shared between endpoints live at resource level. + +``` +src/Infrastructure/Bakery/ +├── BakeryClientConfig.php # step 0 +├── BakeryClientFactory.php # step 0 +├── BakeryTransportFactory.php # step 0 +└── Order/ + ├── Customer.php # shared response model + ├── Order.php # shared response model + ├── OrderStatus.php # shared enum + ├── FetchOrder/ + │ ├── FetchOrderRequest.php + │ ├── FetchOrderRequestHandler.php + │ └── FetchOrderRequestHandlerInterface.php + └── PlaceOrder/ + ├── PlaceOrderRequest.php + ├── PlaceOrderRequestHandler.php + └── PlaceOrderRequestHandlerInterface.php + +tests/ +├── Unit/Infrastructure/Bakery/Order/OrderTest.php +└── Integration/Infrastructure/Bakery/ + ├── BakeryWebserviceTestCase.php + └── Order/FetchOrder/FetchOrderRequestHandlerTest.php +``` + +One handler per endpoint. A single class exposing ten endpoints forces consumers to depend on nine calls they don't make. + +## Worked example + +Imaginary "Crumbs Bakery" API (snippets below omit `use` statements — the per-step skills show complete files): `GET /orders/{orderId}` returns +`{"id":"…","status":"baking","loaves":3,"customer":{"name":"…","email":"…"}}`. + +```php +// 1. Request — the outgoing side. +/** + * @psalm-immutable + * + * @template-implements RequestInterface + */ +final readonly class FetchOrderRequest implements RequestInterface +{ + public function __construct( + public string $orderId, + ) { + } + + public function method(): string + { + return 'GET'; + } + + public function uri(): string + { + return '/orders/{orderId}'; + } + + public function uriParameters(): array + { + return ['orderId' => $this->orderId]; + } + + public function body(): null + { + return null; + } +} +``` + +```php +// 2. Response — the incoming side. ::type() describes the payload, ::parse() enforces it. +final readonly class Order +{ + public function __construct( + public string $id, + public OrderStatus $status, + public int $loaves, + public Customer $customer, + ) { + } + + public static function parse(mixed $data): self + { + return self::type()->coerce($data); + } + + /** + * @return TypeInterface + */ + public static function type(): TypeInterface + { + return converted( + shape([ + 'id' => non_empty_string(), + 'status' => backed_enum(OrderStatus::class), + 'loaves' => int(), + 'customer' => Customer::type(), + ]), + instance_of(self::class), + static fn (array $data): self => new self( + $data['id'], + $data['status'], + $data['loaves'], + $data['customer'], + ), + ); + } +} +``` + +```php +// 3. Handler — glues both sides through the transport. +final readonly class FetchOrderRequestHandler implements FetchOrderRequestHandlerInterface +{ + /** + * @param TransportInterface $transport + */ + public function __construct( + private TransportInterface $transport, + ) { + } + + public function handle(FetchOrderRequest $request): Order + { + return Order::parse( + ($this->transport)($request), + ); + } +} +``` + +The `TransportInterface` generics must match the request's `BodyType` and the decoder's output. `null` here because `FetchOrderRequest implements RequestInterface`; `array` because `JsonPreset` decodes to `array`. + +## Wiring it up + +The handler takes a transport, so the container binds the transport factory's output: + +```yaml +services: + App\Infrastructure\Bakery\BakeryClientConfig: + arguments: + $apiUri: '%env(APP_BAKERY_API_URI)%' + $apiKey: '%env(APP_BAKERY_API_KEY)%' + + bakery.transport: + class: Phpro\HttpTools\Transport\TransportInterface + factory: ['@App\Infrastructure\Bakery\BakeryTransportFactory', 'create'] + + App\Infrastructure\Bakery\Order\FetchOrder\FetchOrderRequestHandler: + arguments: ['@bakery.transport'] +``` + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| Handler returns `array` | Return a parsed value object. Raw payloads must not leave the slice. | +| Response model with getters over `$this->data['x'] ?? null` | Parse once in `::type()`; the constructor gets typed properties. | +| One handler class per API instead of per endpoint | Split it — consumers should depend only on the call they make. | +| Request model built from a raw array | Take typed constructor args (or a named constructor from a command/DTO). | +| Transport generics left as bare `TransportInterface` | Annotate `TransportInterface`; psalm catches real mismatches. | +| Duplicate client/transport wiring per endpoint | One `…TransportFactory` per API, injected everywhere. | +| String literals for statuses | Backed enum + `backed_enum()` in the shape. | diff --git a/.agents/skills/generate-http-request-handler/SKILL.md b/.agents/skills/generate-http-request-handler/SKILL.md new file mode 100644 index 0000000..01f8c2f --- /dev/null +++ b/.agents/skills/generate-http-request-handler/SKILL.md @@ -0,0 +1,220 @@ +--- +name: generate-http-request-handler +description: Use when writing the class that executes one API call in a phpro/http-tools integration — a request handler taking a TransportInterface and returning a typed response model — or when deciding where API error handling, retries and exception translation belong. Triggers on "add a request handler", "call the API from my service", "where do I catch HTTP errors", "translate API errors to domain exceptions". +--- + +# Generate an HTTP request handler + +## Overview + +A request handler is the seam between your application and one API endpoint. It does three things: + +```php +public function handle(FetchOrderRequest $request): Order +{ + return Order::parse(($this->transport)($request)); +} +``` + +1. Takes a request model. +2. Runs it through the injected `TransportInterface`. +3. Returns a typed response model — or throws a meaningful exception. + +**One handler per endpoint.** Application services depend on the handler *interface*, so they mock one call rather than a whole API client. + +## When to Use + +- Wiring a request model + response model into something callable. +- Deciding where to put error handling for a specific endpoint. + +Part of the `generate-http-api-call` slice. + +## Always ship an interface + +The handler always comes as a pair: a one-method interface and a `final readonly` implementation. + +The interface is not ceremony. It is what makes the endpoint mockable in the tests of every service that calls it, without those tests knowing that HTTP exists. + +```php + $transport + */ + public function __construct( + private TransportInterface $transport, + ) { + } + + public function handle(FetchOrderRequest $request): Order + { + return Order::parse( + ($this->transport)($request), + ); + } +} +``` + +`handle()` (rather than `__invoke()`) keeps the class readable when a handler grows a second, related method — but `__invoke()` is fine if the project prefers it. Be consistent within an integration. + +## Getting the transport generics right + +`TransportInterface`: + +- `RequestBody` = the `BodyType` of the request models this handler passes in — `null` for a bodyless GET, `array` for a JSON POST. +- `DecodedResponse` = whatever the transport's decoder produces — `array` for `JsonPreset`, `string` for `RawPreset`, `BinaryFile` for `BinaryDownloadPreset`. + +Mismatched generics are the one thing psalm reliably catches here, so annotate them. + +## Where error handling goes + +Push each concern to the layer that owns it. Duplicating error handling per handler is the most common design mistake in these integrations. + +| Concern | Layer | How | +|---|---|---| +| 4xx/5xx must throw | **client plugin** | `Http\Client\Common\Plugin\ErrorPlugin` — already in `ClientBuilder::default()` | +| Retry on timeout | **client plugin** | `Http\Client\Common\Plugin\RetryPlugin` | +| Authentication, base URI, headers, logging | **client plugin** | see `configure-http-client` | +| API-wide error envelope (`200 {"isError":true}`), problem+json → exception | **transport decorator** | wrap the preset once per API | +| Payload validation | **response model** | `::parse()` — see `generate-http-response` | +| *This endpoint's* semantics: 404 means "no such order", not a crash | **request handler** | catch and translate, as below | + +So the handler catches only what is specific to this endpoint: + +```php +public function handle(FetchOrderRequest $request): Order +{ + try { + return Order::parse(($this->transport)($request)); + } catch (ClientErrorException $e) { + if (404 === $e->getResponse()->getStatusCode()) { + throw OrderNotFound::withId($request->orderId, $e); + } + + throw $e; + } +} +``` + +`ClientErrorException` and `ServerErrorException` come from `Http\Client\Common\Exception` and are thrown by `ErrorPlugin`. Always pass the original exception as `$previous` — the response body is usually the only clue about what the API actually objected to. + +If you find yourself writing the same `catch` in three handlers, it belongs in a transport decorator instead. + +### Transport decorator for API-wide behaviour + +A decorator is just another `TransportInterface`, so handlers stay unchanged: + +```php +/** + * @template RequestType + * + * @implements TransportInterface + */ +final readonly class BakeryErrorHandlingTransport implements TransportInterface +{ + /** + * @param TransportInterface $transport + */ + public function __construct( + private TransportInterface $transport, + ) { + } + + public function __invoke(RequestInterface $request): array + { + $response = ($this->transport)($request); + + if (BakeryError::type()->matches($response)) { + throw BakeryApiException::fromError(BakeryError::parse($response)); + } + + return $response; + } +} +``` + +Compose it in the transport factory — see `configure-http-client` — so every handler inherits it for free. + +## Responses with no body + +A `204 No Content` endpoint has nothing to parse. Return `void` rather than inventing an empty model: + +```php +interface CancelOrderRequestHandlerInterface +{ + public function handle(CancelOrderRequest $request): void; +} +``` + +```php +public function handle(CancelOrderRequest $request): void +{ + ($this->transport)($request); +} +``` + +Yes, the body looks pointless. It is still the right place for the endpoint's error translation, and consumers still get a mockable seam. + +## Calling it from the application + +Application code depends on the interface, never on the transport or the client: + +```php +final readonly class NotifyCustomerWhenReady +{ + public function __construct( + private FetchOrderRequestHandlerInterface $fetchOrder, + private Mailer $mailer, + ) { + } + + public function __invoke(string $orderId): void + { + $order = $this->fetchOrder->handle(new FetchOrderRequest($orderId)); + + if (OrderStatus::Ready === $order->status) { + $this->mailer->send(new OrderReadyMail($order->customer, $order)); + } + } +} +``` + +That test needs one stub and no HTTP at all. + +## Common Mistakes + +| Mistake | Fix | +|---|---| +| No interface, services depend on the concrete class | Ship the one-method interface; bind it in the container. | +| Handler returns `array` | Return a parsed response model, or `void`. | +| One `BakeryClient` class with ten public methods | One handler per endpoint. | +| `ClientInterface`/`ClientBuilder` injected into the handler | Inject `TransportInterface`. Client construction is the factory's job. | +| Same `catch (ClientErrorException)` in every handler | Move it to a transport decorator or a client plugin. | +| Catching `Throwable` and returning `null` | Translate to a named domain exception, or let it bubble. | +| Discarding the original exception when translating | Pass it as `$previous`; the response body lives there. | +| Bare `TransportInterface` type annotation | Annotate `TransportInterface`. | +| Retry loop written inside the handler | `RetryPlugin` on the client. | diff --git a/.agents/skills/generate-http-request/SKILL.md b/.agents/skills/generate-http-request/SKILL.md new file mode 100644 index 0000000..9dd5527 --- /dev/null +++ b/.agents/skills/generate-http-request/SKILL.md @@ -0,0 +1,184 @@ +--- +name: generate-http-request +description: Use when writing or reviewing a phpro/http-tools request model — a class implementing Phpro\HttpTools\Request\RequestInterface — including choosing the URI template, mapping query/path parameters, shaping the request body, or deciding the BodyType generic. Triggers on "add a request model", "how do I pass query parameters", "POST body for this endpoint". +--- + +# Generate an HTTP request model + +## Overview + +A request model is an immutable value object describing **one** outgoing call. It answers four questions and nothing else — no HTTP client, no headers, no serialization: + +```php +interface RequestInterface // @template BodyType +{ + public function method(): string; // 'GET'|'POST'|'PUT'|'PATCH'|'DELETE'|'OPTIONS'|'HEAD' + public function uri(): string; // RFC 6570 URI template, path only + public function uriParameters(): array;// variables for the template + public function body(); // BodyType — what the encoder receives +} +``` + +The `BodyType` generic is what the transport's **encoder** consumes. Get it wrong and psalm complains at the handler. + +## When to Use + +- Adding a call to an external API endpoint. +- A call needs query parameters, path parameters, or a payload. + +Part of the `generate-http-api-call` slice. The matching handler is `generate-http-request-handler`. + +## BodyType per transport + +| Transport preset | `RequestInterface<…>` | `body()` returns | +|---|---|---| +| `JsonPreset` | `array` or `null` | associative array, or `null` for no body | +| `FormUrlencodedPreset` | `array` or `null` | flat associative array | +| `RawPreset` / `PsrPreset` | `string` | raw string | +| `BinaryDownloadPreset::withEmptyRequest` | `null` | `null` | +| `BinaryDownloadPreset::withMultiPartRequest` | `MultiPart` | `FormDataPart` | +| Custom `EncodedTransportFactory` | whatever the encoder accepts | idem | + +For a GET with no payload under `JsonPreset`, use `RequestInterface` and `body(): null` — `JsonEncoder` writes an empty body for `null` instead of `"null"`. + +## Complete example + +`GET /orders{?status,page}` on the imaginary Crumbs Bakery API: + +```php + + */ +final readonly class ListOrdersRequest implements RequestInterface +{ + private function __construct( + private ?OrderStatus $status, + private int $page, + ) { + } + + public static function all(int $page = 1): self + { + return new self(null, $page); + } + + public static function withStatus(OrderStatus $status, int $page = 1): self + { + return new self($status, $page); + } + + public function method(): string + { + return 'GET'; + } + + public function uri(): string + { + return '/orders{?status,page}'; + } + + public function uriParameters(): array + { + return [ + 'status' => $this->status?->value, + 'page' => $this->page, + ]; + } + + public function body(): null + { + return null; + } +} +``` + +Named constructors are the point: `ListOrdersRequest::withStatus(OrderStatus::Ready)` reads better at the call site than a nullable positional argument, and each variant documents a real use case. Keep the real constructor private when you have them. + +### A request with a body + +`POST /orders` — the body is a plain array because `JsonPreset` encodes arrays: + +```php +/** + * @psalm-immutable + * + * @template-implements RequestInterface + */ +final readonly class PlaceOrderRequest implements RequestInterface +{ + public function __construct( + private Customer $customer, + private int $loaves, + ) { + } + + public function method(): string + { + return 'POST'; + } + + public function uri(): string + { + return '/orders'; + } + + public function uriParameters(): array + { + return []; + } + + public function body(): array + { + return [ + 'customer' => [ + 'name' => $this->customer->name, + 'email' => $this->customer->email, + ], + 'loaves' => $this->loaves, + ]; + } +} +``` + +`body()` is where domain objects flatten into the wire format. Do the mapping here, explicitly, so the payload is readable in one place. + +## URI templates + +`TemplatedUriBuilder` expands [RFC 6570](https://www.rfc-editor.org/rfc/rfc6570) templates and handles the encoding. + +| Need | Template | Parameters | +|---|---|---| +| Path segment | `/orders/{orderId}` | `['orderId' => 'abc-1']` | +| Query string | `/orders{?status,page}` | `['status' => 'ready', 'page' => 2]` | +| Optional query param | `/orders{?status}` | `['status' => null]` → omitted | +| Repeated query param | `/orders{?ids*}` | `['ids' => ['a', 'b']]` → `?ids=a&ids=b` | +| Reserved chars kept | `/files{+path}` | `['path' => 'a/b.txt']` | + +A `null` parameter is dropped from the expansion, which is what you want for optional filters — no `?status=` noise. + +Use `RawUriBuilder` instead only when the URI is already final and contains no template. Never build the query string by hand with `http_build_query` in `uri()`: you lose the null-dropping and re-encode already-encoded values. + +Paths are relative. The base URI comes from the client (`ClientBuilder::addBaseUri()` — see `configure-http-client`), so `uri()` must not contain a host. + +## Common Mistakes + +| Mistake | Fix | +|---|---| +| Absolute URL in `uri()` | Relative path only; base URI belongs on the client. | +| `'/orders?status='.$status` | Use a template: `'/orders{?status}'`. | +| Constructor takes `array $data` | Take typed arguments, or add a named constructor from your command/DTO. | +| Headers set in the request model | Headers are a client concern (plugins) or an encoder concern (`ContentTypeAwareEncoder`). | +| `RequestInterface` with `body(): null` | Match the generic to what `body()` actually returns. | +| Mutable class with setters | `final readonly` + named constructors; annotate `@psalm-immutable`. | +| Same class serving two endpoints via a flag | One request model per endpoint. | diff --git a/.agents/skills/generate-http-response/SKILL.md b/.agents/skills/generate-http-response/SKILL.md new file mode 100644 index 0000000..8448a5d --- /dev/null +++ b/.agents/skills/generate-http-response/SKILL.md @@ -0,0 +1,267 @@ +--- +name: generate-http-response +description: Use when turning a decoded API payload into a typed value object for a phpro/http-tools integration — writing a response model, parsing JSON into objects, handling optional/nullable/unknown fields, mapping status strings to enums, or validating that an API returned what it promised. Triggers on "parse this response", "response model", "map the JSON to objects", "handle a missing field". +--- + +# Generate an HTTP response model + +## Overview + +**Never trust the API.** A response model converts a decoded payload (usually `array`) into a value object with typed properties, and fails loudly when the payload doesn't match. + +Two rules, in order of importance: + +1. **Non-negotiable:** the model is strictly typed and the payload is validated **once**, at construction. No raw array survives inside the object; no accessor re-guesses a missing key. +2. **Recommended:** do that validation with [`psl/type`](https://github.com/php-standard-library/php-standard-library) (`php-standard-library/type`), which this library itself uses. It is a recommendation, not a requirement — see [Without psl/type](#without-psltype) for equally valid alternatives. Follow whatever the surrounding project already does. + +## When to Use + +- The handler needs to return something other than a raw array (i.e. always). +- Modelling a nested object, a list, an enum, or an optional field from an API payload. + +Part of the `generate-http-api-call` slice. + +## The Iron Rule: parse once, then it's an object + +**No `fromArray()` that stores the array. No getters with `$this->data['x'] ?? null`.** + +That pattern defers every failure to the call site and hides which fields the integration actually depends on. A single parse step states the contract in one place, and the constructor receives values that are already the right type. + +```php +// ✗ WRONG — the array survives, every accessor re-guesses, nothing is typed +final class Order +{ + private function __construct(private array $data) {} + + public static function fromArray(array $data): self { return new self($data); } + + public function status(): ?string { return $this->data['status'] ?? null; } +} + +// ✓ RIGHT — validated at the boundary, typed from then on +final readonly class Order +{ + public function __construct( + public string $id, + public OrderStatus $status, + ) {} + + public static function parse(mixed $data): self { /* validate, then construct */ } +} +``` + +## Recommended: psl/type + +With `psl/type`, every response model exposes exactly two static methods: + +- `::type(): TypeInterface` — declares the payload shape and how to build the object from it. +- `::parse(mixed $data): self` — `self::type()->coerce($data)`. + +Because `::type()` returns a composable type, nested models compose: a parent shape references `Customer::type()` directly, and you never hand-roll recursion. + +```bash +composer require php-standard-library/type +``` + +### Complete example + +`{"id":"…","status":"baking","loaves":3,"customer":{"name":"…","email":"…"},"notes":null}` from the imaginary Crumbs Bakery API. + +```php +coerce($data); + } + + /** + * @return TypeInterface + */ + public static function type(): TypeInterface + { + return converted( + shape([ + 'id' => non_empty_string(), + 'status' => backed_enum(OrderStatus::class), + 'loaves' => int(), + 'customer' => Customer::type(), + 'notes' => optional(nullable(string())), + ]), + instance_of(self::class), + static fn (array $data): self => new self( + $data['id'], + $data['status'], + $data['loaves'], + $data['customer'], + $data['notes'] ?? null, + ), + ); + } +} +``` + +`shape()`'s second argument, `allowUnknownFields`, does **not** control whether an unexpected field breaks parsing — under `coerce()` it never does. It only controls whether unknown keys survive into the coerced array (as `mixed`) or are dropped. Since the converter closure reads only the keys it names, the flag makes no observable difference in this pattern, so leave it at its default. It starts to matter only if you `assert()` instead of `coerce()`, or use `matches()` to pick between payload shapes — see [references/psl-types.md](references/psl-types.md). + +The nested model is a plain sibling with the same two methods: + +```php +final readonly class Customer +{ + public function __construct( + public string $name, + public string $email, + ) { + } + + public static function parse(mixed $data): self + { + return self::type()->coerce($data); + } + + /** + * @return TypeInterface + */ + public static function type(): TypeInterface + { + return converted( + shape([ + 'name' => non_empty_string(), + 'email' => non_empty_string(), + ]), + instance_of(self::class), + static fn (array $data): self => new self($data['name'], $data['email']), + ); + } +} +``` + +And the enum carries the wire values: + +```php +enum OrderStatus: string +{ + case Baking = 'baking'; + case Ready = 'ready'; + case Collected = 'collected'; +} +``` + +### Lists + +A collection response wraps a `vec()` of the item type: + +```php +final readonly class OrderList +{ + /** + * @param list $orders + */ + public function __construct( + public array $orders, + public int $total, + ) { + } + + public static function parse(mixed $data): self + { + return self::type()->coerce($data); + } + + /** + * @return TypeInterface + */ + public static function type(): TypeInterface + { + return converted( + shape([ + 'items' => vec(Order::type()), + 'total' => int(), + ]), + instance_of(self::class), + static fn (array $data): self => new self($data['items'], $data['total']), + ); + } +} +``` + +Note the constructor property is `orders` while the wire key is `items` — the converter closure is exactly where you rename wire vocabulary into your own. + +### psl/type quick reference + +Full catalogue and `coerce` vs `assert` semantics: [references/psl-types.md](references/psl-types.md). + +| Payload | Type | +|---|---| +| `"abc"` | `string()`, `non_empty_string()` | +| `3` / `"3"` | `int()` (coerces numeric strings), `positive_int()` | +| `1.5` | `float()` | +| `true` / `"1"` | `bool()` | +| `"baking"` → enum | `backed_enum(OrderStatus::class)` | +| `null` allowed | `nullable(string())` | +| key may be absent | `optional(string())` | +| absent **or** null | `optional(nullable(string()))` | +| `["a","b"]` | `vec(string())` | +| `{"a":1,"b":2}` | `dict(string(), int())` | +| `{"id":…}` | `shape([...])` | +| either shape | `union(A::type(), B::type())` | +| `"2029-10-08"` → object | `converted(string(), instance_of(Date::class), $fn)` | + +## Without psl/type + +`psl/type` is a suggestion in this library's `composer.json`, not a dependency of your integration. Any approach that satisfies the Iron Rule is fine. Pick the one the project already uses: + +| Approach | How it fits | Notes | +|---|---|---| +| **`symfony/serializer`** | Use `SerializerTransport` + `SymfonySerializer` instead of a preset, and `->withOutputType(Order::class)` in the handler. See `docs/transports.md`. | Denormalizes straight into typed constructors; no `::parse()` needed. | +| **`cuyz/valinor`** | Keep the preset; `::parse()` becomes `$mapper->map(self::class, Source::array($data))`. | Same two-method shape, different engine. Strong on unions and enums. | +| **`webmozart/assert`** | Already a dependency of this library. Assert in a private constructor or named constructor, then assign. | Verbose for nested payloads; fine for flat ones. | +| **Plain PHP** | Named constructor with explicit `match`/`instanceof` checks that throw on anything unexpected. | Acceptable when the payload is two or three scalars. Gets unmaintainable fast. | + +Whatever you choose, keep the public surface identical — `::parse(mixed $data): self` plus typed readonly properties — so handlers and tests don't care which validator is underneath. + +## Empty and error responses + +- `JsonDecoder` returns `[]` for an empty body, so a `204 No Content` reaches you as `[]`. Model that as a request handler returning `void`, not as a response model with all-optional fields. +- HTTP error statuses should never reach `::parse()` — add `Http\Client\Common\Plugin\ErrorPlugin` to the client (`ClientBuilder::default()` already includes it) so 4xx/5xx throw. See `configure-http-client`. +- APIs that signal failure with `200 {"isError": true}` need a transport decorator that inspects the payload and throws, not an `isError` property on the response model. See `configure-http-client`. + +## Common Mistakes + +| Mistake | Fix | +|---|---| +| `fromArray()` storing the raw array | Validate at the boundary; typed constructor properties. | +| Getters with `?? null` fallbacks | Declare optionality once, in the shape/mapping. | +| Status/type as `string` | Backed enum + `backed_enum()` (or your validator's enum support). | +| Nested payload inlined as `dict(string(), mixed())` | Give it its own model with its own `::type()`. | +| `->assert()` on a JSON payload | Use `->coerce()` — JSON gives you `"3"` where you want `3`. | +| Response model with `toArray()` | Outgoing mapping belongs in the request model's `body()`. | +| Swallowing the validation exception and returning `null` | Let it bubble. A payload you can't parse is a real failure. | +| Adding `psl/type` to a project that standardised on another mapper | Follow the project; the Iron Rule is what matters, the tool isn't. | diff --git a/.agents/skills/generate-http-response/references/psl-types.md b/.agents/skills/generate-http-response/references/psl-types.md new file mode 100644 index 0000000..af3a1f2 --- /dev/null +++ b/.agents/skills/generate-http-response/references/psl-types.md @@ -0,0 +1,167 @@ +# psl/type reference for response models + +Package: `php-standard-library/type` (namespace `Psl\Type`). Import the constructors as functions: +`use function Psl\Type\shape;`. + +## coerce vs assert vs matches + +| Method | Behaviour | Throws | Use for | +|---|---|---|---| +| `coerce($value)` | Converts compatible values (`"3"` → `3`, `1` → `true`) | `Psl\Type\Exception\CoercionException` | **Decoded API payloads.** JSON and form encoding blur scalar types. | +| `assert($value)` | Requires an exact type match | `Psl\Type\Exception\AssertException` | Values you produced yourself and want to prove. | +| `matches($value)` | `bool`, no exception | — | Branching on which of two payload shapes you got. | + +Both exceptions implement `Psl\Type\Exception\ExceptionInterface` and extend `Psl\Type\Exception\Exception`. Their message names the failing path (e.g. `customer.email`), which makes them genuinely useful in logs — don't wrap them in a generic "invalid response" exception unless you keep the previous exception. + +Default to `coerce()` in `::parse()`. + +## Scalars + +| Function | Accepts | +|---|---| +| `string()` | strings, and anything `Stringable`/numeric under coercion | +| `non_empty_string()` | as above, rejects `''` | +| `numeric_string()` | `"42"`, `"1.5"` | +| `int()` | ints, numeric strings, `"3"` | +| `positive_int()` | `>= 1` | +| `uint()` | `>= 0` | +| `int_range($min, $max)` | bounded int | +| `float()` / `f64()` | floats, numeric strings | +| `bool()` | `true`/`false`, `"1"`/`"0"`, `1`/`0` | +| `null()` | only `null` | +| `mixed()` | anything — a last resort, it defeats the purpose | +| `uuid()` | UUID-shaped string | + +## Structures + +| Function | Payload | +|---|---| +| `shape(array $elements, bool $allowUnknownFields = false)` | object/associative array with known keys | +| `vec(TypeInterface $t)` | JSON array → `list` | +| `non_empty_vec($t)` | as above, rejects `[]` | +| `dict($keyType, $valueType)` | JSON object with arbitrary keys → `array` | +| `non_empty_dict($k, $v)` | as above, rejects `{}` | +| `set($t)` | unique values | + +#### `allowUnknownFields`, precisely + +An unexpected field in the payload is **not** an error under `coerce()`, whatever the flag says. What the flag changes: + +| | `allowUnknownFields: false` (default) | `true` | +|---|---|---| +| `coerce()` | unknown keys are **dropped** from the result | unknown keys are **kept**, typed `mixed` | +| `assert()` | unknown key throws `AssertException` | kept | +| `matches()` | unknown key → `false` | unknown keys ignored | + +So for the `converted(shape(...), instance_of(...), $fn)` + `coerce()` pattern that response models use, the flag has **no observable effect** — the converter reads only the keys it names, and the dropped-or-kept extras never reach your constructor. Leave it at the default. + +Reach for `allowUnknownFields: true` only when you genuinely consume the passthrough keys (a `dict`-ish payload with a few known keys plus arbitrary extras you want to keep), or when a `matches()` check must tolerate extra fields. Conversely, keep it `false` when `matches()` is your discriminator between two payload shapes and an extra key should mean "not this shape". + +## Optionality + +Three distinct cases — pick deliberately: + +```php +'notes' => string(), // key MUST exist and be a string +'notes' => nullable(string()), // key MUST exist, may be null +'notes' => optional(string()), // key may be absent; if present, a string +'notes' => optional(nullable(string())), // key may be absent OR null +``` + +`optional()` is only meaningful directly inside `shape()`. When a field is optional, read it in the converter with `$data['notes'] ?? null`. + +## Enums + +```php +backed_enum(OrderStatus::class) // "baking" -> OrderStatus::Baking +backed_enum_value(OrderStatus::class) // "baking" -> "baking", validated against the enum +unit_enum(SomeEnum::class) // non-backed enums +enum_case_of(OrderStatus::class, 'Baking') +``` + +An unknown status now fails at the boundary with a clear message, instead of silently flowing through as a string. + +## Composition + +| Function | Use | +|---|---| +| `union($a, $b, ...)` | payload is one of several shapes — order matters, first match wins | +| `intersection($a, $b)` | value must satisfy both | +| `converted($from, $into, Closure $converter)` | validate as `$from`, then map into `$into` | +| `instance_of(SomeClass::class)` | the object type, used as `converted()`'s `$into` | +| `class_string(SomeClass::class)` | a class-string | + +`converted()` is the workhorse for value objects: + +```php +converted( + shape([...]), // what the wire looks like + instance_of(self::class), // what you want + static fn (array $d): self => new self(...), +); +``` + +It also handles scalar-to-object mapping: + +```php +// "2029-10-08" -> DateTimeImmutable +converted( + non_empty_string(), + instance_of(DateTimeImmutable::class), + static fn (string $date): DateTimeImmutable => new DateTimeImmutable($date), +); +``` + +Extract that into a reusable `BakeryDate::type()` when more than one model needs it. + +## Handling either-of payloads + +Some APIs return a success shape or an error shape under `200`. `union()` plus `matches()` keeps it explicit: + +```php +public static function parse(mixed $data): Order +{ + if (BakeryError::type()->matches($data)) { + throw BakeryApiException::fromError(BakeryError::parse($data)); + } + + return Order::parse($data); +} +``` + +Better still, move that decision into a transport decorator so every endpoint inherits it — see `configure-http-client`. + +## Testing types + +`::type()` is a pure function, so parsing tests need no HTTP at all: + +```php +#[Test] +public function it_parses_an_order(): void +{ + $order = Order::parse([ + 'id' => 'ord-1', + 'status' => 'baking', + 'loaves' => 3, + 'customer' => ['name' => 'Jo', 'email' => 'jo@example.com'], + ]); + + self::assertSame(OrderStatus::Baking, $order->status); + self::assertNull($order->notes); +} + +#[Test] +public function it_rejects_an_unknown_status(): void +{ + $this->expectException(CoercionException::class); + + Order::parse([ + 'id' => 'ord-1', + 'status' => 'exploded', + 'loaves' => 3, + 'customer' => ['name' => 'Jo', 'email' => 'jo@example.com'], + ]); +} +``` + +See `test-http-integration` for the rest of the test layers. diff --git a/.agents/skills/test-http-integration/SKILL.md b/.agents/skills/test-http-integration/SKILL.md new file mode 100644 index 0000000..4d56b62 --- /dev/null +++ b/.agents/skills/test-http-integration/SKILL.md @@ -0,0 +1,222 @@ +--- +name: test-http-integration +description: Use when writing or fixing tests for a phpro/http-tools integration — testing request models, response parsing, request handlers, custom plugins or transport decorators — or when deciding between a mock client and VCR cassette recording. Triggers on "test this API call", "how do I mock the HTTP client", "record a cassette", "my VCR test fails offline", "test the request handler". +--- + +# Test an HTTP integration + +## Overview + +Four layers, each with a different tool. Picking the wrong tool is what makes these tests either useless or permanently broken. + +| What you're testing | Tool | Test type | +|---|---|---| +| Request model: method, URI template, body mapping | nothing — plain assertions | unit | +| Response model: payload → typed object, and rejection of bad payloads | nothing — call `::parse()` | unit | +| Custom plugin, encoder/decoder, transport decorator | `UseMockClient` (`php-http/mock-client`) | unit | +| **Request handler**, end to end through the real transport | `addRecording()` / `UseVcrClient` — VCR cassettes | integration | +| Application service that *calls* a handler | stub the handler interface | unit | + +**Core principle: handler tests go through the real transport.** A handler test that mocks `TransportInterface` proves only that you can call a closure — it exercises none of the URI templating, encoding, plugin stack or decoding, which is where integrations actually break. + +## When to Use + +- Step 4 of `generate-http-api-call`. +- A VCR test fails and you need to know whether to re-record. +- Deciding how to test something that talks to an API. + +## Layer 1 — models, no HTTP + +```php +final class ListOrdersRequestTest extends TestCase +{ + #[Test] + public function it_omits_the_status_filter_when_listing_all(): void + { + $request = ListOrdersRequest::all(page: 2); + + self::assertSame('GET', $request->method()); + self::assertSame('/orders{?status,page}', $request->uri()); + self::assertSame(['status' => null, 'page' => 2], $request->uriParameters()); + self::assertNull($request->body()); + } +} +``` + +Response parsing is equally cheap — payload in, object out, plus one test per way the API can lie to you. Examples in `generate-http-response`'s [psl-types reference](../generate-http-response/references/psl-types.md). + +Assert the *template*, not the expanded URI. Expansion is `TemplatedUriBuilder`'s job and it already has tests here. + +## Layer 2 — plugins and transports, mock client + +`UseMockClient` gives you `mockClient()` (an `Http\Mock\Client`) and, through `UseHttpFactories`, `createRequest()` / `createResponse()` / `createStream()` / `createEmptyHttpClientException()`. `UseHttpToolsFactories` adds `createToolsRequest()` for building request models inline. + +```php +final class BakeryTenantPluginTest extends TestCase +{ + use UseMockClient; + use UseHttpToolsFactories; + + #[Test] + public function it_adds_the_tenant_header(): void + { + $client = $this->mockClient(function (Client $client): Client { + $client->setDefaultResponse($this->createResponse(200)); + + return $client; + }); + + $configured = ClientBuilder::default($client) + ->addPlugin(new BakeryTenantPlugin(new FixedTenantContext('crumbs-be'))) + ->build(); + + $configured->sendRequest($this->createRequest('GET', '/orders')); + + self::assertSame('crumbs-be', $client->getLastRequest()->getHeaderLine('X-Tenant')); + } +} +``` + +`mockClient()` without a configurator returns a bare client; add responses with `addResponse()` (queued, in order) or `setDefaultResponse()`. `setDefaultException()` is the honest default for a client that should never be called: + +```php +$this->mockClient(function (Client $client): Client { + $client->setDefaultException(new \Exception('Dont call me!')); + + return $client; +}); +``` + +Then inspect `getLastRequest()` / `getRequests()` to assert what went out. + +Same pattern for a transport decorator: build the real preset over a mock client, queue the payload, assert the decorator's behaviour. + +## Layer 3 — request handlers, VCR cassettes + +VCR records real HTTP traffic to files on first run and replays it forever after. That means one honest round-trip against the real API, then a fast offline test that still exercises your entire stack. + +**Reuse the production transport factory.** This is the whole reason it takes an optional client: + +```php + + */ + protected function createTransport(): TransportInterface + { + $config = new BakeryClientConfig( + apiUri: $_ENV['APP_BAKERY_API_URI'], + apiKey: $_ENV['APP_BAKERY_API_KEY'], + logger: new NullLogger(), + ); + + $recordingClient = ClientBuilder::default() + ->addBaseUri($config->apiUri) + ->addHeaders(['X-Bakery-Key' => $config->apiKey]) + ->addRecording( + new PathNamingStrategy(['hash_headers' => ['X-Bakery-Key']]), + new FilesystemRecorder(FIXTURE_DIR.'/Bakery'), + ) + ->build(); + + return (new BakeryTransportFactory($config))->create($recordingClient); + } +} +``` + +The test itself then reads like application code: + +```php +final class FetchOrderRequestHandlerTest extends BakeryWebserviceTestCase +{ + #[Test] + public function it_fetches_an_order(): void + { + $handler = new FetchOrderRequestHandler($this->createTransport()); + + $order = $handler->handle(new FetchOrderRequest('ord-1')); + + self::assertSame('ord-1', $order->id); + self::assertSame(OrderStatus::Baking, $order->status); + } +} +``` + +`addRecording()` installs both `RecordPlugin` and `ReplayPlugin` at logging priority, so the cassette contains the request as it was actually sent — auth headers included. Alternatively `UseVcrClient::useRecording($path, $namingStrategy)` returns the plugin pair for a client factory that takes a plugin list; it asserts the directory exists, which catches a mistyped fixture path immediately. + +Cassette details, naming strategies and re-recording: [references/vcr.md](references/vcr.md). + +## Layer 4 — consumers stub the interface + +Everything that *uses* a handler depends on `…RequestHandlerInterface`, so its tests need no HTTP at all: + +```php +$fetchOrder = new class implements FetchOrderRequestHandlerInterface { + public function handle(FetchOrderRequest $request): Order + { + return new Order('ord-1', OrderStatus::Ready, 3, new Customer('Jo', 'jo@example.com'), null); + } +}; + +(new NotifyCustomerWhenReady($fetchOrder, $mailer))('ord-1'); +``` + +This is what the handler interface buys you. Without it, every consumer test drags in a transport. + +## Error paths + +Endpoint-specific error translation is worth a test, and a mock client is the right tool — a cassette for a 404 is more trouble than it's worth: + +```php +#[Test] +public function it_throws_when_the_order_does_not_exist(): void +{ + $client = $this->mockClient(function (Client $client): Client { + $client->setDefaultResponse($this->createResponse(404)); + + return $client; + }); + + $handler = new FetchOrderRequestHandler( + (new BakeryTransportFactory($this->config()))->create($client), + ); + + $this->expectException(OrderNotFound::class); + + $handler->handle(new FetchOrderRequest('nope')); +} +``` + +The transport still comes from the real factory, so `ErrorPlugin` and the decorators are in play — only the wire response is faked. + +## Common Mistakes + +| Mistake | Fix | +|---|---| +| Handler test mocks `TransportInterface` | Use a cassette (or a mock *client*) through the real transport factory. | +| Test rebuilds the plugin stack by hand | Reuse the transport factory; give it an optional `?ClientInterface`. | +| Cassettes not committed | Commit them — that's what makes the test run offline and in CI. | +| Real credentials or customer data in a cassette | Sanitise before committing; record against a test account. See the VCR reference. | +| Asserting the expanded URI in a request model test | Assert the template and the parameters. | +| `expectNotToPerformAssertions()` as the whole test | Assert something about the parsed response. | +| A `Psl\Type` exception treated as a test bug | It means the API changed shape. Update the model, re-record. | +| Recording plugin added at default priority | Leave it at logging priority so the cassette matches the real request. | diff --git a/.agents/skills/test-http-integration/references/vcr.md b/.agents/skills/test-http-integration/references/vcr.md new file mode 100644 index 0000000..ec36da9 --- /dev/null +++ b/.agents/skills/test-http-integration/references/vcr.md @@ -0,0 +1,132 @@ +# VCR cassettes reference + +`php-http/vcr-plugin` records real HTTP responses to files and replays them on later runs. That gives request-handler tests one honest round-trip against the real API, then permanent offline reruns through the full stack. + +```bash +composer require --dev php-http/vcr-plugin +``` + +## How the two plugins interact + +| Plugin | Role | +|---|---| +| `ReplayPlugin($namingStrategy, $player, bool $throw)` | Looks for a cassette matching the request. Found → returns it and stops. | +| `RecordPlugin($namingStrategy, $recorder)` | Writes the response to a cassette after a real request. | + +`ClientBuilder::addRecording()` installs both with `throw: false`, which produces the self-recording behaviour you want: + +1. **No cassette, network available** → real request, cassette written. +2. **Cassette present** → replayed, no network. Response carries `X-VCR-REPLAYED`. +3. **No cassette, no network** → the test fails with a connection error. + +Case 3 is the expected state in CI for a cassette you forgot to commit. `useRecording()` from `UseVcrClient` also uses `throw: false`; construct `ReplayPlugin` yourself with `throw: true` if you would rather fail loudly than silently hit the network. + +## Naming strategies + +`PathNamingStrategy` builds the filename from host, method, path, a hash of selected headers, a hash of the query string, and — for `PUT`/`POST`/`PATCH` — a hash of the body: + +``` +tb-bakery-test.example.com_GET_orders_ord-1.txt +tb-bakery-test.example.com_POST_orders_3f0a1.txt +``` + +```php +new PathNamingStrategy([ + 'hash_headers' => ['X-Bakery-Key'], // default: [] + 'hash_body_methods' => ['PUT', 'POST', 'PATCH'], // the default +]); +``` + +Consequences to plan for: + +- **Body hashing means POST cassettes are payload-specific.** Change one field in the request model and the cassette no longer matches; you get a re-record (or a CI failure). That's a feature — the recorded response really did correspond to that payload. +- **`hash_headers` when the credential varies per test.** Add the header so two users don't collide on one cassette; leave it out when the credential is constant, or every rotation invalidates every cassette. +- **The host is part of the name.** Pointing tests at a different environment invalidates all cassettes. Keep the base URI stable via a committed test env var. + +Write your own `NamingStrategyInterface` when you need something else — for instance including the test method name so each test owns its cassettes: + +```php +final readonly class TestAwareNamingStrategy implements NamingStrategyInterface +{ + public function __construct( + private string $prefix, + private NamingStrategyInterface $inner = new PathNamingStrategy(), + ) { + } + + public function name(RequestInterface $request): string + { + return $this->prefix.'_'.$this->inner->name($request); + } +} +``` + +## Sanitising secrets before they hit disk + +Cassettes are serialized HTTP responses, and they go into version control. `FilesystemRecorder`'s third argument is a map of **regex pattern → replacement**, applied at record time: + +```php +new FilesystemRecorder( + FIXTURE_DIR.'/Bakery', + filters: [ + '/Set-Cookie: .*/' => 'Set-Cookie: [REDACTED]', + '/"token":"[^"]+"/' => '"token":"[REDACTED]"', + '/[\w.+-]+@[\w-]+\.[\w.]+/' => 'redacted@example.com', + ], +); +``` + +This only covers the **response**. The request is not stored at all, so request-side credentials never land in a cassette — but they do land in *logs* if you also attached a logger, which is what the sensitive-header formatters are for (see `configure-http-client`). + +Rules worth keeping: + +- Record against a test account with fabricated data, never production. +- Read a new cassette before committing it. Once it's in git history, a leaked token is leaked. +- Never sanitise by hand-editing a cassette without also adding the filter — the next re-record puts the secret back. + +## Where cassettes live + +Commit them next to the fixtures, grouped per integration: + +``` +tests/Fixtures/Bakery/ +├── bakery-test.example.com_GET_orders_ord-1.txt +└── bakery-test.example.com_POST_orders_3f0a1.txt +``` + +Define `FIXTURE_DIR` in `tests/bootstrap.php`: + +```php +define('FIXTURE_DIR', __DIR__.'/Fixtures'); +``` + +`FilesystemRecorder` creates the directory if it's missing; `UseVcrClient::useRecording()` asserts it exists, which turns a typo into an immediate, clear failure rather than a mystery cassette written somewhere else. + +## Re-recording + +When the API changes, or you change a request payload: + +1. Delete the affected cassette files. +2. Make sure the test env vars point at a reachable test environment. +3. Run the test — it records again. +4. Read the new cassette, check it holds no secrets, commit it. + +Only delete the cassettes you mean to re-record. Wiping the directory turns one focused re-record into a full-suite network run. + +## Diagnosing a failing cassette test + +| Symptom | Cause | +|---|---| +| `Unable to find a response to replay request "…"` | `throw: true` and no matching cassette. Check the generated name against the filenames. | +| Connection refused / DNS failure in CI | Cassette missing or misnamed; the test fell through to a real request. | +| Passes locally, fails in CI | Cassette not committed, or `FIXTURE_DIR` differs. | +| Suddenly re-records on every run | Something in the name is unstable — a rotating credential in `hash_headers`, a timestamp or a random id in the query or body. | +| `Psl\Type` coercion error on replay | The cassette predates a model change, or the API changed shape. Compare cassette to `::type()`. | +| Response replayed but assertions fail | Read the cassette. It is a plain text HTTP response — usually the fastest debugging tool you have. | + +## When not to use VCR + +- **Error paths.** A 404 or a malformed payload is easier and clearer with `UseMockClient` than with a recorded cassette. +- **Plugins, encoders, decoders.** No real API needed — mock client. +- **Model parsing.** No HTTP at all. +- **APIs with no test environment.** Record once by hand, or fall back to a mock client and accept that you're testing less. diff --git a/.gitignore b/.gitignore index c127a69..913a521 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ composer.lock .phpunit.result.cache /coverage/ infection.log +.claude/skills