From e86cd7f88d35cae4eb1da099da82b4830081858c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Thu, 6 Aug 2026 10:09:11 +0200 Subject: [PATCH 1/2] Make unconditionally-required request fields required constructor params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified against the live Quickpay API (not just the docs) with empty-body probes — the API validates body params before transaction state, so an unauthorized probe payment sufficed: - POST /payments without order_id -> 400 order_id length error; with a valid order_id but no currency -> 400 'currency is missing' - PUT /payments/{id}/link without amount -> 400 'amount is missing'; with only amount -> 2xx - capture/refund/authorize with an empty body -> 400 'body is invalid' (with amount they proceed to state/acquirer checks) CreatePaymentRequest::$orderId/$currency and $amount on CreateLinkRequest, CaptureRequest, RefundRequest and AuthorizePaymentRequest are now required, non-nullable constructor parameters, so a missing one fails at the call site (and in static analysis) instead of as a ValidationException after a network round-trip. A bodyless authorize is never valid, so PaymentsEndpoint::authorize() now requires its request too. All required fields were already the first constructor parameters, so positional callers are unaffected. Conditionally-required fields stay optional; there is still no construction-time validation logic. --- CLAUDE.md | 4 ++-- src/Client/Endpoint/PaymentsEndpoint.php | 5 +++-- src/Request/Payload.php | 12 ++++++++---- src/Request/Payment/AuthorizePaymentRequest.php | 5 +++-- src/Request/Payment/CaptureRequest.php | 8 +++++--- src/Request/Payment/CreateLinkRequest.php | 6 ++++-- src/Request/Payment/CreatePaymentRequest.php | 11 +++++------ src/Request/Payment/RefundRequest.php | 8 +++++--- tests/Client/Endpoint/PaymentsEndpointTest.php | 10 +++++++--- 9 files changed, 42 insertions(+), 27 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c7e6f4c..59ae1ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,11 +21,11 @@ CI (`.github/workflows/build.yaml`, branch `1.x`): coding-standards, dependency- ## Architecture -**Client (`src/Client/Client.php`, `ClientInterface`)** — PSR-18/17 + `php-http/discovery`, Valinor for (de)serialization. Constructor: `(string $apiKey, ?HttpClientInterface, ?RequestFactoryInterface, ?StreamFactoryInterface, ?MapperBuilder, ?NormalizerBuilder, bool $synchronized = false)` — only `$apiKey` is required; the rest are discovered/defaulted. `$synchronized` (exposed via `isSynchronized()` on `ClientInterface`) is the client-wide default for the payment operation methods' `$synchronized` flag. Immutable (`private readonly`, no setters). **Auth is HTTP Basic with an EMPTY username and the API key as the password** (`Basic base64(':'.$apiKey)`), plus a mandatory **`Accept-Version: v10`** header. Single host `https://api.quickpay.net` (there is no sandbox host). `request()` stamps the headers, tracks `lastRequest`/`lastResponse`, and routes non-2xx through `assertStatusCode()` (a `match` on the status code). Helpers: `get()`, `post()`, `put()`, `patch()` (the body-carrying ones take `?Payload` — nullable because cancel/authorize send no body; **payment update is PATCH, not PUT**), and `ping(): bool`. `payments()` is lazily memoized. `resolveUrl()` pins the host: an absolute URL to any other host, a non-default port, or an absolute URL combined with a `$query` throws `InvalidUrlException` — the credential-leak guard. `configureMapperBuilder()` / `registerNormalizerTransformers()` are the public hooks for consumers wiring a cached Valinor builder. +**Client (`src/Client/Client.php`, `ClientInterface`)** — PSR-18/17 + `php-http/discovery`, Valinor for (de)serialization. Constructor: `(string $apiKey, ?HttpClientInterface, ?RequestFactoryInterface, ?StreamFactoryInterface, ?MapperBuilder, ?NormalizerBuilder, bool $synchronized = false)` — only `$apiKey` is required; the rest are discovered/defaulted. `$synchronized` (exposed via `isSynchronized()` on `ClientInterface`) is the client-wide default for the payment operation methods' `$synchronized` flag. Immutable (`private readonly`, no setters). **Auth is HTTP Basic with an EMPTY username and the API key as the password** (`Basic base64(':'.$apiKey)`), plus a mandatory **`Accept-Version: v10`** header. Single host `https://api.quickpay.net` (there is no sandbox host). `request()` stamps the headers, tracks `lastRequest`/`lastResponse`, and routes non-2xx through `assertStatusCode()` (a `match` on the status code). Helpers: `get()`, `post()`, `put()`, `patch()` (the body-carrying ones take `?Payload` — nullable because cancel sends no body (authorize requires one — the live API rejects a bodyless authorize); **payment update is PATCH, not PUT**), and `ping(): bool`. `payments()` is lazily memoized. `resolveUrl()` pins the host: an absolute URL to any other host, a non-default port, or an absolute URL combined with a `$query` throws `InvalidUrlException` — the credential-leak guard. `configureMapperBuilder()` / `registerNormalizerTransformers()` are the public hooks for consumers wiring a cached Valinor builder. **Endpoint hierarchy (`src/Client/Endpoint/`)** — `Endpoint` (base: `$client` + `$mapperBuilder`; `mapItem()` runs the source through Valinor `Source::camelCaseKeys()`, maps to the typed DTO, stamps `$raw`, and converts Valinor `MappingError` → `MappingException`) → `ResourceEndpoint` (`getOne`/`createOne`/`update` [PATCH]/`operation` [POST `{id}/{action}`, appends `?synchronized` when asked]/`putSub`) → `CollectionEndpoint` (`getPage`/`paginate`). Quickpay list pagination is **header-less** — `?page=N&page_size=M` returns a bare JSON array, so `paginate()` stops when a page returns fewer items than `pageSize`. `PaymentsEndpoint` (`final`) exposes `getById`/`create`/`updatePayment`/`authorize`/`capture`/`refund`/`cancel`/`createLink`; the operation methods take an optional `?bool $synchronized = null` — `null` falls back to the client-wide `synchronized` constructor flag (Quickpay processes operations async by default and returns a pending op; `synchronized: true` waits for the completed transaction). -**Request DTOs (`src/Request/`)** — `Payload` is a mutable marker base. Concrete DTOs are `final class` with plain `public` promoted properties, **all optional/nullable with no construction-time validation** — Quickpay enforces required fields (a missing one surfaces as a `ValidationException`). On serialization the `Payload` normalizer transformer strips `null`/`[]` and converts camelCase → snake_case (`Client::camelToSnake`). `CreatePaymentRequest`, `UpdatePaymentRequest` (PATCH; **no `order_id`/`basket` — not updatable**), `AuthorizePaymentRequest`, `CaptureRequest`, `RefundRequest`, `CreateLinkRequest`, plus nested `Address`/`BasketItem`/`Shipping`. `CollectionRequestOptions` (`page`/`pageSize`, asserted `>= 1`, `toArray()` → `page`/`page_size`). Capture/refund/authorize take an `extras` hash (acquirer-specific) — **`extras` keys pass through verbatim, NOT snake_cased**; `acquirer` is a *link* param, not an operation param. +**Request DTOs (`src/Request/`)** — `Payload` is a mutable marker base. Concrete DTOs are `final class` with plain `public` promoted properties. Fields the API **unconditionally** requires are required, non-nullable constructor params — verified against the LIVE API (2026-08-06), not the docs: `CreatePaymentRequest::$orderId`+`$currency`, and `$amount` on `CreateLinkRequest`/`CaptureRequest`/`RefundRequest`/`AuthorizePaymentRequest`. Everything else is optional/nullable with no construction-time validation — Quickpay enforces conditional requirements and format rules (violations surface as a `ValidationException`). Before marking a new field required, verify with a live probe (empty-body requests get param errors BEFORE state errors, so an unauthorized probe payment suffices). On serialization the `Payload` normalizer transformer strips `null`/`[]` and converts camelCase → snake_case (`Client::camelToSnake`). `CreatePaymentRequest`, `UpdatePaymentRequest` (PATCH; **no `order_id`/`basket` — not updatable**), `AuthorizePaymentRequest`, `CaptureRequest`, `RefundRequest`, `CreateLinkRequest`, plus nested `Address`/`BasketItem`/`Shipping`. `CollectionRequestOptions` (`page`/`pageSize`, asserted `>= 1`, `toArray()` → `page`/`page_size`). Capture/refund/authorize take an `extras` hash (acquirer-specific) — **`extras` keys pass through verbatim, NOT snake_cased**; `acquirer` is a *link* param, not an operation param. **Response DTOs (`src/Response/`)** — entry DTOs extend `Resource` (`public array $raw`, stamped by the endpoint after mapping). `final class` (NOT `final readonly`, so `$raw` can be set post-construction — hence the rector skip). **Type only the stable, commonly-used fields; reach everything else via `$raw`** (original snake_case keys). `Payment`, `Operation`, `Link`, `Metadata`, and `Collection` (passive carrier; pagination logic lives on the endpoint). **GOTCHA learned the hard way: a single mis-typed *nested* field fails the WHOLE resource mapping** (Valinor is strict; the `$raw` fallback only protects fields you DON'T type). E.g. `Metadata::$is3dSecure` is `?bool` even though the API docs label it "string" — the live API returns a boolean. Verify nested field types against real responses, not the docs, and keep the typed subset conservative. Dates are `?\DateTimeImmutable` (`supportDateFormats('Y-m-d\TH:i:sP', 'Y-m-d\TH:i:s.uP')`). diff --git a/src/Client/Endpoint/PaymentsEndpoint.php b/src/Client/Endpoint/PaymentsEndpoint.php index 8493a49..9a4e2fe 100644 --- a/src/Client/Endpoint/PaymentsEndpoint.php +++ b/src/Client/Endpoint/PaymentsEndpoint.php @@ -48,11 +48,12 @@ public function updatePayment(int $id, UpdatePaymentRequest $request): Payment } /** - * POST `/payments/{id}/authorize`. Pass `$synchronized = true` to wait for and return the + * POST `/payments/{id}/authorize`. The body is required — the live API rejects a bodyless + * authorize (`body: "is invalid"`). Pass `$synchronized = true` to wait for and return the * completed transaction instead of the default asynchronous (pending) response; `null` (the * default) falls back to the client-wide `synchronized` flag set on the `Client` constructor. */ - public function authorize(int $id, ?AuthorizePaymentRequest $request = null, ?bool $synchronized = null): Payment + public function authorize(int $id, AuthorizePaymentRequest $request, ?bool $synchronized = null): Payment { return $this->operation($id, 'authorize', $request, $synchronized); } diff --git a/src/Request/Payload.php b/src/Request/Payload.php index 8553734..3642e3a 100644 --- a/src/Request/Payload.php +++ b/src/Request/Payload.php @@ -14,10 +14,14 @@ * filters out `null` / `[]` entries — so optional DTO properties that default to `null` are absent * from the produced JSON rather than serialized as `"field": null`. * - * Subclasses are `final class` with **mutable** `public` promoted properties and all-optional - * constructor arguments, so a request can be built incrementally (`new CreatePaymentRequest()`, then - * assign fields) or in one named-argument call. There is no construction-time validation — required - * fields are enforced by the Quickpay API (a missing one surfaces as a `ValidationException`). + * Subclasses are `final class` with **mutable** `public` promoted properties. Fields the Quickpay + * API *unconditionally* requires — verified against the live API, not just the docs — are required, + * non-nullable constructor arguments, so forgetting one fails at the call site (and is caught by + * static analysis) instead of surfacing as a `ValidationException` after a network round-trip. All + * other arguments are optional, so a request can be built in one named-argument call or + * incrementally (`new CreatePaymentRequest('order-1', 'DKK')`, then assign fields). Beyond those + * required arguments there is no construction-time validation — conditional requirements and format + * rules are enforced by the Quickpay API (a violation surfaces as a `ValidationException`). */ abstract class Payload { diff --git a/src/Request/Payment/AuthorizePaymentRequest.php b/src/Request/Payment/AuthorizePaymentRequest.php index 435bf6e..f3de4dc 100644 --- a/src/Request/Payment/AuthorizePaymentRequest.php +++ b/src/Request/Payment/AuthorizePaymentRequest.php @@ -9,7 +9,8 @@ /** * Body for `POST /payments/{id}/authorize`. * - * `amount` (smallest currency unit) is required. Authorizing directly via the API generally requires + * `amount` (smallest currency unit) is required — verified against the live API, which rejects an + * authorize without it (`body: "is invalid"`). Authorizing directly via the API generally requires * you to supply card data via `$card` (e.g. `['number' => ..., 'expiration' => ..., 'cvd' => ...]`, * or a `token`/wallet token) — which puts you in PCI scope. Most integrations instead authorize * through the hosted payment window; see @@ -26,7 +27,7 @@ final class AuthorizePaymentRequest extends Payload * @param array $extras */ public function __construct( - public ?int $amount = null, + public int $amount, public ?bool $autoCapture = null, public ?string $autoCaptureAt = null, public ?float $vatRate = null, diff --git a/src/Request/Payment/CaptureRequest.php b/src/Request/Payment/CaptureRequest.php index 73654ec..cdf1c93 100644 --- a/src/Request/Payment/CaptureRequest.php +++ b/src/Request/Payment/CaptureRequest.php @@ -10,8 +10,10 @@ * Body for `POST /payments/{id}/capture`. * * `amount` is the amount to capture, in the payment's currency expressed in the smallest unit - * (e.g. cents/øre). `extras` is the API's optional hash of acquirer-specific extra parameters; its - * keys are passed through verbatim (not converted to snake_case). + * (e.g. cents/øre). It is required — verified against the live API, which rejects a capture without + * it (`body: "is invalid"`) before even checking the transaction state. `extras` is the API's + * optional hash of acquirer-specific extra parameters; its keys are passed through verbatim (not + * converted to snake_case). */ final class CaptureRequest extends Payload { @@ -19,7 +21,7 @@ final class CaptureRequest extends Payload * @param array $extras */ public function __construct( - public ?int $amount = null, + public int $amount, public array $extras = [], ) { } diff --git a/src/Request/Payment/CreateLinkRequest.php b/src/Request/Payment/CreateLinkRequest.php index b225d7d..000beb0 100644 --- a/src/Request/Payment/CreateLinkRequest.php +++ b/src/Request/Payment/CreateLinkRequest.php @@ -10,7 +10,9 @@ * Body for `PUT /payments/{id}/link` — creates (or updates) the payment window link the customer is * redirected to. * - * `amount` is required by the API (smallest currency unit). `continueUrl` / `cancelUrl` are where the + * `amount` (smallest currency unit) is required — verified against the live API, which rejects a + * link without it (`amount: "is missing"`) and accepts a link with only it. `continueUrl` / + * `cancelUrl` are where the * customer is redirected after a successful / cancelled payment; `callbackUrl` overrides the * account's default server-to-server callback URL for this payment. Property names are converted to * the snake_case keys Quickpay expects (e.g. `continueUrl` → `continue_url`); `brandingConfig` is an @@ -22,7 +24,7 @@ final class CreateLinkRequest extends Payload * @param array $brandingConfig */ public function __construct( - public ?int $amount = null, + public int $amount, public ?int $agreementId = null, public ?string $language = null, public ?string $continueUrl = null, diff --git a/src/Request/Payment/CreatePaymentRequest.php b/src/Request/Payment/CreatePaymentRequest.php index fc2b701..3f176e7 100644 --- a/src/Request/Payment/CreatePaymentRequest.php +++ b/src/Request/Payment/CreatePaymentRequest.php @@ -9,10 +9,9 @@ /** * Body for `POST /payments`. * - * `orderId` and `currency` are required by the Quickpay API. Following the SDK's `Payload` - * convention they are nullable with a `null` default (so a request can be built incrementally); - * omitting them surfaces as a `ValidationException` from the API rather than a construction-time - * error. + * `orderId` (4–20 characters) and `currency` are required — verified against the live API, which + * rejects a create missing either (`order_id` length validation / `currency: "is missing"`). All + * other fields are optional. */ final class CreatePaymentRequest extends Payload { @@ -21,8 +20,8 @@ final class CreatePaymentRequest extends Payload * @param list $basket */ public function __construct( - public ?string $orderId = null, - public ?string $currency = null, + public string $orderId, + public string $currency, public ?string $textOnStatement = null, public ?int $brandingId = null, public array $variables = [], diff --git a/src/Request/Payment/RefundRequest.php b/src/Request/Payment/RefundRequest.php index 7fb20c9..0918b01 100644 --- a/src/Request/Payment/RefundRequest.php +++ b/src/Request/Payment/RefundRequest.php @@ -10,8 +10,10 @@ * Body for `POST /payments/{id}/refund`. * * `amount` is the amount to refund, in the payment's currency expressed in the smallest unit - * (e.g. cents/øre). `vatRate` optionally states the VAT rate of the refunded amount. `extras` is the - * API's optional hash of acquirer-specific extra parameters; its keys are passed through verbatim. + * (e.g. cents/øre). It is required — verified against the live API, which rejects a refund without + * it (`body: "is invalid"`) before even checking the transaction state. `vatRate` optionally states + * the VAT rate of the refunded amount. `extras` is the API's optional hash of acquirer-specific + * extra parameters; its keys are passed through verbatim. */ final class RefundRequest extends Payload { @@ -19,7 +21,7 @@ final class RefundRequest extends Payload * @param array $extras */ public function __construct( - public ?int $amount = null, + public int $amount, public ?float $vatRate = null, public array $extras = [], ) { diff --git a/tests/Client/Endpoint/PaymentsEndpointTest.php b/tests/Client/Endpoint/PaymentsEndpointTest.php index d1eeaed..cf0fdf1 100644 --- a/tests/Client/Endpoint/PaymentsEndpointTest.php +++ b/tests/Client/Endpoint/PaymentsEndpointTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\Attributes\Test; use Setono\Quickpay\Enum\PaymentState; use Setono\Quickpay\QuickpayTestCase; +use Setono\Quickpay\Request\Payment\AuthorizePaymentRequest; use Setono\Quickpay\Request\Payment\CaptureRequest; use Setono\Quickpay\Request\Payment\CreateLinkRequest; use Setono\Quickpay\Request\Payment\CreatePaymentRequest; @@ -138,15 +139,18 @@ public function it_cancels_without_a_body(): void } #[Test] - public function it_authorizes_without_a_body(): void + public function it_authorizes_with_an_amount_body(): void { $http = (new ScriptedHttpClient())->on(self::BASE . '/payments/1234/authorize', self::fixture('payment.json')); - $this->client($http)->payments()->authorize(1234); + $this->client($http)->payments()->authorize(1234, new AuthorizePaymentRequest(1000)); $sent = $http->sentRequests[0]; self::assertSame(self::BASE . '/payments/1234/authorize', (string) $sent->getUri()); - self::assertSame('', (string) $sent->getBody()); + + /** @var array $body */ + $body = json_decode((string) $sent->getBody(), true, flags: \JSON_THROW_ON_ERROR); + self::assertSame(['amount' => 1000], $body); } #[Test] From ba27ec33f41d3e29d54dd18b0ab1bdb91cc3ef76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Thu, 6 Aug 2026 10:16:37 +0200 Subject: [PATCH 2/2] Correct the required-amount evidence and send {} for an empty Payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first probe run sent an empty Payload, which the normalizer encodes as [] (a JSON array) — the API's 'body is invalid' was about that shape, not the missing amount. Re-probed with proper {} bodies and a test-card authorization (test_mode, charges nothing): - validation order is body shape -> transaction state -> params - capture without amount on an AUTHORIZED payment: 400 'amount is missing' - refund without amount on a CAPTURED payment: 400 'amount is missing' (the API does not fall back to capturing/refunding the remaining balance) - authorize with card data but no amount: 400 'amount is missing' So amount is genuinely required for all three operations and the required constructor params stand; docblocks now cite the real evidence. The probe also exposed an SDK bug: any Payload with no set fields was sent as [] and rejected by the API. Client::send() now rewrites the empty-array encoding to {}, with a regression test. --- CLAUDE.md | 4 ++-- src/Client/Client.php | 15 ++++++++++----- src/Client/Endpoint/PaymentsEndpoint.php | 4 ++-- src/Request/Payment/AuthorizePaymentRequest.php | 3 ++- src/Request/Payment/CaptureRequest.php | 9 +++++---- src/Request/Payment/RefundRequest.php | 5 +++-- tests/Client/Endpoint/PaymentsEndpointTest.php | 12 ++++++++++++ 7 files changed, 36 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 59ae1ef..773eb58 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,7 +25,7 @@ CI (`.github/workflows/build.yaml`, branch `1.x`): coding-standards, dependency- **Endpoint hierarchy (`src/Client/Endpoint/`)** — `Endpoint` (base: `$client` + `$mapperBuilder`; `mapItem()` runs the source through Valinor `Source::camelCaseKeys()`, maps to the typed DTO, stamps `$raw`, and converts Valinor `MappingError` → `MappingException`) → `ResourceEndpoint` (`getOne`/`createOne`/`update` [PATCH]/`operation` [POST `{id}/{action}`, appends `?synchronized` when asked]/`putSub`) → `CollectionEndpoint` (`getPage`/`paginate`). Quickpay list pagination is **header-less** — `?page=N&page_size=M` returns a bare JSON array, so `paginate()` stops when a page returns fewer items than `pageSize`. `PaymentsEndpoint` (`final`) exposes `getById`/`create`/`updatePayment`/`authorize`/`capture`/`refund`/`cancel`/`createLink`; the operation methods take an optional `?bool $synchronized = null` — `null` falls back to the client-wide `synchronized` constructor flag (Quickpay processes operations async by default and returns a pending op; `synchronized: true` waits for the completed transaction). -**Request DTOs (`src/Request/`)** — `Payload` is a mutable marker base. Concrete DTOs are `final class` with plain `public` promoted properties. Fields the API **unconditionally** requires are required, non-nullable constructor params — verified against the LIVE API (2026-08-06), not the docs: `CreatePaymentRequest::$orderId`+`$currency`, and `$amount` on `CreateLinkRequest`/`CaptureRequest`/`RefundRequest`/`AuthorizePaymentRequest`. Everything else is optional/nullable with no construction-time validation — Quickpay enforces conditional requirements and format rules (violations surface as a `ValidationException`). Before marking a new field required, verify with a live probe (empty-body requests get param errors BEFORE state errors, so an unauthorized probe payment suffices). On serialization the `Payload` normalizer transformer strips `null`/`[]` and converts camelCase → snake_case (`Client::camelToSnake`). `CreatePaymentRequest`, `UpdatePaymentRequest` (PATCH; **no `order_id`/`basket` — not updatable**), `AuthorizePaymentRequest`, `CaptureRequest`, `RefundRequest`, `CreateLinkRequest`, plus nested `Address`/`BasketItem`/`Shipping`. `CollectionRequestOptions` (`page`/`pageSize`, asserted `>= 1`, `toArray()` → `page`/`page_size`). Capture/refund/authorize take an `extras` hash (acquirer-specific) — **`extras` keys pass through verbatim, NOT snake_cased**; `acquirer` is a *link* param, not an operation param. +**Request DTOs (`src/Request/`)** — `Payload` is a mutable marker base. Concrete DTOs are `final class` with plain `public` promoted properties. Fields the API **unconditionally** requires are required, non-nullable constructor params — verified against the LIVE API (2026-08-06), not the docs: `CreatePaymentRequest::$orderId`+`$currency`, and `$amount` on `CreateLinkRequest`/`CaptureRequest`/`RefundRequest`/`AuthorizePaymentRequest`. Everything else is optional/nullable with no construction-time validation — Quickpay enforces conditional requirements and format rules (violations surface as a `ValidationException`). Before marking a new field required, verify with a live probe — and mind the validation ORDER: body shape → transaction state → params. A `[]` (JSON array) body fails with the generic `body: "is invalid"`, and state errors mask param validation, so operation params (capture/refund) can only be probed on a payment in the right state. To get one without the payment window: authorize via the API with a test card (`card: {number: '1000000000000008', expiration: '2612', cvd: '123'}`) — it works (at least on this account), producing a `test_mode` payment that charges nothing. On serialization the `Payload` normalizer transformer strips `null`/`[]` and converts camelCase → snake_case (`Client::camelToSnake`). `CreatePaymentRequest`, `UpdatePaymentRequest` (PATCH; **no `order_id`/`basket` — not updatable**), `AuthorizePaymentRequest`, `CaptureRequest`, `RefundRequest`, `CreateLinkRequest`, plus nested `Address`/`BasketItem`/`Shipping`. `CollectionRequestOptions` (`page`/`pageSize`, asserted `>= 1`, `toArray()` → `page`/`page_size`). Capture/refund/authorize take an `extras` hash (acquirer-specific) — **`extras` keys pass through verbatim, NOT snake_cased**; `acquirer` is a *link* param, not an operation param. **Response DTOs (`src/Response/`)** — entry DTOs extend `Resource` (`public array $raw`, stamped by the endpoint after mapping). `final class` (NOT `final readonly`, so `$raw` can be set post-construction — hence the rector skip). **Type only the stable, commonly-used fields; reach everything else via `$raw`** (original snake_case keys). `Payment`, `Operation`, `Link`, `Metadata`, and `Collection` (passive carrier; pagination logic lives on the endpoint). **GOTCHA learned the hard way: a single mis-typed *nested* field fails the WHOLE resource mapping** (Valinor is strict; the `$raw` fallback only protects fields you DON'T type). E.g. `Metadata::$is3dSecure` is `?bool` even though the API docs label it "string" — the live API returns a boolean. Verify nested field types against real responses, not the docs, and keep the typed subset conservative. Dates are `?\DateTimeImmutable` (`supportDateFormats('Y-m-d\TH:i:sP', 'Y-m-d\TH:i:s.uP')`). @@ -38,7 +38,7 @@ CI (`.github/workflows/build.yaml`, branch `1.x`): coding-standards, dependency- ## Key facts / gotchas - **No sandbox, no test key.** Consumers use their real API key; a payment becomes a *test* payment (`test_mode: true`) only when paid with a [test card](https://learn.quickpay.net/tech-talk/appendixes/test/). Test callbacks are real and signed exactly like production. -- **Valinor wiring:** `camelCaseKeys` on input, `camelToSnake` + null/`[]`-strip (the `Payload` transformer) on output. We do NOT register Valinor converters (they leak memory); `$raw` is stamped inline in `Endpoint::mapItem()`. +- **Valinor wiring:** `camelCaseKeys` on input, `camelToSnake` + null/`[]`-strip (the `Payload` transformer) on output. We do NOT register Valinor converters (they leak memory); `$raw` is stamped inline in `Endpoint::mapItem()`. A `Payload` with no set fields normalizes to `[]`, which the API rejects (`body: "is invalid"`) — `Client::send()` rewrites it to `{}`. - `examples/e2e/` is committed dev tooling and is intentionally OUT of the phpstan/ecs/rector paths (`src` + `tests` only) — check those scripts with `php -l`. Secrets live only in the gitignored `.env.local`; never commit them. ## Testing diff --git a/src/Client/Client.php b/src/Client/Client.php index 1d618ac..4f8eefb 100644 --- a/src/Client/Client.php +++ b/src/Client/Client.php @@ -219,11 +219,16 @@ private function send(string $method, string $uri, ?Payload $body): array $request = $this->requestFactory->createRequest($method, $this->resolveUrl($uri)); if (null !== $body) { - $request = $request->withBody( - $this->streamFactory->createStream( - $this->normalizerBuilder->normalizer(Format::json())->normalize($body), - ), - ); + $json = $this->normalizerBuilder->normalizer(Format::json())->normalize($body); + + // A Payload whose optional fields are all unset normalizes to an empty PHP array, which + // JSON-encodes as `[]` — the Quickpay API rejects that shape (`body: "is invalid"`); an + // empty body must be the empty JSON object. + if ('[]' === $json) { + $json = '{}'; + } + + $request = $request->withBody($this->streamFactory->createStream($json)); } return self::decodeJson($request, $this->request($request)); diff --git a/src/Client/Endpoint/PaymentsEndpoint.php b/src/Client/Endpoint/PaymentsEndpoint.php index 9a4e2fe..aa5b364 100644 --- a/src/Client/Endpoint/PaymentsEndpoint.php +++ b/src/Client/Endpoint/PaymentsEndpoint.php @@ -48,8 +48,8 @@ public function updatePayment(int $id, UpdatePaymentRequest $request): Payment } /** - * POST `/payments/{id}/authorize`. The body is required — the live API rejects a bodyless - * authorize (`body: "is invalid"`). Pass `$synchronized = true` to wait for and return the + * POST `/payments/{id}/authorize`. The body is required — the live API validates `amount` as + * required, so a request-less authorize can never succeed. Pass `$synchronized = true` to wait for and return the * completed transaction instead of the default asynchronous (pending) response; `null` (the * default) falls back to the client-wide `synchronized` flag set on the `Client` constructor. */ diff --git a/src/Request/Payment/AuthorizePaymentRequest.php b/src/Request/Payment/AuthorizePaymentRequest.php index f3de4dc..d6f5bdd 100644 --- a/src/Request/Payment/AuthorizePaymentRequest.php +++ b/src/Request/Payment/AuthorizePaymentRequest.php @@ -10,7 +10,8 @@ * Body for `POST /payments/{id}/authorize`. * * `amount` (smallest currency unit) is required — verified against the live API, which rejects an - * authorize without it (`body: "is invalid"`). Authorizing directly via the API generally requires + * authorize without it (`amount: "is missing"`, probed with card data supplied so validation is + * actually reached). Authorizing directly via the API generally requires * you to supply card data via `$card` (e.g. `['number' => ..., 'expiration' => ..., 'cvd' => ...]`, * or a `token`/wallet token) — which puts you in PCI scope. Most integrations instead authorize * through the hosted payment window; see diff --git a/src/Request/Payment/CaptureRequest.php b/src/Request/Payment/CaptureRequest.php index cdf1c93..b12ea57 100644 --- a/src/Request/Payment/CaptureRequest.php +++ b/src/Request/Payment/CaptureRequest.php @@ -10,10 +10,11 @@ * Body for `POST /payments/{id}/capture`. * * `amount` is the amount to capture, in the payment's currency expressed in the smallest unit - * (e.g. cents/øre). It is required — verified against the live API, which rejects a capture without - * it (`body: "is invalid"`) before even checking the transaction state. `extras` is the API's - * optional hash of acquirer-specific extra parameters; its keys are passed through verbatim (not - * converted to snake_case). + * (e.g. cents/øre). It is required — verified against the live API: a capture without it on an + * AUTHORIZED payment is rejected with `amount: "is missing"`; the API does NOT fall back to + * capturing the remaining authorized balance. `extras` is the API's optional hash of + * acquirer-specific extra parameters; its keys are passed through verbatim (not converted to + * snake_case). */ final class CaptureRequest extends Payload { diff --git a/src/Request/Payment/RefundRequest.php b/src/Request/Payment/RefundRequest.php index 0918b01..5c166ce 100644 --- a/src/Request/Payment/RefundRequest.php +++ b/src/Request/Payment/RefundRequest.php @@ -10,8 +10,9 @@ * Body for `POST /payments/{id}/refund`. * * `amount` is the amount to refund, in the payment's currency expressed in the smallest unit - * (e.g. cents/øre). It is required — verified against the live API, which rejects a refund without - * it (`body: "is invalid"`) before even checking the transaction state. `vatRate` optionally states + * (e.g. cents/øre). It is required — verified against the live API: a refund without it on a + * CAPTURED payment (positive balance) is rejected with `amount: "is missing"`; the API does NOT + * fall back to refunding the remaining balance. `vatRate` optionally states * the VAT rate of the refunded amount. `extras` is the API's optional hash of acquirer-specific * extra parameters; its keys are passed through verbatim. */ diff --git a/tests/Client/Endpoint/PaymentsEndpointTest.php b/tests/Client/Endpoint/PaymentsEndpointTest.php index cf0fdf1..ca92f11 100644 --- a/tests/Client/Endpoint/PaymentsEndpointTest.php +++ b/tests/Client/Endpoint/PaymentsEndpointTest.php @@ -207,6 +207,18 @@ public function it_can_request_a_synchronized_operation(): void self::assertSame(self::BASE . '/payments/1234/capture?synchronized', (string) $http->sentRequests[0]->getUri()); } + #[Test] + public function it_sends_an_empty_json_object_for_an_empty_payload(): void + { + $http = (new ScriptedHttpClient())->on(self::BASE . '/payments/1234', self::fixture('payment.json')); + + $this->client($http)->payments()->updatePayment(1234, new UpdatePaymentRequest()); + + // An all-unset Payload normalizes to an empty PHP array (`[]` as JSON) — the live API + // rejects a JSON array body, so the client must send the empty JSON object instead. + self::assertSame('{}', (string) $http->sentRequests[0]->getBody()); + } + #[Test] public function it_uses_the_client_wide_synchronized_default(): void {