diff --git a/AUDIT.md b/AUDIT.md deleted file mode 100644 index c0f5332..0000000 --- a/AUDIT.md +++ /dev/null @@ -1,81 +0,0 @@ -# Quality audit — 2026-05-27 — closeout - -All 26 audit findings actioned. Test suite went from 120 → 141 tests / 299 → 335 assertions. All four gates clean (PHPUnit, PHPStan max, ECS, Rector). Safe to delete this file. - -## Robustness & error handling - -- [x] **F01** — pre-read body once in `Client::assertStatusCode`, thread into `ResponseAwareException` via new `body:` ctor param; `parseBody()` uses cached value. Lazy-parse now survives non-seekable PSR-7 streams. -- [x] **F02** — `MappingException extends MalformedResponseException` wraps Valinor `MappingError`. Threaded via new `ResourceEndpoint::mapResource()` helper used by all three `->map()` call sites. Original error preserved as `$previous`; message embeds `[METHOD URL]`. -- [x] **F03** — `paginate()` docblock documents the non-transactional contract; regression test `mid_walk_failure_exhausts_the_generator_after_yielding_completed_pages` drives the generator step-by-step. -- [x] **F04** — defensive probe at `Client::__construct`: normalizes `Identifier::layout(1)` through the supplied builder; throws `\LogicException` with `Client::registerNormalizerTransformers` as the remediation hint if the SDK transformers aren't wired. -- [x] **F05** — new public `Client::registerNormalizerTransformers(NormalizerBuilder): NormalizerBuilder` wires BOTH `Identifier` and `Payload` transformers. `Client::defaultNormalizerBuilder()` uses it. README's production-usage example updated. -- [x] **F06** — `Client::request()` stamps `Content-Type: application/json` only if the request lacks one. Tests cover both branches. -- [x] **F07** — `Page` constructor validates `skippages` / `pagesize` (numeric, range bounds against e-conomic's 1-1000 max). Server-issued malformed nextPage URLs now fail loudly instead of silently coercing to 0. - -## Type system rigor & DX - -- [x] **F08** — `Client::post()` and `ClientInterface::post()` narrowed from `object` to `Payload`. All in-tree request DTOs already implement Payload; no test fallout. -- [x] **F09** — exception messages embed `[METHOD URL]` (query/fragment stripped). `ResponseAwareException` ctor gains optional `request:` param; threaded through `Client::assertStatusCode`. -- [x] **F10** — Identifier/Client transformer docblocks call out Valinor's LIFO transformer order. -- [x] **F11** — `Assert::stringNotEmpty(trim(...))` on all required string fields (`CustomerRequest::$name`/`$currency`, `DraftOrderRequest::$date`/`$currency`, `Recipient::$name`, `Identifier::$value` for string branch). Whitespace-only now rejected with clear messages. New tests cover the whitespace path. -- [x] **F12** — existing `@param list|null` PHPDoc on `DraftOrderRequest::__construct` is already what PHPStan reads (constructor-promoted property inherits the param type). No action needed. -- [x] **F13** — existing `@return T` / `@return T|null` PHPDocs on `ResourceEndpoint::getOne` / `CollectionEndpoint::getItem` already give PHPStan the typed return inference at consumer call sites. Audit's claim was overstated; no action needed. -- [x] **F14** — recursive null-skip lock-in: `Client::registerNormalizerTransformers` now strips BOTH `null` AND `[]` values from `Payload` normalized output. Caught a real behavior bug — nested all-null `Payload` (e.g. `new Accrual()`) was producing `"accrual":[]` instead of being omitted. Test added. - -## Developer experience & docs - -- [x] **F15** — README error-handling section rewritten: explicit `null` vs `throw` semantics, retry-policy guidance pointing at PSR-18 decorators, MappingException note. -- [x] **F18** — README gained a "Testing your code that calls the SDK" subsection with a minimal anonymous-class `ClientInterface` fake. -- [x] **F19** — README gained guidance: re-serialize via `$dto->raw`, not `json_encode($dto)`. -- [x] **F24** — README gained "Long-running processes" subsection with antipattern vs corrected `Client` reuse pattern (including the `mapperBuilder` + `normalizerBuilder` cache wiring). - -## Security - -- [x] **F16** — `Client::resolveUrl` lowercases both base and incoming hosts before comparison. Mixed-case server-issued URLs are accepted. Added private static helper `parseStringPart` to coerce parse_url's `string|false|null` to `string` for PHPStan-clean strtolower calls. -- [x] **F17** — `Client::resolveUrl` rejects any explicit non-default port (HTTPS → 443, HTTP → 80). New tests for both reject (`:9999`) and accept (`:443`). -- [x] **F22** — exception messages strip query string + fragment from the embedded URL. Implemented in both `Client::decodeJson` and `ResponseAwareException::buildRequestContext`. Test verifies secrets in query params don't leak. -- [x] **F25** — `tests/Client/ClientTest.php` gained mixed-case-host test (covers F16) and port-mismatch test (covers F17). - -## Specs, CI, testing infrastructure - -- [x] **F20** — `.github/workflows/build.yaml` `unit-tests` job now runs `lowest` + `highest`; `fail-fast: false` so one resolution failing doesn't mask the other. -- [x] **F21** — `tests/Exception/ExceptionHierarchyTest.php` gained `network_errors_from_the_psr18_layer_propagate_unwrapped_to_the_consumer` locking the contract: PSR-18 `NetworkExceptionInterface` propagates as-is (no SDK wrapping). Retry policy is BYOHC consumer's responsibility. -- [x] **F23** — `openspec/specs/http-transport/spec.md` updated for Content-Type override semantics + new scenario. - -## Standalone - -- [-] **F26** — perf micro (RawStamper / MapperBuilder allocation per Client construct). **Dropped.** Single-Client-per-process is the dominant pattern; multi-Client cost is negligible relative to Valinor mapper compilation. README's "Long-running processes" section (F24) documents the right pattern instead. - ---- - -## Summary stats - -| | Before | After | -|---|---|---| -| Tests | 120 | 141 (+21) | -| Assertions | 299 | 335 (+36) | -| HIGH findings open | 8 | 0 | -| MEDIUM findings open | 15 | 0 | -| LOW findings open | 3 | 0 | - -All four gates: ✓ `composer phpunit` ✓ `composer analyse` ✓ `composer check-style` ✓ `vendor/bin/rector --dry-run`. - -Mutation testing (`composer phpunit` doesn't gate it locally) deferred to CI as before — no pcov/xdebug for PHP 8.4 on this dev box. - -## Files touched - -- `src/Client/Client.php` — biggest surface change: new `registerNormalizerTransformers()` public helper, defensive probe in `__construct`, `assertStatusCode` pre-reads body & threads request, `decodeJson` strips query/fragment, `resolveUrl` case-insensitive host + port validation + new `parseStringPart` helper, `Content-Type` conditional stamping, narrowed `post()` to `Payload`. -- `src/Client/ClientInterface.php` — narrowed `post()` signature to `Payload`, added `Payload` import. -- `src/Client/Endpoint/ResourceEndpoint.php` — new `mapResource()` helper wrapping Valinor `MappingError` into `MappingException`; `getOne` and `createOne` route through it. -- `src/Client/Endpoint/CollectionEndpoint.php` — `mapPage` routes through `mapResource`; `paginate()` docblock expanded with non-transactional contract. -- `src/Exception/ResponseAwareException.php` — ctor gains `?string $body` + `?RequestInterface $request`; new `tryDecode()` and `buildRequestContext()` static helpers; default message embeds sanitized `[METHOD URL]`. -- `src/Exception/MalformedResponseException.php` — un-finalized (now extensible by MappingException). -- `src/Exception/MappingException.php` — new typed exception. -- `src/Request/Identifier.php` — docblock expanded with priority note; uses `Assert::stringNotEmpty(trim(...))` for string-value branch. -- `src/Request/Payload.php` — docblock describes recursive null-skipping contract. -- `src/Request/Customer/CustomerRequest.php`, `src/Request/Order/DraftOrderRequest.php`, `src/Request/Order/Recipient.php` — `Assert::stringNotEmpty(trim(...))` swap with descriptive messages. -- `src/Response/Pagination/Page.php` — validates `skippages`/`pagesize` query params from server-issued pagination URLs. -- `openspec/specs/http-transport/spec.md` — Content-Type override semantics updated + new scenario. -- `README.md` — Error handling section rewrite + testing guidance + production-usage cache example using `Client::registerNormalizerTransformers` + long-running processes subsection + json_encode/raw guidance. -- `.github/workflows/build.yaml` — `unit-tests` matrix runs lowest + highest. -- `tests/*` — 21 new tests across `ClientTest`, `ResponseAwareExceptionTest`, `ExceptionHierarchyTest`, `CustomersLookupTest`, `CustomerRequestTest`, `DraftOrderRequestTest`, `DraftOrdersCreateTest`, `PaginationTest`. diff --git a/CLAUDE.md b/CLAUDE.md index 703d59b..e1baf9f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,15 +24,15 @@ CI matrix (`.github/workflows/build.yaml`) runs PHP 8.4 against both `lowest` an Three layers, kept deliberately thin: -**1. `Client` (`src/Client/Client.php`)** — the user-facing entrypoint. Constructed with `(appSecretToken, agreementGrantToken)` plus five optional named collaborators: `httpClient` (PSR-18 `ClientInterface`), `requestFactory` (PSR-17 `RequestFactoryInterface`), `streamFactory` (PSR-17 `StreamFactoryInterface`), `mapperBuilder` (Valinor `MapperBuilder` for decoding responses), and `normalizerBuilder` (Valinor `NormalizerBuilder` for serializing POST/PUT bodies — separate from `MapperBuilder` per Valinor 2.x's API). Any collaborator left as `null` is resolved eagerly inside `__construct`: PSR-18/17 via `php-http/discovery` (`Psr18ClientDiscovery::find()`, `Psr17FactoryDiscovery::findRequestFactory()`, `Psr17FactoryDiscovery::findStreamFactory()`); the `MapperBuilder` falls back to `Client::defaultMapperBuilder()` and the `NormalizerBuilder` to `Client::defaultNormalizerBuilder()` (which registers the `Identifier` transformer and the `Payload` null-skipping transformer). The resolved collaborators are stored on `private readonly` properties — there are **no setters**, the client is immutable after construction. The auth tokens become the `X-AppSecretToken` / `X-AgreementGrantToken` headers on every request, alongside a `User-Agent` (`Setono-Economic-PHP/ (+url)`, version resolved at runtime via `Composer\InstalledVersions`). Exposes lazy accessors (`products()`, `customers()`, `orders()`, `invoices()`, `self()`), a low-level `request(RequestInterface): ResponseInterface` escape hatch, plus two JSON-aware helpers: `get(string $uri, array $query = []): array` and `post(string $uri, object $body): array`. `get()` accepts either a relative path (`'products'`) or a fully-qualified URL pointing at the e-conomic API host (e.g. a `pagination.nextPage.url`), detected by the `https?://` prefix. `post()` is strictly typed — it takes an `object`, normalizes it via the `NormalizerBuilder` (yielding JSON), builds a PSR-7 request via the stream factory, and dispatches through `request()`. Both helpers decode the response body internally and return `array`, NOT `ResponseInterface`. Absolute URLs are validated against the SDK's base host — `get()` throws `\InvalidArgumentException` before dispatch if the host differs, so auth credentials never leak to a non-e-conomic host. Combining an absolute URL with a non-empty `$query` is also rejected. For non-JSON endpoints (PDFs, attachment files) consumers fall back to `request()`. Base URI is hardcoded to `https://restapi.e-conomic.com`. There is no logger — `Client` does NOT implement `LoggerAwareInterface` and `psr/log` is NOT a dependency. Non-2xx responses funnel through `assertStatusCode()` which dispatches by status code into the exception hierarchy below. +**1. `Client` (`src/Client/Client.php`)** — the user-facing entrypoint. Constructed with `(appSecretToken, agreementGrantToken)` plus five optional named collaborators: `httpClient` (PSR-18 `ClientInterface`), `requestFactory` (PSR-17 `RequestFactoryInterface`), `streamFactory` (PSR-17 `StreamFactoryInterface`), `mapperBuilder` (Valinor `MapperBuilder` for decoding responses), and `normalizerBuilder` (Valinor `NormalizerBuilder` for serializing POST/PUT bodies — separate from `MapperBuilder` per Valinor 2.x's API). Any collaborator left as `null` is resolved eagerly inside `__construct`: PSR-18/17 via `php-http/discovery` (`Psr18ClientDiscovery::find()`, `Psr17FactoryDiscovery::findRequestFactory()`, `Psr17FactoryDiscovery::findStreamFactory()`); the `MapperBuilder` falls back to `Client::defaultMapperBuilder()` and the `NormalizerBuilder` to `Client::defaultNormalizerBuilder()` (which registers the `Identifier` transformer and the `Payload` null-skipping transformer). The resolved collaborators are stored on `private readonly` properties — there are **no setters**, the client is immutable after construction. The auth tokens become the `X-AppSecretToken` / `X-AgreementGrantToken` headers on every request, alongside a `User-Agent` (`Setono-Economic-PHP/ (+url)`, version resolved at runtime via `Composer\InstalledVersions`). Exposes lazy accessors (`products()`, `customers()`, `orders()`, `invoices()`, `self()`), a low-level `request(RequestInterface): ResponseInterface` escape hatch, plus three JSON-aware helpers: `get(string $uri, array $query = []): array`, `post(string $uri, Payload $body): array`, and `put(string $uri, Payload $body): array`. `get()` accepts either a relative path (`'products'`) or a fully-qualified URL pointing at the e-conomic API host (e.g. a `pagination.nextPage.url`), detected by the `https?://` prefix. `post()` and `put()` are strictly typed — they take a `Payload`, normalize it via the `NormalizerBuilder` (yielding JSON), build a PSR-7 request via the stream factory, and dispatch through `request()`; both delegate to a private `send(string $method, ...)`. All helpers decode the response body internally and return `array`, NOT `ResponseInterface`. Absolute URLs are validated against the SDK's base host — `get()` throws `\InvalidArgumentException` before dispatch if the host differs, so auth credentials never leak to a non-e-conomic host. Combining an absolute URL with a non-empty `$query` is also rejected. For non-JSON endpoints (PDFs, attachment files) consumers fall back to `request()`. Base URI is hardcoded to `https://restapi.e-conomic.com`. There is no logger — `Client` does NOT implement `LoggerAwareInterface` and `psr/log` is NOT a dependency. Non-2xx responses funnel through `assertStatusCode()` which dispatches by status code into the exception hierarchy below. **2. Endpoint hierarchy (`src/Client/Endpoint/`)** — three abstract tiers plus concrete leaves and dispatchers: - `Endpoint` (abstract base): holds `$client` and `$mapperBuilder`. Nothing else. Dispatchers stop here. -- `ResourceEndpoint extends Endpoint` (abstract): declares the per-resource hints `getPath(): string` and `getItemClass(): class-string` (both `static`). Provides two shared, symmetric pipelines — `getOne(int|string|null $id = null): T` (GET → decode → Valinor map → `$raw` stamp; with `$id = null` it fetches `getPath()`, with `$id` it fetches `"{getPath()}/{$id}"`) and `createOne(Payload $request): T` (POST typed body → decode → Valinor map → `$raw` stamp). Both helpers are `protected` and `@internal` — used by leaf endpoints to implement their typed public methods (`getByNumber()`, `create()`). +- `ResourceEndpoint extends Endpoint` (abstract): declares the per-resource hints `getPath(): string` and `getItemClass(): class-string` (both `static`). Provides three shared, symmetric pipelines — `getOne(int|string|null $id = null): T` (GET → decode → Valinor map → `$raw` stamp; with `$id = null` it fetches `getPath()`, with `$id` it fetches `"{getPath()}/{$id}"`), `createOne(Payload $request): T` (POST typed body → decode → Valinor map → `$raw` stamp), and `updateOne(int|string $id, Payload $request): T` (PUT typed body to `"{getPath()}/{$id}"` → same pipeline; e-conomic PUT is full-replace). All helpers are `protected` and `@internal` — used by leaf endpoints to implement their typed public methods (`getByNumber()`, `create()`, `update()`). - `CollectionEndpoint extends ResourceEndpoint` (abstract): adds `getItem(int|string $id): ?T` (wraps `getOne` with 404→null), plus concrete `getPage(?CollectionRequestOptions): Collection` and `paginate(?CollectionRequestOptions): \Generator` (every leaf inherits these unchanged). - Concrete classes: - - **Leaf collection endpoints** (`ProductsEndpoint`, `CustomersEndpoint`, `Orders/DraftOrdersEndpoint`, `Orders/SentOrdersEndpoint`, `Invoices/BookedInvoicesEndpoint`) extend `CollectionEndpoint`. Public surface: `getPage()` / `getByNumber()` / `paginate()`. `getByNumber()` is a one-liner: `return $this->getItem($number);`. Write-capable leaves (`CustomersEndpoint`, `Orders/DraftOrdersEndpoint`) additionally expose a typed `create()` method that is a one-liner: `return $this->createOne($request);`. Each leaf only declares the two static hints plus the typed public lookup/create method. + - **Leaf collection endpoints** (`ProductsEndpoint`, `CustomersEndpoint`, `Orders/DraftOrdersEndpoint`, `Orders/SentOrdersEndpoint`, `Invoices/BookedInvoicesEndpoint`) extend `CollectionEndpoint`. Public surface: `getPage()` / `getByNumber()` / `paginate()`. `getByNumber()` is a one-liner: `return $this->getItem($number);`. Write-capable leaves (`CustomersEndpoint`, `Orders/DraftOrdersEndpoint`) additionally expose typed `create()` / `update(int $number, ...)` methods that are one-liners: `return $this->createOne($request);` / `return $this->updateOne($number, $request);`. Each leaf only declares the two static hints plus the typed public lookup/write methods. - **Single-resource leaves** (`SelfEndpoint`) extend `ResourceEndpoint` directly (NOT `CollectionEndpoint` — Self has no by-id or pagination semantics). `SelfEndpoint::get()` calls `$this->getOne()` and memoizes so `$client->self()->get()` twice → one HTTP request. - **Dispatcher endpoints** (`OrdersEndpoint`, `InvoicesEndpoint`) extend plain `Endpoint`. They own NO collection methods. They expose lazy memoized accessors to their leaf sub-endpoints (`drafts()`, `sent()`, `booked()`). @@ -40,14 +40,16 @@ There are **no endpoint interfaces** — the concrete classes are `final` and co **3. Exception hierarchy (`src/Exception/`)** — `EconomicException` (marker interface, implemented by every SDK throw) → `ResponseAwareException` (abstract; carries the PSR response, lazy-parses `errorCode`/`developerHint`/`logId`/`logTime`/`getValidationErrors()` from e-conomic's JSON body) → `ClientErrorException` (4xx) and `ServerErrorException` (5xx) abstract bases → concrete types: `UnauthorizedException` (401), `ForbiddenException` (403), `NotFoundException` (404), `MethodNotAllowedException` (405), `ValidationException` (400/422), `InternalServerErrorException` (500), `NotImplementedException` (501). `UnexpectedStatusCodeException` is the catch-all for non-2xx codes not matched above (415, 429, 502, 504, etc. — e-conomic's docs never list 429, so it deliberately has no named subclass). `MalformedResponseException` (also extends `ResponseAwareException`) is thrown for 2xx responses whose body isn't the JSON object the SDK expects. `InvalidUrlException` (extends `\InvalidArgumentException`, implements `EconomicException`) is the only pre-flight exception — thrown by `Client::get()` when an absolute URL points to a foreign host or is combined with `$query`. The SDK does NOT implement retry semantics. `getValidationErrors()` returns e-conomic's raw nested document verbatim, NOT a flattened list. -**4. Response DTOs (`src/Response/`)** — entry-point DTOs (`Product`, `Order`, `BookedInvoice`, `Self_`, `Collection`) extend `Resource` and carry `public array $raw` populated by the endpoint after Valinor maps the typed fields. These DTOs are `final class` (NOT `final readonly class`) so `$raw` can be assigned after construction — `rector.php` skips `ReadOnlyClassRector` for them. Typed fields are still `public readonly` constructor-promoted. Nested DTOs (`Inventory`, `Line`, `Pagination`, `Page`) do NOT carry `$raw` — they're reachable through the parent's `$raw['nested-key']`. `Collection` is a passive data carrier (collection + pagination + raw); pagination logic lives on `CollectionEndpoint`, never on `Collection`. Valinor is configured with `enableFlexibleCasting()` and `allowSuperfluousKeys()`. **`Self_` uses the trailing-underscore PHP convention** because `Self` is a reserved word. +**4. Response DTOs (`src/Response/`)** — entry-point DTOs (`Product`, `Customer`, `Order`, `BookedInvoice`, `Self_`, `Collection`) extend `Resource` and carry `public array $raw` populated by the endpoint after Valinor maps the typed fields. These DTOs are `final class` (NOT `final readonly class`) so `$raw` can be assigned after construction — `rector.php` skips `ReadOnlyClassRector` for them. Typed fields are still `public readonly` constructor-promoted. Nested DTOs (`Inventory`, `Line`, `Pagination`, `Page`) do NOT carry `$raw` — they're reachable through the parent's `$raw['nested-key']`. `Collection` is a passive data carrier (collection + pagination + raw); pagination logic lives on `CollectionEndpoint`, never on `Collection`. Valinor is configured with `enableFlexibleCasting()` and `allowSuperfluousKeys()`. **`Self_` uses the trailing-underscore PHP convention** because `Self` is a reserved word. **5. Request helpers (`src/Request/`)** — read- and write-side request DTOs. - `CollectionRequestOptions` (read-side): an immutable final-readonly options DTO (skipPages, pageSize, filter, sortBy) with `withX()` builders and a `toArray(): array` projection. `pageSize` is asserted `>= 1 AND <= 1000` (e-conomic's server maximum). The previous `Query` class is removed — `Client::get()` takes an `array` directly. -- `Identifier` (write-side, cross-cutting): single foreign-key wrapper for every `{Number: int|string}` reference the e-conomic write schemas accept. Private constructor; 13 named static factories — `Identifier::layout()`, `::paymentTerms()`, `::customer()`, `::customerGroup()`, `::vatZone()`, `::project()`, `::deliveryLocation()`, `::product()` (string), `::unit()`, `::employee()`, `::customerContact()`, `::vendor()`, `::departmentalDistribution()`. Serialized as `{: }` by the SDK's normalizer transformer (registered on `Client::defaultNormalizerBuilder()`; consumers supplying their own builder MUST call `Identifier::registerTransformer($builder)` to wire it). -- `Payload` (marker interface): every request DTO implements it. The default `NormalizerBuilder` has a transformer keyed on `Payload` that strips `null` entries from the object's normalized array — so optional fields default to `null` and are absent from the produced JSON rather than serialized as `"field": null`. -- `Order/*` (write-side, scoped to one endpoint): `DraftOrderRequest` plus its typed nested objects (`Recipient`, `Delivery`, `Notes`, `References`, `Line`, `Accrual`) and the `NemHandelType` backed enum. All `final readonly implements Payload`. Required fields are non-nullable constructor args; optional fields default to `null`. `Webmozart\Assert` is used for cheap typo guards only (`notEmpty` on required strings, `length($currency, 3)` on the currency code, `positiveInteger` on identifier ints). Format / enum / conditional-required-ness validation is left to the e-conomic server. +- `Identifier` (write-side, cross-cutting): single foreign-key wrapper for every `{Number: int|string}` reference the e-conomic write schemas accept. Private constructor; 13 named static factories — `Identifier::layout()`, `::paymentTerms()`, `::customer()`, `::customerGroup()`, `::vatZone()`, `::project()`, `::deliveryLocation()`, `::product()` (string), `::unit()`, `::employee()`, `::customerContact()`, `::vendor()`, `::departmentalDistribution()` — plus `::fromReference(array $reference)`, which infers the field name from the reference object's single scalar `Number` key (throws on zero or multiple candidates) and exists for the `Payload::fromResponse()` mapper, where it makes identifiers round-trip back to the exact shape the server produced. Serialized as `{: }` by the SDK's normalizer transformer (registered on `Client::defaultNormalizerBuilder()`; consumers supplying their own builder MUST call `Identifier::registerTransformer($builder)` to wire it). +- `Payload` (abstract base class — NOT an interface): every request DTO extends it. Two roles. (1) Null-stripping marker: the default `NormalizerBuilder` has a transformer keyed on `Payload` that strips `null` entries from the object's normalized array — so optional fields default to `null` and are absent from the produced JSON rather than serialized as `"field": null` (mandatory, not cosmetic: e-conomic rejects explicit nulls; omission is its documented mechanism for clearing a field). (2) Read-modify-write prefill: `Payload::fromResponse(Resource $response): static` maps `$response->raw` into the calling DTO class via a memoized, SDK-internal Valinor mapper — strict (NO scalar casting; malformed raw data must throw, never be coerced or silently dropped, since a dropped field is cleared server-side on full-replace PUT), `allowSuperfluousKeys()` (raw carries the full response body; server-computed fields, HATEOAS links and unmodeled schema fields are dropped), `allowPermissiveTypes()` (required because `Identifier::fromReference()` takes `array`), with `Identifier::fromReference(...)` registered as constructor and `filterExceptions()` converting the SDK's own `\InvalidArgumentException` guards into mapping errors with node-path context. `MappingError` is wrapped into `\InvalidArgumentException` whose message carries the "requires a response fetched through the SDK" hint (hand-constructed responses have empty `$raw`). Because the method is inherited, the `$response` parameter is the wide `Resource` type — passing the wrong resource class is a runtime mapping error, not a compile-time error. +- All `Payload` DTOs are **deliberately mutable** — `final class` with plain `public` promoted properties, NOT `final readonly` (`rector.php` skips `ReadOnlyClassRector` and `ReadOnlyPropertyRector` for `src/Request/Customer` and `src/Request/Order`). Rationale: e-conomic updates are full-replace PUT, so the read-modify-write flow is "prefill via `fromResponse()` → assign the fields to change → `update()`"; setting a property to `null` after prefill means "omit from JSON = cleared server-side". Constructor `Assert` guards run at construction time only — mutation bypasses them, the server validates. `Identifier` and `CollectionRequestOptions` remain `final readonly` (value object / read-side options, not PUT/POST bodies). +- `Customer/CustomerRequest` (write-side): the POST/PUT body for customers, prefilled via the inherited `CustomerRequest::fromResponse($customer)`. Unmodeled schema fields (`priceGroup`, `customerContact`, `attention`, `defaultDeliveryLocation`) can NOT be carried over. `eInvoicingDisabledByDefault` is PATCH-only per the e-conomic docs ("updatable only by using PATCH to /customers/:customerNumber" — the API's sole exception to its no-PATCH-on-JSON rule) — its value in a PUT body is ignored and it is not cleared by omission, so mutating it on a prefilled request has no effect through `update()`. +- `Order/*` (write-side, scoped to one endpoint): `DraftOrderRequest` plus its typed nested objects (`Recipient`, `Delivery`, `Notes`, `References`, `Line`, `Accrual`) and the `NemHandelType` backed enum. All `final class extends Payload` (mutable, see above). Required fields are non-nullable constructor args; optional fields default to `null`. `Webmozart\Assert` is used for cheap typo guards only (`notEmpty` on required strings, `length($currency, 3)` on the currency code, `positiveInteger` on identifier ints). Format / enum / conditional-required-ness validation is left to the e-conomic server. `DraftOrderRequest::fromResponse($order)` works through the same inherited pipeline — nested `Payload`s and `Identifier`s map recursively. ## URL construction policy diff --git a/README.md b/README.md index 07b104c..0b7f0e2 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,19 @@ $request = new DraftOrderRequest( `Identifier` is the single foreign-key wrapper for every `{Number: int}` (or `productNumber: string`) reference the schema accepts. Named factories cover every reference type: `Identifier::layout()`, `Identifier::customer()`, `Identifier::customerGroup()`, `Identifier::vatZone()`, `Identifier::project()`, `Identifier::product()`, `Identifier::unit()`, `Identifier::employee()`, `Identifier::customerContact()`, `Identifier::vendor()`, `Identifier::deliveryLocation()`, `Identifier::paymentTerms()`, `Identifier::departmentalDistribution()`. +### Updating a draft order + +`update(int $number, DraftOrderRequest $request): Order` PUTs to `orders/drafts/:number`. **e-conomic PUT is full-replace**: any field absent from the body — including every `null` property, which the SDK omits from the JSON — is cleared server-side. Use the same read-modify-write flow as for customers (see below): fetch the order, prefill with `DraftOrderRequest::fromResponse()`, change what you need, PUT the whole thing back: + +```php +$order = $client->orders()->drafts()->getByNumber(42); + +$request = DraftOrderRequest::fromResponse($order); +$request->notes = new Notes(heading: 'Updated'); + +$updated = $client->orders()->drafts()->update(42, $request); +``` + ### Creating a customer Schema: [`customers.post.schema.json`](https://restapi.e-conomic.com/schema/customers.post.schema.json). @@ -270,6 +283,26 @@ $request = new CustomerRequest( **Note on `priceGroup`:** the e-conomic schema describes `priceGroup` as a `{ self: uri }` reference with no `priceGroupNumber` field, breaking the universal `Number` convention. `CustomerRequest` therefore omits it. Consumers needing to set the price group can drop down to the low-level helper: `$client->post('customers', $hand_built_payload)`. +### Updating a customer (read–modify–write) + +**e-conomic PUT is full-replace**: any field absent from the body is cleared server-side. Never build an update request with only the fields you want to change — fetch the customer first, prefill a request from it with `CustomerRequest::fromResponse()`, mutate what you need, and PUT the whole thing back: + +```php +$customer = $client->customers()->getByNumber(42); + +$request = CustomerRequest::fromResponse($customer); +$request->email = 'billing@acme.example'; // change what you need +$request->mobilePhone = null; // null = omitted from the JSON = cleared server-side + +$updated = $client->customers()->update(42, $request); +``` + +`fromResponse()` is available on every request DTO (it lives on the `Payload` base class): it maps the response's full decoded body (`$raw`) into the request DTO via Valinor. Every field the DTO models is carried over — reference objects like `customerGroup` or `salesPerson` become `Identifier` instances automatically, with the JSON field name inferred from the reference's `Number` key — and everything else (server-computed fields, HATEOAS links, unmodeled schema fields) is dropped. It therefore requires a response fetched through the SDK — on a hand-constructed instance (empty `$raw`) it throws. Raw data that is present but malformed also throws instead of being silently dropped, because a dropped field would be wiped by the subsequent PUT. + +**Caveat:** schema fields the SDK doesn't model (`priceGroup`, `customerContact`, `attention`, `defaultDeliveryLocation`, …) cannot be carried over and **will be cleared** by an update built this way. If you use those fields, hand-build the body and dispatch via `Client::request()`. + +**Caveat on `eInvoicingDisabledByDefault`:** the e-conomic docs state this property "is updatable only by using PATCH to /customers/:customerNumber" — the API's one exception to its no-PATCH-on-JSON rule. Its value in a PUT body is ignored, so changing it via `update()` has no effect (it is not cleared by omission either). To toggle it, send a JSON Patch request via `Client::request()`. + ## Error handling The SDK uses two distinct error patterns for reads and writes: @@ -283,7 +316,7 @@ if ($product === null) { } ``` -**Writes (`create`), custom `request()` calls, and `get()` always throw on any non-2xx.** Catch by type: +**Writes (`create`, `update`), custom `request()` calls, and `get()` always throw on any non-2xx.** Catch by type: ```php use Setono\Economic\Exception\EconomicException; @@ -586,15 +619,17 @@ if ($_ENV['APP_ENV'] === 'dev') { See [Valinor: Performance and caching](https://valinor.cuyz.io/latest/other/performance-and-cache/) for full details. +**Caveat on `supportDateFormats()`:** the SDK maps timestamp fields (e.g. `Customer::$lastUpdated`) to `\DateTimeImmutable` via Valinor's default date handling, which accepts e-conomic's `2020-02-19T09:18:09Z` format out of the box. Calling `supportDateFormats()` on a custom `MapperBuilder` *replaces* those defaults — if you do, include a format covering e-conomic's timestamps (e.g. `Y-m-d\TH:i:sp`), or mapping will throw `MappingException`. + ## Supported endpoints The SDK currently types the following endpoints. Anything not listed is reachable via the low-level helpers (see [Other requests](#other-requests)) — pull requests adding more endpoints are welcome. | Resource | Read (typed DTO returned) | Write | |---|---|---| -| Customers | `$client->customers()->getByNumber(int)`, `->getPage()`, `->paginate()` | `$client->customers()->create(CustomerRequest)` | +| Customers | `$client->customers()->getByNumber(int)`, `->getPage()`, `->paginate()` | `$client->customers()->create(CustomerRequest)`, `->update(int, CustomerRequest)` | | Products | `$client->products()->getByNumber(string)`, `->getPage()`, `->paginate()` | — | -| Draft orders | `$client->orders()->drafts()->getByNumber(int)`, `->getPage()`, `->paginate()` | `$client->orders()->drafts()->create(DraftOrderRequest)` | +| Draft orders | `$client->orders()->drafts()->getByNumber(int)`, `->getPage()`, `->paginate()` | `$client->orders()->drafts()->create(DraftOrderRequest)`, `->update(int, DraftOrderRequest)` | | Sent orders | `$client->orders()->sent()->getByNumber(int)`, `->getPage()`, `->paginate()` | — | | Booked invoices | `$client->invoices()->booked()->getByNumber(int)`, `->getPage()`, `->paginate()` | — | | Self / current agreement | `$client->self()->get()` | — | diff --git a/composer.json b/composer.json index 54d6e37..85c2862 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ ], "require": { "php": ">=8.4", - "cuyz/valinor": "^2.0", + "cuyz/valinor": "^2.2.2", "php-http/discovery": "^1.19", "psr/http-client": "^1.0", "psr/http-client-implementation": "^1", diff --git a/rector.php b/rector.php index 21401df..5ed83fd 100644 --- a/rector.php +++ b/rector.php @@ -3,8 +3,9 @@ declare(strict_types=1); use Rector\Config\RectorConfig; -use Rector\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector; +use Rector\Php81\Rector\Property\ReadOnlyPropertyRector; use Rector\Php82\Rector\Class_\ReadOnlyClassRector; +use Rector\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector; use Rector\Set\ValueObject\LevelSetList; return static function (RectorConfig $rectorConfig): void { @@ -28,6 +29,15 @@ __DIR__ . '/src/Response/Collection/Collection.php', __DIR__ . '/src/Response/Self_/Self_.php', __DIR__ . '/src/Response/Customer/Customer.php', + // Request `Payload` DTOs are deliberately mutable: the read-modify-write flow for + // full-replace PUT updates is "prefill via fromResponse() → assign fields → update()". + __DIR__ . '/src/Request/Customer', + __DIR__ . '/src/Request/Order', + ], + ReadOnlyPropertyRector::class => [ + // Same rationale — promoted properties on request DTOs must stay writable. + __DIR__ . '/src/Request/Customer', + __DIR__ . '/src/Request/Order', ], ]); }; diff --git a/src/Client/Client.php b/src/Client/Client.php index 7072432..3076919 100644 --- a/src/Client/Client.php +++ b/src/Client/Client.php @@ -144,9 +144,33 @@ public function get(string $uri, array $query = []): array * @return array */ public function post(string $uri, Payload $body): array + { + return $this->send('POST', $uri, $body); + } + + /** + * PUT a typed request DTO to `$uri` and return the decoded JSON body. Same serialization + * pipeline as {@see self::post()}. Note that e-conomic PUT endpoints are full-replace: + * `null` properties are stripped from the JSON by the `Payload` transformer, and any field + * absent from the body is cleared server-side. + * + * @return array + */ + public function put(string $uri, Payload $body): array + { + return $this->send('PUT', $uri, $body); + } + + /** + * Shared POST/PUT pipeline: normalize the typed DTO to JSON via the constructor-injected + * (or default) {@see NormalizerBuilder}, build the PSR-7 request, dispatch, decode. + * + * @return array + */ + private function send(string $method, string $uri, Payload $body): array { $request = $this->requestFactory - ->createRequest('POST', $this->resolveUrl($uri)) + ->createRequest($method, $this->resolveUrl($uri)) ->withBody( $this->streamFactory->createStream( $this->normalizerBuilder->normalizer(Format::json())->normalize($body), diff --git a/src/Client/ClientInterface.php b/src/Client/ClientInterface.php index 4d6d599..ca87f0a 100644 --- a/src/Client/ClientInterface.php +++ b/src/Client/ClientInterface.php @@ -76,6 +76,22 @@ public function get(string $uri, array $query = []): array; */ public function post(string $uri, Payload $body): array; + /** + * PUT the given typed request DTO to `$uri` and return the decoded JSON body. + * + * Serialization works exactly as in {@see self::post()}. Note that e-conomic PUT endpoints + * are full-replace: `null` properties are stripped from the JSON by the `Payload` + * null-skipping transformer, and any field absent from the body is cleared server-side. + * + * @return array + * + * @throws \Setono\Economic\Exception\InvalidUrlException if `$uri` is absolute and points to a different host than the base URI + * @throws ClientExceptionInterface if an error happens while processing the request + * @throws EconomicException if the response is non-2xx (concrete subtype depends on the status code) + * @throws \Setono\Economic\Exception\MalformedResponseException if the response body is not valid JSON or does not decode to an object + */ + public function put(string $uri, Payload $body): array; + public function customers(): CustomersEndpoint; public function invoices(): InvoicesEndpoint; diff --git a/src/Client/Endpoint/CustomersEndpoint.php b/src/Client/Endpoint/CustomersEndpoint.php index 6cef32e..36e5d0f 100644 --- a/src/Client/Endpoint/CustomersEndpoint.php +++ b/src/Client/Endpoint/CustomersEndpoint.php @@ -30,6 +30,28 @@ public function create(CustomerRequest $request): Customer return $this->createOne($request); } + /** + * PUT the given customer payload to `customers/{$number}` and return the updated + * {@see Customer}. Delegates to {@see ResourceEndpoint::updateOne()} — same Valinor + * pipeline and `$raw` stamping as {@see self::create()}. + * + * WARNING: e-conomic PUT is full-replace. Any field absent from the serialized body — + * including `null` properties (stripped by the `Payload` transformer) and schema fields + * not modeled on {@see CustomerRequest} (`priceGroup`, `customerContact`, `attention`, + * `defaultDeliveryLocation`, …) — is cleared server-side. Use + * {@see CustomerRequest::fromResponse()} to prefill a request from a fetched customer, + * or hand-build the body and use `Client::request()` for unmodeled fields. + * + * Exception: `eInvoicingDisabledByDefault` is "updatable only by using PATCH to + * /customers/:customerNumber" per the e-conomic docs — its value in a PUT body is + * ignored, so changing it on `$request` has no effect through this method (and it is + * not cleared by omission either). + */ + public function update(int $number, CustomerRequest $request): Customer + { + return $this->updateOne($number, $request); + } + protected static function getPath(): string { return 'customers'; diff --git a/src/Client/Endpoint/Orders/DraftOrdersEndpoint.php b/src/Client/Endpoint/Orders/DraftOrdersEndpoint.php index ac52063..1e60d4d 100644 --- a/src/Client/Endpoint/Orders/DraftOrdersEndpoint.php +++ b/src/Client/Endpoint/Orders/DraftOrdersEndpoint.php @@ -31,6 +31,23 @@ public function create(DraftOrderRequest $request): Order return $this->createOne($request); } + /** + * PUT the given draft-order payload to `orders/drafts/{$number}` and return the updated + * {@see Order}. Delegates to + * {@see \Setono\Economic\Client\Endpoint\ResourceEndpoint::updateOne()} — same Valinor + * pipeline and `$raw` stamping as {@see self::create()}. + * + * WARNING: e-conomic PUT is full-replace. Any field absent from the serialized body — + * including `null` properties (stripped by the `Payload` transformer) and schema fields + * not modeled on {@see DraftOrderRequest} — is cleared server-side. Use + * {@see DraftOrderRequest::fromResponse()} to prefill a request from a fetched order, + * or hand-build the body and use `Client::request()` for unmodeled fields. + */ + public function update(int $number, DraftOrderRequest $request): Order + { + return $this->updateOne($number, $request); + } + protected static function getPath(): string { return 'orders/drafts'; diff --git a/src/Client/Endpoint/ResourceEndpoint.php b/src/Client/Endpoint/ResourceEndpoint.php index 6d28e7a..f966ab1 100644 --- a/src/Client/Endpoint/ResourceEndpoint.php +++ b/src/Client/Endpoint/ResourceEndpoint.php @@ -14,15 +14,17 @@ * Abstract base for endpoints that represent a single REST resource at a path with a typed item DTO. * * Subclasses declare two protected hints: `getPath()` (the resource URL path) and `getItemClass()` - * (the typed DTO class). Two shared helpers, symmetric in shape: + * (the typed DTO class). Three shared helpers, symmetric in shape: * - {@see self::getOne()} — GET + decode + map + `$raw` stamp. * - {@see self::createOne()} — POST typed body + decode + map + `$raw` stamp. + * - {@see self::updateOne()} — PUT typed body to `"{getPath()}/{$id}"` + decode + map + `$raw` stamp. * * Three known subclass shapes today: * - {@see CollectionEndpoint} — adds pagination + `getItem(int|string $id)` for by-id lookups. * - {@see SelfEndpoint} — fetches the single resource at `/self` (no id), with memoization. - * - Leaf write-capable endpoints (e.g. `DraftOrdersEndpoint`, `CustomersEndpoint`) expose a typed - * public `create()` method that one-line-delegates to {@see self::createOne()}. + * - Leaf write-capable endpoints (e.g. `DraftOrdersEndpoint`, `CustomersEndpoint`) expose typed + * public `create()` / `update()` methods that one-line-delegate to {@see self::createOne()} / + * {@see self::updateOne()}. * * @template T of Resource */ @@ -98,6 +100,27 @@ protected function createOne(Payload $request): Resource return $item; } + /** + * PUT a typed request DTO to `"{getPath()}/{$id}"` and map the response into the typed item. + * Same Valinor map pipeline and `$raw` stamping as {@see self::createOne()}. e-conomic PUT + * endpoints are full-replace — any field absent from the serialized body is cleared + * server-side. Any non-2xx response propagates as the appropriate typed exception. + * + * @internal Shared pipeline used by SDK-internal leaf write endpoints. Not part of the + * package's BC promise. + * + * @return T + */ + protected function updateOne(int|string $id, Payload $request): Resource + { + $data = $this->client->put(sprintf('%s/%s', static::getPath(), $id), $request); + + /** @var T $item */ + $item = $this->mapResource(static::getItemClass(), $data); + + return $item; + } + /** * Run a Valinor mapping call and convert `MappingError` (a 2xx response whose body * decoded as JSON but didn't fit the target DTO) into the SDK's typed diff --git a/src/Request/Customer/CustomerRequest.php b/src/Request/Customer/CustomerRequest.php index 32fd9a8..915168f 100644 --- a/src/Request/Customer/CustomerRequest.php +++ b/src/Request/Customer/CustomerRequest.php @@ -6,24 +6,35 @@ use Setono\Economic\Request\Identifier; use Setono\Economic\Request\Payload; +use Setono\Economic\Response\Customer\Customer; use Webmozart\Assert\Assert; /** - * Typed request body for `POST /customers`. Required fields are non-nullable constructor - * arguments; optional fields default to `null` and are omitted from the serialized JSON via - * the SDK's `Payload` null-skipping transformer. + * Typed request body for `POST /customers` and `PUT /customers/:number`. Required fields are + * non-nullable constructor arguments; optional fields default to `null` and are omitted from + * the serialized JSON via the SDK's `Payload` null-skipping transformer. + * + * Deliberately mutable (NOT `readonly`): e-conomic updates are full-replace PUT, so the + * read-modify-write flow is "prefill via the inherited {@see Payload::fromResponse()} + * (`CustomerRequest::fromResponse($customer)`) → assign the fields to change → + * `CustomersEndpoint::update()`". The constructor `Assert` guards run at construction + * time only. * * Read-only server-computed fields (`balance`, `dueAmount`, `lastUpdated`, …) are intentionally - * absent — they belong on the {@see \Setono\Economic\Response\Customer\Customer} response side. + * absent — they belong on the {@see Customer} response side. * * The schema's `priceGroup` field is NOT exposed here. Its schema shape is `{ self: string(uri) }` * with no `priceGroupNumber`, breaking the universal `{Number: int}` identifier convention. * Consumers needing to set `priceGroup` use `Client::post('customers', $hand_built_payload)` * directly. See `openspec/changes/archive/-create-customer/design.md` for the rationale. * + * Note on `eInvoicingDisabledByDefault`: per the e-conomic docs it is "updatable only by + * using PATCH to /customers/:customerNumber" — mutating it on a prefilled request has no + * effect through `update()` (PUT ignores it; it is not cleared by omission either). + * * `customerNumber` is optional — when null, e-conomic auto-assigns one server-side. */ -final readonly class CustomerRequest implements Payload +final class CustomerRequest extends Payload { public function __construct( public string $name, diff --git a/src/Request/Identifier.php b/src/Request/Identifier.php index 72c9a2f..9b24378 100644 --- a/src/Request/Identifier.php +++ b/src/Request/Identifier.php @@ -102,6 +102,42 @@ public static function departmentalDistribution(int $number): self return new self('departmentalDistributionNumber', $number); } + /** + * Build an `Identifier` from an e-conomic reference object as returned by the REST API — + * e.g. `['customerGroupNumber' => 1, 'self' => 'https://...']`. The JSON field name is + * inferred from the object's single scalar `Number` key, so one factory covers every + * reference type the API returns (including the string-valued `productNumber`) and the + * identifier round-trips back to the exact shape the server itself produced. + * + * Registered as a Valinor constructor on the request mapper behind + * {@see Payload::fromResponse()}. Prefer the named factories above in hand-written code. + * + * @param array $reference + * + * @throws \InvalidArgumentException if the reference object does not contain exactly one scalar `Number` key + */ + public static function fromReference(array $reference): self + { + $candidates = []; + foreach ($reference as $field => $value) { + if (is_string($field) && str_ends_with($field, 'Number') && (is_int($value) || is_string($value))) { + $candidates[$field] = $value; + } + } + + if (1 !== \count($candidates)) { + throw new \InvalidArgumentException(sprintf( + 'Expected the reference object to contain exactly one scalar `Number` key, found %d (object keys: %s)', + \count($candidates), + implode(', ', array_map(strval(...), array_keys($reference))), + )); + } + + $field = array_key_first($candidates); + + return new self($field, $candidates[$field]); + } + /** * Append the SDK's `Identifier` transformer to a consumer-supplied {@see NormalizerBuilder}. * diff --git a/src/Request/Order/Accrual.php b/src/Request/Order/Accrual.php index 8244d77..efa9201 100644 --- a/src/Request/Order/Accrual.php +++ b/src/Request/Order/Accrual.php @@ -9,7 +9,7 @@ /** * Optional accrual period on an order line. Both dates are ISO-8601 (YYYY-MM-DD). */ -final readonly class Accrual implements Payload +final class Accrual extends Payload { public function __construct( public ?string $startDate = null, diff --git a/src/Request/Order/Delivery.php b/src/Request/Order/Delivery.php index 95a64b2..b5e17fa 100644 --- a/src/Request/Order/Delivery.php +++ b/src/Request/Order/Delivery.php @@ -10,7 +10,7 @@ * Delivery details for the order. All fields are optional per the e-conomic schema. * `deliveryDate` is ISO-8601 (YYYY-MM-DD). */ -final readonly class Delivery implements Payload +final class Delivery extends Payload { public function __construct( public ?string $address = null, diff --git a/src/Request/Order/DraftOrderRequest.php b/src/Request/Order/DraftOrderRequest.php index 727b25b..d1b5520 100644 --- a/src/Request/Order/DraftOrderRequest.php +++ b/src/Request/Order/DraftOrderRequest.php @@ -19,8 +19,12 @@ * * The conditional schema rule "`dueDate` is required when `paymentTermsType` is * `duedate`" is enforced by the e-conomic server; the SDK does not duplicate it. + * + * Deliberately mutable (NOT `readonly`): e-conomic updates are full-replace PUT, so the + * read-modify-write flow is "build/prefill a request → assign the fields to change → + * `update()`". The constructor `Assert` guards run at construction time only. */ -final readonly class DraftOrderRequest implements Payload +final class DraftOrderRequest extends Payload { /** * @param list|null $lines diff --git a/src/Request/Order/Line.php b/src/Request/Order/Line.php index 1c49fc0..d9aa53e 100644 --- a/src/Request/Order/Line.php +++ b/src/Request/Order/Line.php @@ -17,7 +17,7 @@ * `product` uses a string `productNumber`; `unit` and `departmentalDistribution` * use integers. All three are constructed via {@see Identifier} factories. */ -final readonly class Line implements Payload +final class Line extends Payload { public function __construct( public ?int $lineNumber = null, diff --git a/src/Request/Order/Notes.php b/src/Request/Order/Notes.php index a55d8d4..443fc0d 100644 --- a/src/Request/Order/Notes.php +++ b/src/Request/Order/Notes.php @@ -10,7 +10,7 @@ * Free-form note fields displayed on the printed/PDF representation of the order. * All fields are optional per the e-conomic schema. */ -final readonly class Notes implements Payload +final class Notes extends Payload { public function __construct( public ?string $heading = null, diff --git a/src/Request/Order/Recipient.php b/src/Request/Order/Recipient.php index c5b0fdf..dc9c8df 100644 --- a/src/Request/Order/Recipient.php +++ b/src/Request/Order/Recipient.php @@ -16,7 +16,7 @@ * `Identifier::customerContact(int)`. `vatZone` is constructed via * `Identifier::vatZone(int)`. */ -final readonly class Recipient implements Payload +final class Recipient extends Payload { public function __construct( public string $name, diff --git a/src/Request/Order/References.php b/src/Request/Order/References.php index d986b3c..b40ac85 100644 --- a/src/Request/Order/References.php +++ b/src/Request/Order/References.php @@ -16,7 +16,7 @@ * `Identifier::employee()`, `Identifier::customerContact()`, and * `Identifier::vendor()` respectively. */ -final readonly class References implements Payload +final class References extends Payload { public function __construct( public ?Identifier $salesPerson = null, diff --git a/src/Request/Payload.php b/src/Request/Payload.php index 1aef8d4..fe38496 100644 --- a/src/Request/Payload.php +++ b/src/Request/Payload.php @@ -4,23 +4,115 @@ namespace Setono\Economic\Request; +use CuyZ\Valinor\Mapper\MappingError; +use CuyZ\Valinor\Mapper\Tree\Message\ErrorMessage; +use CuyZ\Valinor\Mapper\Tree\Message\MessageBuilder; +use CuyZ\Valinor\Mapper\TreeMapper; +use CuyZ\Valinor\MapperBuilder; +use Setono\Economic\Response\Resource; + /** - * Marker interface implemented by every request DTO that should have its `null` - * properties stripped during JSON serialization. + * Base class for every request DTO. Serves two roles: + * + * 1. **Null-stripping marker.** The SDK's default {@see \CuyZ\Valinor\NormalizerBuilder} + * configuration has a transformer that matches `Payload` and filters nulls out of the + * object-normalized array, so optional DTO properties (defaulting to `null`) are absent + * from the produced JSON rather than serialized as `"field": null` — which e-conomic + * would reject ("we do not generally accept null as a value"). Valinor's normalizer + * recurses through the object graph, so the transformer fires for every `Payload` + * instance at every depth: a `DraftOrderRequest` containing a `Line` containing an + * `Accrual` whose nullable fields are all `null` will have its `accrual` key omitted + * entirely (because the normalized `Accrual` is itself empty after null-stripping). + * Consumers wiring a custom `NormalizerBuilder` must call + * {@see \Setono\Economic\Client\Client::registerNormalizerTransformers()} to get this + * behavior — see that method's docblock for the full contract. * - * Used by the SDK's default {@see \CuyZ\Valinor\NormalizerBuilder} configuration: - * a transformer matches `Payload` and filters nulls out of the object-normalized - * array, so optional DTO properties (defaulting to `null`) are absent from the - * produced JSON rather than serialized as `"field": null`. + * 2. **Read-modify-write prefill** via {@see self::fromResponse()}. * - * Valinor's normalizer recurses through the object graph, so the transformer fires - * for every `Payload` instance at every depth. A `DraftOrderRequest` containing a - * `Line` containing an `Accrual` whose nullable fields are all `null` will have its - * `accrual` key omitted entirely (because the normalized `Accrual` is itself empty - * after null-stripping). Consumers wiring a custom `NormalizerBuilder` must call - * {@see \Setono\Economic\Client\Client::registerNormalizerTransformers()} to get - * this behavior — see that method's docblock for the full contract. + * Subclasses are deliberately mutable (`final class` with plain `public` promoted + * properties): e-conomic updates are full-replace PUT, so the read-modify-write flow is + * "prefill via `fromResponse()` → assign the fields to change → `update()`". Constructor + * `Assert` guards run at construction time only. */ -interface Payload +abstract class Payload { + private static ?TreeMapper $requestMapper = null; + + /** + * Build a request DTO prefilled from a fetched response — the safe starting point for + * the read-modify-write flow that e-conomic's full-replace PUT semantics require: + * + * ``` + * $customer = $client->customers()->getByNumber(42); + * $request = CustomerRequest::fromResponse($customer); + * $request->email = 'new@example.com'; // change what you need + * $request->mobilePhone = null; // null = omitted from JSON = cleared server-side + * $client->customers()->update(42, $request); + * ``` + * + * The response's full decoded body (`$response->raw`) is mapped into the request DTO via + * Valinor: fields the DTO models are carried over (reference objects become + * {@see Identifier} instances via {@see Identifier::fromReference()}); everything else — + * server-computed fields, HATEOAS links, unmodeled schema fields — is dropped as + * superfluous. `$response` therefore MUST come from an SDK fetch: on a hand-constructed + * instance `$raw` is empty and the DTO's required fields fail to map. + * + * Raw data that is present but malformed (a reference object without its `Number` + * key, a wrong-typed scalar) throws rather than being silently dropped — a dropped + * field would be cleared server-side on the subsequent full-replace PUT. + * + * WARNING: schema fields NOT modeled on the request DTO are dropped for the same + * full-replace reason documented on the endpoints' `update()` methods — they WILL be + * cleared by an update built from this prefill. Hand-build the body and use + * `Client::request()` if you need to preserve them. + * + * @throws \InvalidArgumentException if `$response->raw` cannot be mapped into this DTO + * (hand-constructed response, missing required fields, or malformed raw data) + */ + public static function fromResponse(Resource $response): static + { + try { + return self::requestMapper()->map(static::class, $response->raw); + } catch (MappingError $e) { + throw new \InvalidArgumentException( + sprintf( + 'Could not build a %s from the given %s. fromResponse() requires a response fetched through the SDK — on hand-constructed instances $raw is empty. %s', + static::class, + $response::class, + $e->getMessage(), + ), + previous: $e, + ); + } + } + + /** + * The memoized Valinor mapper for the response-raw → request-DTO direction. Deliberately + * separate from the response mapper on `Client`: this one is strict (no scalar casting — + * malformed raw data must throw, see {@see self::fromResponse()}), allows superfluous + * keys (raw carries the full response body), and registers + * {@see Identifier::fromReference()} so reference objects map to `Identifier` properties. + * + * `allowPermissiveTypes()` is required because `Identifier::fromReference()` takes + * `array`, which Valinor's strict signature check rejects otherwise; it does not + * loosen value-level type checking. + */ + private static function requestMapper(): TreeMapper + { + return self::$requestMapper ??= new MapperBuilder() + ->allowSuperfluousKeys() + ->allowPermissiveTypes() + ->registerConstructor(Identifier::fromReference(...)) + ->filterExceptions(static function (\Throwable $exception): ErrorMessage { + // Surface the SDK's own guards (Identifier::fromReference, constructor + // asserts) as mapping errors with node-path context instead of letting + // them escape raw; anything unexpected keeps propagating. + if ($exception instanceof \InvalidArgumentException) { + return MessageBuilder::from($exception); + } + + throw $exception; + }) + ->mapper(); + } } diff --git a/src/Response/Customer/Customer.php b/src/Response/Customer/Customer.php index 44598d5..c9182b0 100644 --- a/src/Response/Customer/Customer.php +++ b/src/Response/Customer/Customer.php @@ -10,8 +10,8 @@ * Entry-point response DTO for a single customer. Typed fields cover identity / metadata, * contact + address, and server-computed financial state. Everything else (reference objects * like `customerGroup`, `vatZone`, `paymentTerms`, `layout`, `salesPerson`, …; HATEOAS link - * blobs; niche scalars like `pNumber`, `ean`, `publicEntryNumber`, `mobilePhone`, etc.) is - * accessible via {@see Resource::$raw}. + * blobs; niche scalars like `pNumber`, `ean`, `publicEntryNumber`, etc.) is accessible via + * {@see Resource::$raw}. * * `final class` (NOT `final readonly class`) so `$raw` can be assigned after construction — * see {@see Resource} for the rationale; `rector.php` skips this from `ReadOnlyClassRector`. @@ -24,7 +24,8 @@ public function __construct( public readonly ?string $name = null, public readonly ?string $currency = null, public readonly ?bool $barred = null, - public readonly ?string $lastUpdated = null, + // The original wire string remains available at `$raw['lastUpdated']`. + public readonly ?\DateTimeImmutable $lastUpdated = null, // contact & address public readonly ?string $email = null, public readonly ?string $address = null, @@ -33,6 +34,8 @@ public function __construct( public readonly ?string $country = null, public readonly ?string $corporateIdentificationNumber = null, public readonly ?string $vatNumber = null, + public readonly ?string $telephoneAndFaxNumber = null, + public readonly ?string $mobilePhone = null, // financial state (server-computed) public readonly ?float $balance = null, public readonly ?float $dueAmount = null, diff --git a/tests/Client/ClientTest.php b/tests/Client/ClientTest.php index d2274d0..6624353 100644 --- a/tests/Client/ClientTest.php +++ b/tests/Client/ClientTest.php @@ -16,6 +16,8 @@ use Psr\Http\Message\ResponseInterface; use Setono\Economic\Exception\InvalidUrlException; use Setono\Economic\Exception\MalformedResponseException; +use Setono\Economic\Request\Customer\CustomerRequest; +use Setono\Economic\Request\Identifier; #[CoversClass(Client::class)] final class ClientTest extends TestCase @@ -201,6 +203,63 @@ public function get_allows_relative_uri_with_embedded_query_when_query_array_is_ ); } + #[Test] + public function put_dispatches_a_put_with_the_serialized_payload(): void + { + $httpClient = new MockHttpClient(); + $client = self::createClient($httpClient); + + $client->put('customers/1', self::somePayload()); + + self::assertNotNull($httpClient->lastRequest); + self::assertSame('PUT', $httpClient->lastRequest->getMethod()); + self::assertSame( + 'https://restapi.e-conomic.com/customers/1', + (string) $httpClient->lastRequest->getUri(), + ); + + /** @var array $decoded */ + $decoded = json_decode((string) $httpClient->lastRequest->getBody(), true, flags: \JSON_THROW_ON_ERROR); + self::assertSame([ + 'name' => 'Acme', + 'currency' => 'DKK', + 'customerGroup' => ['customerGroupNumber' => 1], + 'vatZone' => ['vatZoneNumber' => 1], + 'paymentTerms' => ['paymentTermsNumber' => 1], + ], $decoded, 'the PUT body must run through the Identifier + Payload null-skipping transformers'); + } + + #[Test] + public function put_refuses_absolute_url_with_foreign_host(): void + { + $httpClient = new MockHttpClient(); + $client = self::createClient($httpClient); + + try { + $client->put('https://attacker.example.com/customers/1', self::somePayload()); + self::fail('expected InvalidUrlException'); + } catch (InvalidUrlException) { + // expected + } + + self::assertNull($httpClient->lastRequest, 'no request must be dispatched to the foreign host'); + } + + #[Test] + public function put_throws_malformed_response_exception_when_body_is_not_json(): void + { + $http = new class() implements HttpClientInterface { + public function sendRequest(RequestInterface $request): ResponseInterface + { + return new Response(200, ['Content-Type' => 'application/json'], 'this is not json'); + } + }; + $client = self::createClient($http); + + $this->expectException(MalformedResponseException::class); + $client->put('customers/1', self::somePayload()); + } + #[Test] public function get_throws_malformed_response_exception_when_body_is_not_json(): void { @@ -465,6 +524,17 @@ private static function createClient(?HttpClientInterface $httpClient = null): C { return new Client('app-secret-token', 'agreement-grant-token', httpClient: $httpClient); } + + private static function somePayload(): CustomerRequest + { + return new CustomerRequest( + name: 'Acme', + currency: 'DKK', + customerGroup: Identifier::customerGroup(1), + vatZone: Identifier::vatZone(1), + paymentTerms: Identifier::paymentTerms(1), + ); + } } final class MockHttpClient implements HttpClientInterface diff --git a/tests/Client/Endpoint/CustomersLookupTest.php b/tests/Client/Endpoint/CustomersLookupTest.php index 417591c..d255abf 100644 --- a/tests/Client/Endpoint/CustomersLookupTest.php +++ b/tests/Client/Endpoint/CustomersLookupTest.php @@ -85,6 +85,74 @@ public function customers_get_by_number_leaves_untyped_fields_in_raw(): void self::assertSame('https://example/invoices/drafts', $invoices['drafts']); } + #[Test] + public function phone_fields_map_to_typed_properties(): void + { + $http = new ScriptedHttpClient() + ->on( + 'https://restapi.e-conomic.com/customers/1', + '{"customerNumber":1,"telephoneAndFaxNumber":"+45 11111111","mobilePhone":"+45 22222222"}', + ) + ; + + $client = new Client('app', 'agreement', httpClient: $http); + $customer = $client->customers()->getByNumber(1); + + self::assertNotNull($customer); + self::assertSame('+45 11111111', $customer->telephoneAndFaxNumber); + self::assertSame('+45 22222222', $customer->mobilePhone); + } + + #[Test] + public function last_updated_maps_to_date_time_immutable_and_raw_keeps_the_wire_string(): void + { + $http = new ScriptedHttpClient() + ->on( + 'https://restapi.e-conomic.com/customers/1', + '{"customerNumber":1,"lastUpdated":"2020-02-19T09:18:09Z"}', + ) + ; + + $client = new Client('app', 'agreement', httpClient: $http); + $customer = $client->customers()->getByNumber(1); + + self::assertNotNull($customer); + self::assertNotNull($customer->lastUpdated); + self::assertSame('2020-02-19T09:18:09+00:00', $customer->lastUpdated->format(\DateTimeInterface::RFC3339)); + self::assertSame('2020-02-19T09:18:09Z', $customer->raw['lastUpdated']); + } + + #[Test] + public function absent_last_updated_maps_to_null(): void + { + $http = new ScriptedHttpClient() + ->on('https://restapi.e-conomic.com/customers/1', '{"customerNumber":1}') + ; + + $client = new Client('app', 'agreement', httpClient: $http); + $customer = $client->customers()->getByNumber(1); + + self::assertNotNull($customer); + self::assertNull($customer->lastUpdated); + } + + #[Test] + public function garbage_last_updated_surfaces_as_mapping_exception(): void + { + $http = new ScriptedHttpClient() + ->on( + 'https://restapi.e-conomic.com/customers/1', + '{"customerNumber":1,"lastUpdated":"not-a-date"}', + ) + ; + + $client = new Client('app', 'agreement', httpClient: $http); + + $this->expectException(MappingException::class); + + $client->customers()->getByNumber(1); + } + #[Test] public function valinor_mapping_failure_surfaces_as_mapping_exception_implementing_economic_exception(): void { diff --git a/tests/Client/Endpoint/CustomersUpdateTest.php b/tests/Client/Endpoint/CustomersUpdateTest.php new file mode 100644 index 0000000..5833b53 --- /dev/null +++ b/tests/Client/Endpoint/CustomersUpdateTest.php @@ -0,0 +1,221 @@ +on(self::UPDATE_URL, '{"customerNumber":1234}') + ; + $client = new Client('app', 'agreement', httpClient: $http); + + $client->customers()->update(1234, self::minimalRequiredRequest()); + + self::assertCount(1, $http->sentRequests); + $sent = $http->sentRequests[0]; + + self::assertSame('PUT', $sent->getMethod()); + self::assertSame(self::UPDATE_URL, (string) $sent->getUri()); + self::assertSame('application/json', $sent->getHeaderLine('Content-Type')); + self::assertSame('app', $sent->getHeaderLine('X-AppSecretToken')); + self::assertSame('agreement', $sent->getHeaderLine('X-AgreementGrantToken')); + self::assertNotSame('', $sent->getHeaderLine('User-Agent')); + } + + #[Test] + public function update_serializes_required_only_payload_with_identifiers_keyed_correctly(): void + { + $http = new ScriptedHttpClient() + ->on(self::UPDATE_URL, '{"customerNumber":1234}') + ; + $client = new Client('app', 'agreement', httpClient: $http); + + $client->customers()->update(1234, self::minimalRequiredRequest()); + + $body = (string) $http->sentRequests[0]->getBody(); + /** @var array $decoded */ + $decoded = json_decode($body, true, flags: \JSON_THROW_ON_ERROR); + + self::assertSame([ + 'name' => 'Acme', + 'currency' => 'DKK', + 'customerGroup' => ['customerGroupNumber' => 1], + 'vatZone' => ['vatZoneNumber' => 1], + 'paymentTerms' => ['paymentTermsNumber' => 1], + ], $decoded, 'required-only payload must serialize to exactly the 5 required keys; no nulls leak through'); + } + + #[Test] + public function update_returns_the_customer_dto_with_typed_scalars_and_raw_populated(): void + { + $responseBody = json_encode([ + 'customerNumber' => 1234, + 'name' => 'Acme', + 'currency' => 'DKK', + 'telephoneAndFaxNumber' => '+45 11111111', + 'mobilePhone' => '+45 22222222', + 'lastUpdated' => '2020-02-19T09:18:09Z', + ], \JSON_THROW_ON_ERROR); + + $http = new ScriptedHttpClient() + ->on(self::UPDATE_URL, $responseBody) + ; + $client = new Client('app', 'agreement', httpClient: $http); + + $customer = $client->customers()->update(1234, self::minimalRequiredRequest()); + + self::assertSame(1234, $customer->customerNumber); + self::assertSame('Acme', $customer->name); + self::assertSame('+45 11111111', $customer->telephoneAndFaxNumber); + self::assertSame('+45 22222222', $customer->mobilePhone); + self::assertNotNull($customer->lastUpdated); + self::assertSame('2020-02-19T09:18:09+00:00', $customer->lastUpdated->format(\DateTimeInterface::RFC3339)); + self::assertSame('Acme', $customer->raw['name']); + } + + #[Test] + public function update_surfaces_422_as_validation_exception(): void + { + $validationDoc = json_encode([ + 'errorCode' => 4400, + 'developerHint' => 'Validation failed', + 'errors' => [ + 'customerGroup' => ['customerGroupNumber' => ['Customer group 1 does not exist']], + ], + ], \JSON_THROW_ON_ERROR); + + $http = new ScriptedHttpClient() + ->on(self::UPDATE_URL, new Response(422, ['Content-Type' => 'application/json'], $validationDoc)) + ; + $client = new Client('app', 'agreement', httpClient: $http); + + try { + $client->customers()->update(1234, self::minimalRequiredRequest()); + self::fail('expected ValidationException'); + } catch (ValidationException $e) { + self::assertSame(422, $e->getResponse()->getStatusCode()); + self::assertSame(4400, $e->getErrorCode()); + } + } + + /** + * The documented read-modify-write flow end to end: GET a customer rich in reference + * objects and untyped scalars, prefill a request via {@see CustomerRequest::fromResponse()}, + * mutate two fields, PUT it back — and assert the ENTIRE serialized body, proving every + * carry-over (typed and `$raw`-sourced), the mutation, and the null-clears-field semantics. + */ + #[Test] + public function read_modify_write_round_trip_carries_over_every_modeled_field(): void + { + $url = 'https://restapi.e-conomic.com/customers/1'; + $getBody = json_encode([ + 'customerNumber' => 1, + 'name' => 'Acme', + 'currency' => 'DKK', + 'barred' => false, + 'lastUpdated' => '2020-02-19T09:18:09Z', + 'email' => 'foo@example.com', + 'address' => 'Main 1', + 'zip' => '9000', + 'city' => 'Aalborg', + 'country' => 'Denmark', + 'corporateIdentificationNumber' => '12345678', + 'vatNumber' => 'DK12345678', + 'telephoneAndFaxNumber' => '+45 11111111', + 'mobilePhone' => '+45 22222222', + 'balance' => 100.5, + 'dueAmount' => 50.25, + 'creditLimit' => 1000.5, + 'pNumber' => '1007331700', + 'ean' => '5790000123456', + 'publicEntryNumber' => 'PEN-1', + 'website' => 'https://acme.example.com', + 'eInvoicingDisabledByDefault' => true, + 'customerGroup' => ['customerGroupNumber' => 1, 'self' => 'https://example/cg/1'], + 'vatZone' => ['vatZoneNumber' => 2, 'self' => 'https://example/vz/2'], + 'paymentTerms' => ['paymentTermsNumber' => 3, 'self' => 'https://example/pt/3'], + 'layout' => ['layoutNumber' => 17, 'self' => 'https://example/layouts/17'], + 'salesPerson' => ['employeeNumber' => 5, 'self' => 'https://example/employees/5'], + ], \JSON_THROW_ON_ERROR); + + // ScriptedHttpClient keys on the URI alone, so the GET and the PUT share the script. + $http = new ScriptedHttpClient()->on($url, $getBody); + $client = new Client('app', 'agreement', httpClient: $http); + + $customer = $client->customers()->getByNumber(1); + self::assertInstanceOf(Customer::class, $customer); + + $request = CustomerRequest::fromResponse($customer); + $request->name = 'Renamed'; + $request->mobilePhone = null; + + $client->customers()->update(1, $request); + + self::assertCount(2, $http->sentRequests); + self::assertSame('PUT', $http->sentRequests[1]->getMethod()); + + $putBody = (string) $http->sentRequests[1]->getBody(); + /** @var array $decoded */ + $decoded = json_decode($putBody, true, flags: \JSON_THROW_ON_ERROR); + + self::assertSame([ + 'name' => 'Renamed', + 'currency' => 'DKK', + 'customerGroup' => ['customerGroupNumber' => 1], + 'vatZone' => ['vatZoneNumber' => 2], + 'paymentTerms' => ['paymentTermsNumber' => 3], + 'customerNumber' => 1, + 'barred' => false, + 'address' => 'Main 1', + 'city' => 'Aalborg', + 'country' => 'Denmark', + 'zip' => '9000', + 'corporateIdentificationNumber' => '12345678', + 'pNumber' => '1007331700', + 'creditLimit' => 1000.5, + 'ean' => '5790000123456', + 'email' => 'foo@example.com', + 'layout' => ['layoutNumber' => 17], + 'publicEntryNumber' => 'PEN-1', + 'telephoneAndFaxNumber' => '+45 11111111', + // mobilePhone was set to null after prefill → stripped → cleared server-side + 'eInvoicingDisabledByDefault' => true, + 'vatNumber' => 'DK12345678', + 'website' => 'https://acme.example.com', + 'salesPerson' => ['employeeNumber' => 5], + ], $decoded, 'the PUT body must carry over every modeled field, apply the mutation, and omit the nulled field; server-computed fields (balance, dueAmount, lastUpdated) must be absent'); + } + + private static function minimalRequiredRequest(): CustomerRequest + { + return new CustomerRequest( + name: 'Acme', + currency: 'DKK', + customerGroup: Identifier::customerGroup(1), + vatZone: Identifier::vatZone(1), + paymentTerms: Identifier::paymentTerms(1), + ); + } +} diff --git a/tests/Client/Endpoint/Orders/DraftOrdersUpdateTest.php b/tests/Client/Endpoint/Orders/DraftOrdersUpdateTest.php new file mode 100644 index 0000000..1faaa1d --- /dev/null +++ b/tests/Client/Endpoint/Orders/DraftOrdersUpdateTest.php @@ -0,0 +1,237 @@ +on(self::UPDATE_URL, '{"orderNumber":9}') + ; + $client = new Client('app', 'agreement', httpClient: $http); + + $client->orders()->drafts()->update(9, self::minimalRequiredRequest()); + + self::assertCount(1, $http->sentRequests); + $sent = $http->sentRequests[0]; + + self::assertSame('PUT', $sent->getMethod()); + self::assertSame(self::UPDATE_URL, (string) $sent->getUri()); + self::assertSame('application/json', $sent->getHeaderLine('Content-Type')); + } + + #[Test] + public function update_serializes_payload_through_identifier_and_null_skipping_transformers(): void + { + $http = new ScriptedHttpClient() + ->on(self::UPDATE_URL, '{"orderNumber":9}') + ; + $client = new Client('app', 'agreement', httpClient: $http); + + $client->orders()->drafts()->update(9, self::minimalRequiredRequest()); + + $body = (string) $http->sentRequests[0]->getBody(); + /** @var array $decoded */ + $decoded = json_decode($body, true, flags: \JSON_THROW_ON_ERROR); + + self::assertSame([ + 'date' => '2026-06-10', + 'currency' => 'DKK', + 'layout' => ['layoutNumber' => 17], + 'paymentTerms' => ['paymentTermsNumber' => 3], + 'customer' => ['customerNumber' => 42], + 'recipient' => [ + 'name' => 'Acme', + 'vatZone' => ['vatZoneNumber' => 2], + ], + ], $decoded, 'identifiers must serialize as {Number: n} and null optionals must be absent'); + } + + #[Test] + public function update_returns_the_order_dto_with_typed_fields_and_raw_populated(): void + { + $http = new ScriptedHttpClient() + ->on(self::UPDATE_URL, '{"orderNumber":9,"currency":"DKK"}') + ; + $client = new Client('app', 'agreement', httpClient: $http); + + $order = $client->orders()->drafts()->update(9, self::minimalRequiredRequest()); + + self::assertSame(9, $order->orderNumber); + self::assertSame('DKK', $order->raw['currency']); + } + + /** + * The read-modify-write flow for orders: GET a draft order rich in nested structures, + * prefill via the inherited {@see DraftOrderRequest::fromResponse()}, mutate, PUT it back — + * and assert the ENTIRE serialized body. Proves the Valinor raw→request mapping recurses + * through nested Payloads (Recipient, Delivery, References, Line) and reconstructs every + * Identifier from the server's own reference shapes. + */ + #[Test] + public function read_modify_write_round_trip_carries_over_every_modeled_field(): void + { + $url = 'https://restapi.e-conomic.com/orders/drafts/1'; + $getBody = json_encode([ + 'orderNumber' => 1, // not on the request DTO — dropped + 'date' => '2026-06-01', + 'currency' => 'DKK', + 'exchangeRate' => 743.21, + 'dueDate' => '2026-07-01', + 'grossAmount' => 124.88, // server-computed — dropped + 'netAmount' => 99.9, // server-computed — dropped + 'customer' => ['customerNumber' => 42, 'self' => 'https://example/customers/42'], + 'layout' => ['layoutNumber' => 17, 'self' => 'https://example/layouts/17'], + 'paymentTerms' => [ + 'paymentTermsNumber' => 3, + 'daysOfCredit' => 30, + 'name' => 'Net 30', + 'paymentTermsType' => 'net', + 'self' => 'https://example/pt/3', + ], + 'recipient' => [ + 'name' => 'Acme', + 'address' => 'Main 1', + 'zip' => '9000', + 'city' => 'Aalborg', + 'country' => 'Denmark', + 'ean' => '5790000123456', + 'mobilePhone' => '+45 22222222', + 'vatZone' => ['vatZoneNumber' => 2, 'self' => 'https://example/vz/2'], + 'attention' => ['customerContactNumber' => 9, 'self' => 'https://example/contacts/9'], + ], + 'delivery' => [ + 'address' => 'Dock 1', + 'zip' => '9000', + 'city' => 'Aalborg', + 'country' => 'Denmark', + 'deliveryTerms' => 'EXW', + 'deliveryDate' => '2026-06-15', + ], + 'notes' => ['heading' => 'Hello', 'textLine1' => 'L1', 'textLine2' => 'L2'], + 'references' => [ + 'salesPerson' => ['employeeNumber' => 5, 'self' => 'https://example/employees/5'], + 'customerContact' => ['customerContactNumber' => 9, 'self' => 'https://example/contacts/9'], + 'other' => 'PO-123', + ], + 'lines' => [ + [ + 'lineNumber' => 1, + 'sortKey' => 1, + 'description' => 'Widget', + 'unit' => ['unitNumber' => 1, 'name' => 'pcs'], + 'product' => ['productNumber' => 'SKU-001', 'self' => 'https://example/products/SKU-001'], + 'quantity' => 2.5, + 'unitNetPrice' => 49.95, + 'discountPercentage' => 2.5, + 'unitCostPrice' => 20.5, + 'marginInBaseCurrency' => 74.88, // server-computed — dropped + 'marginPercentage' => 59.95, // server-computed — dropped + ], + ], + 'pdf' => ['download' => 'https://example/orders/drafts/1/pdf'], // HATEOAS — dropped + 'self' => 'https://example/orders/drafts/1', + ], \JSON_THROW_ON_ERROR); + + // ScriptedHttpClient keys on the URI alone, so the GET and the PUT share the script. + $http = new ScriptedHttpClient()->on($url, $getBody); + $client = new Client('app', 'agreement', httpClient: $http); + + $order = $client->orders()->drafts()->getByNumber(1); + self::assertNotNull($order); + + $request = DraftOrderRequest::fromResponse($order); + $request->currency = 'EUR'; + $request->notes = null; // null = omitted from JSON = cleared server-side + + $client->orders()->drafts()->update(1, $request); + + self::assertCount(2, $http->sentRequests); + self::assertSame('PUT', $http->sentRequests[1]->getMethod()); + + $putBody = (string) $http->sentRequests[1]->getBody(); + /** @var array $decoded */ + $decoded = json_decode($putBody, true, flags: \JSON_THROW_ON_ERROR); + + self::assertSame([ + 'date' => '2026-06-01', + 'currency' => 'EUR', + 'layout' => ['layoutNumber' => 17], + 'paymentTerms' => ['paymentTermsNumber' => 3], + 'customer' => ['customerNumber' => 42], + 'recipient' => [ + 'name' => 'Acme', + 'vatZone' => ['vatZoneNumber' => 2], + 'address' => 'Main 1', + 'zip' => '9000', + 'city' => 'Aalborg', + 'country' => 'Denmark', + 'ean' => '5790000123456', + 'attention' => ['customerContactNumber' => 9], + 'mobilePhone' => '+45 22222222', + ], + 'exchangeRate' => 743.21, + 'dueDate' => '2026-07-01', + 'delivery' => [ + 'address' => 'Dock 1', + 'zip' => '9000', + 'city' => 'Aalborg', + 'country' => 'Denmark', + 'deliveryTerms' => 'EXW', + 'deliveryDate' => '2026-06-15', + ], + // notes was set to null after prefill → stripped → cleared server-side + 'references' => [ + 'salesPerson' => ['employeeNumber' => 5], + 'customerContact' => ['customerContactNumber' => 9], + 'other' => 'PO-123', + ], + 'lines' => [ + [ + 'lineNumber' => 1, + 'sortKey' => 1, + 'description' => 'Widget', + 'unit' => ['unitNumber' => 1], + 'product' => ['productNumber' => 'SKU-001'], + 'quantity' => 2.5, + 'unitNetPrice' => 49.95, + 'discountPercentage' => 2.5, + 'unitCostPrice' => 20.5, + ], + ], + ], $decoded, 'the PUT body must carry over every modeled field (including nested Payloads and Identifiers), apply the mutation, omit the nulled notes block, and drop server-computed fields'); + } + + private static function minimalRequiredRequest(): DraftOrderRequest + { + return new DraftOrderRequest( + date: '2026-06-10', + currency: 'DKK', + layout: Identifier::layout(17), + paymentTerms: Identifier::paymentTerms(3), + customer: Identifier::customer(42), + recipient: new Recipient( + name: 'Acme', + vatZone: Identifier::vatZone(2), + ), + ); + } +} diff --git a/tests/Exception/ExceptionHierarchyTest.php b/tests/Exception/ExceptionHierarchyTest.php index 170b638..e42a6a5 100644 --- a/tests/Exception/ExceptionHierarchyTest.php +++ b/tests/Exception/ExceptionHierarchyTest.php @@ -15,7 +15,8 @@ use Setono\Economic\Client\Client; #[CoversClass(Client::class)] -#[CoversClass(EconomicException::class)] +// EconomicException is deliberately NOT listed: it's an interface, and PHPUnit 11 rejects +// interfaces as coverage targets (they contain no executable code). #[CoversClass(ResponseAwareException::class)] #[CoversClass(ClientErrorException::class)] #[CoversClass(ServerErrorException::class)] diff --git a/tests/Request/Customer/CustomerRequestFromResponseTest.php b/tests/Request/Customer/CustomerRequestFromResponseTest.php new file mode 100644 index 0000000..ca31568 --- /dev/null +++ b/tests/Request/Customer/CustomerRequestFromResponseTest.php @@ -0,0 +1,230 @@ +name); + self::assertSame('DKK', $request->currency); + self::assertSame(['customerGroupNumber' => 1], [$request->customerGroup->fieldName => $request->customerGroup->value]); + self::assertSame(['vatZoneNumber' => 2], [$request->vatZone->fieldName => $request->vatZone->value]); + self::assertSame(['paymentTermsNumber' => 3], [$request->paymentTerms->fieldName => $request->paymentTerms->value]); + self::assertSame(1, $request->customerNumber); + self::assertFalse($request->barred); + self::assertSame('Main 1', $request->address); + self::assertSame('Aalborg', $request->city); + self::assertSame('Denmark', $request->country); + self::assertSame('9000', $request->zip); + self::assertSame('12345678', $request->corporateIdentificationNumber); + self::assertSame('1007331700', $request->pNumber); + self::assertSame(1000.5, $request->creditLimit); + self::assertSame('5790000123456', $request->ean); + self::assertSame('foo@example.com', $request->email); + self::assertNotNull($request->layout); + self::assertSame(['layoutNumber' => 17], [$request->layout->fieldName => $request->layout->value]); + self::assertSame('PEN-1', $request->publicEntryNumber); + self::assertSame('+45 11111111', $request->telephoneAndFaxNumber); + self::assertSame('+45 22222222', $request->mobilePhone); + self::assertTrue($request->eInvoicingDisabledByDefault); + self::assertSame('DK12345678', $request->vatNumber); + self::assertSame('https://acme.example.com', $request->website); + self::assertNotNull($request->salesPerson); + self::assertSame(['employeeNumber' => 5], [$request->salesPerson->fieldName => $request->salesPerson->value]); + } + + #[Test] + public function it_is_mutable_after_prefill(): void + { + $request = CustomerRequest::fromResponse(self::richCustomer()); + + $request->email = 'new@example.com'; + $request->mobilePhone = null; + + self::assertSame('new@example.com', $request->email); + self::assertNull($request->mobilePhone); + } + + #[Test] + public function absent_optional_raw_keys_default_to_null(): void + { + $request = CustomerRequest::fromResponse(self::customerWithRaw(self::minimalRaw())); + + self::assertNull($request->pNumber); + self::assertNull($request->ean); + self::assertNull($request->publicEntryNumber); + self::assertNull($request->website); + self::assertNull($request->eInvoicingDisabledByDefault); + self::assertNull($request->layout); + self::assertNull($request->salesPerson); + self::assertNull($request->barred); + self::assertNull($request->email); + } + + #[Test] + public function explicit_null_reference_object_in_raw_maps_to_null(): void + { + $request = CustomerRequest::fromResponse(self::customerWithRaw(self::minimalRaw() + ['layout' => null])); + + self::assertNull($request->layout); + } + + #[Test] + public function it_throws_when_a_required_reference_object_is_missing_from_raw(): void + { + $raw = self::minimalRaw(); + unset($raw['customerGroup']); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('customerGroup'); + + CustomerRequest::fromResponse(self::customerWithRaw($raw)); + } + + #[Test] + public function it_throws_on_a_hand_constructed_customer_with_empty_raw(): void + { + // Typed properties are ignored on purpose — fromResponse() maps from $raw alone, + // and $raw is only populated on SDK-fetched resources. + $customer = new Customer(customerNumber: 1, name: 'Acme', currency: 'DKK'); + + try { + CustomerRequest::fromResponse($customer); + self::fail('expected InvalidArgumentException'); + } catch (\InvalidArgumentException $e) { + self::assertStringContainsString('fetched through the SDK', $e->getMessage()); + self::assertInstanceOf(MappingError::class, $e->getPrevious(), 'the Valinor error must be preserved as $previous'); + } + } + + #[Test] + public function it_throws_when_a_required_scalar_is_missing_from_raw(): void + { + $raw = self::minimalRaw(); + unset($raw['currency']); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('currency'); + + CustomerRequest::fromResponse(self::customerWithRaw($raw)); + } + + #[Test] + public function it_throws_when_a_reference_object_lacks_its_number_key_instead_of_silently_dropping_it(): void + { + // e-conomic's priceGroup-style shape: a bare HATEOAS self link with no number. Dropping + // it silently would clear the field server-side on the subsequent full-replace PUT. + $raw = self::minimalRaw() + ['layout' => ['self' => 'https://example/layouts/17']]; + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('layout'); + + CustomerRequest::fromResponse(self::customerWithRaw($raw)); + } + + #[Test] + public function it_throws_when_a_reference_object_is_not_an_array(): void + { + $raw = self::minimalRaw(); + $raw['vatZone'] = 'not-an-object'; + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('vatZone'); + + CustomerRequest::fromResponse(self::customerWithRaw($raw)); + } + + #[Test] + public function it_throws_when_an_untyped_raw_scalar_has_the_wrong_type(): void + { + // The request mapper is strict — no scalar casting. A wrong-typed value must throw, + // never be coerced or dropped. + $raw = self::minimalRaw() + ['website' => 123]; + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('website'); + + CustomerRequest::fromResponse(self::customerWithRaw($raw)); + } + + /** + * The minimum `$raw` for fromResponse() to succeed: the request DTO's five required + * constructor fields. + * + * @return array + */ + private static function minimalRaw(): array + { + return [ + 'name' => 'Acme', + 'currency' => 'DKK', + 'customerGroup' => ['customerGroupNumber' => 1], + 'vatZone' => ['vatZoneNumber' => 2], + 'paymentTerms' => ['paymentTermsNumber' => 3], + ]; + } + + /** + * @param array $raw + */ + private static function customerWithRaw(array $raw): Customer + { + $customer = new Customer(); + $customer->raw = $raw; + + return $customer; + } + + private static function richCustomer(): Customer + { + return self::customerWithRaw([ + 'customerNumber' => 1, + 'name' => 'Acme', + 'currency' => 'DKK', + 'barred' => false, + 'lastUpdated' => '2020-02-19T09:18:09Z', // server-computed — dropped as superfluous + 'email' => 'foo@example.com', + 'address' => 'Main 1', + 'zip' => '9000', + 'city' => 'Aalborg', + 'country' => 'Denmark', + 'corporateIdentificationNumber' => '12345678', + 'vatNumber' => 'DK12345678', + 'telephoneAndFaxNumber' => '+45 11111111', + 'mobilePhone' => '+45 22222222', + 'balance' => 100.5, + 'dueAmount' => 50.25, + 'creditLimit' => 1000.5, + 'pNumber' => '1007331700', + 'ean' => '5790000123456', + 'publicEntryNumber' => 'PEN-1', + 'website' => 'https://acme.example.com', + 'eInvoicingDisabledByDefault' => true, + 'customerGroup' => ['customerGroupNumber' => 1, 'self' => 'https://example/cg/1'], + 'vatZone' => ['vatZoneNumber' => 2, 'self' => 'https://example/vz/2'], + 'paymentTerms' => ['paymentTermsNumber' => 3, 'self' => 'https://example/pt/3'], + 'layout' => ['layoutNumber' => 17, 'self' => 'https://example/layouts/17'], + 'salesPerson' => ['employeeNumber' => 5, 'self' => 'https://example/employees/5'], + 'priceGroup' => ['self' => 'https://example/pg/1'], // unmodeled — dropped + 'invoices' => ['drafts' => 'https://example/inv/drafts'], // HATEOAS — dropped + 'self' => 'https://example/customers/1', + ]); + } +} diff --git a/tests/Request/IdentifierTest.php b/tests/Request/IdentifierTest.php index b6d52cd..4c4d8db 100644 --- a/tests/Request/IdentifierTest.php +++ b/tests/Request/IdentifierTest.php @@ -44,6 +44,71 @@ public function integer_factories_pin_the_correct_field_name(Identifier $identif self::assertSame($value, $identifier->value); } + #[Test] + public function from_reference_infers_the_field_name_from_the_single_number_key(): void + { + $identifier = Identifier::fromReference([ + 'customerGroupNumber' => 7, + 'name' => 'Wholesale', + 'self' => 'https://restapi.e-conomic.com/customer-groups/7', + ]); + + self::assertSame('customerGroupNumber', $identifier->fieldName); + self::assertSame(7, $identifier->value); + } + + #[Test] + public function from_reference_accepts_string_valued_number_keys(): void + { + // products use a string productNumber — the value round-trips as the server sent it + $identifier = Identifier::fromReference(['productNumber' => 'SKU-001', 'self' => 'https://example/products/SKU-001']); + + self::assertSame('productNumber', $identifier->fieldName); + self::assertSame('SKU-001', $identifier->value); + } + + #[Test] + public function from_reference_ignores_nested_arrays_when_detecting_the_number_key(): void + { + // order references.customerContact carries a nested customer object whose + // customerNumber must not make the detection ambiguous + $identifier = Identifier::fromReference([ + 'customerContactNumber' => 9, + 'customer' => ['customerNumber' => 42], + ]); + + self::assertSame('customerContactNumber', $identifier->fieldName); + self::assertSame(9, $identifier->value); + } + + #[Test] + public function from_reference_throws_when_no_number_key_is_present(): void + { + // e-conomic's priceGroup-style shape: a bare HATEOAS self link + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('found 0 (object keys: self)'); + + Identifier::fromReference(['self' => 'https://example/pg/1']); + } + + #[Test] + public function from_reference_throws_when_multiple_number_keys_are_present(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('found 2'); + + Identifier::fromReference(['customerNumber' => 1, 'employeeNumber' => 2]); + } + + #[Test] + public function from_reference_throws_when_the_number_value_is_not_a_scalar_identifier(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('found 0'); + + Identifier::fromReference(['customerGroupNumber' => true]); + } + #[Test] public function product_factory_takes_a_string(): void {