Rewrite SDK in the economic-php-sdk style#5
Conversation
Re-architect the SDK to mirror setono/economic-php-sdk while preserving exactly the currently supported functionality (payment gateways, pickup points, sales orders, shipment templates, webhooks + the shipment template resolver). Highlights: - Immutable, constructor-injected Client with a sandbox flag (replaces setters/setDebug); HTTP Basic auth; typed exception hierarchy under ShipmondoException with match-based status mapping. - Base-class endpoints (Endpoint/ResourceEndpoint/CollectionEndpoint) replace the per-endpoint interfaces and capability traits. - Resource responses carrying $raw (stamped by RawStamper, outside Valinor's converter pipeline); Payload requests serialized via the Valinor Normalizer (null-strip + camelCase->snake_case + date->ISO + native enums). - Header-based Collection pagination; pickup points returns a bare list; payment gateways/shipment templates look up by ?id=. Tooling: PHP >=8.1 retained; switch Psalm -> PHPStan level max; cuyz/valinor pinned ^2.2.2 <2.4 (2.4 drops 8.1); PHPUnit ^10.5; drop psr/log. README + CLAUDE.md updated. Verified: PHPUnit 60 tests green, PHPStan max clean, ECS clean, Rector clean, composer validate/normalize clean, Infection MSI 58%/covered 69%.
| "php": ">=8.1", | ||
| "cuyz/valinor": "^1.10", | ||
| "php-http/discovery": "^1.14", | ||
| "cuyz/valinor": "^2.2.2 <2.4", |
There was a problem hiding this comment.
| "cuyz/valinor": "^2.2.2 <2.4", | |
| "cuyz/valinor": "^2.2.2", |
There was a problem hiding this comment.
Done — applied in fb2d722. You're right that the <2.4 cap was redundant for the 8.1 floor: Valinor 2.4 requires PHP ≥8.2, so composer's platform constraint already keeps a PHP-8.1 install on ≤2.3. Dropping the cap also means the highest CI on 8.2/8.3 now actually exercises Valinor 2.4 — I verified 2.4 keeps the same PHPStan-extension paths, Source::camelCaseKeys(), and NormalizerBuilder, and CI is green on it.
|
|
||
| /** | ||
| * Takes a response and returns a Valinor Source representation | ||
| * Recursively convert snake_case array keys to camelCase so they match DTO property names. |
There was a problem hiding this comment.
I think the valinor library can do this. Else the Symfony string component also has this capability I think: https://symfony.com/doc/6.4/components/string.html
There was a problem hiding this comment.
Good call — done in fb2d722. Switched to Valinor's native Source::array($data)->camelCaseKeys() for mapping and removed the hand-rolled converter. That also let me delete the custom RawStamper: $raw is now just the untouched snake_case payload assigned on the top-level resource (which is arguably a better meaning of "raw" anyway, and matches the key names in the Shipmondo docs).
|
|
||
| abstract class ShipmondoTestCase extends TestCase | ||
| { | ||
| protected const BASE = 'https://sandbox.shipmondo.com/api/public/v3'; |
There was a problem hiding this comment.
What is this used for?
There was a problem hiding this comment.
BASE is the sandbox API base URL (https://sandbox.shipmondo.com/api/public/v3). The tests drive the client through ScriptedHttpClient, a fake PSR-18 keyed by the full request URL, so each test scripts a response against self::BASE . '/sales_orders?page=1&per_page=20' etc. Centralising it on the base test case keeps those URL literals consistent across the endpoint tests. Happy to inline or rename it if you'd prefer.
Verified the integration end-to-end against the Shipmondo sandbox (reads for all endpoints, sales order create + getById, webhook create/list/delete/deleteAll). Captured the real response payloads and wired them into the endpoint tests as fixtures (tests/Fixtures), added a ShipmentTemplatesEndpointTest, and made LiveClientTest sandbox-aware via SHIPMONDO_SANDBOX.
Capture the real GET /sales_orders list payload as a fixture and assert getPage() maps it. Document in the README that newly created sales orders are indexed asynchronously and may not appear in getPage()/paginate() immediately (use getById with the id returned by create() for read-after-write).
Two CI failures, both reproduced locally with --prefer-lowest: - Static analysis fataled on the lowest matrix: Valinor's suppress-pure-errors PHPStan extension implements PHPStan\Analyser\IgnoreErrorExtension, which only exists in PHPStan >= 2.2, but ^2.1 allowed 2.1.0. The extension is needed (our registerTransformer closures aren't provably pure), so bump phpstan/phpstan to ^2.2. - Dependency analysis failed: composer-require-checker flagged Composer\InstalledVersions. Drop the version lookup in the User-Agent for a static string (and fix the corresponding ClientTest assertion).
Swap the CI dependency-analysis tool to shipmonk/composer-dependency-analyser (via setup-php). Add composer-dependency-analyser.php config to ignore the unused-dependency error on the virtual psr/http-client-implementation and psr/http-factory-implementation packages (they force a concrete implementation but expose no symbols). Verified locally: no composer issues found.
- Rename top-level request bodies SalesOrder/Webhook -> SalesOrderRequest/WebhookRequest (de-aliases the endpoints; responses stay bare). - Map via Valinor's native Source::camelCaseKeys() instead of a hand-rolled converter; drop the RawStamper class and set $raw to the untouched snake_case payload on the top-level resource. - Relax cuyz/valinor to ^2.2.2 (composer's platform constraint keeps PHP 8.1 on <=2.3; 8.2+ CI now exercises 2.4, verified compatible).
Add a full 1.x->2.x upgrade guide. Document the $raw escape hatch in the README, and capture the request-DTO naming, valinor/phpstan constraints, composer-dependency-analyser, and the snake_case $raw decision in CLAUDE.md.
1 participant
Summary
Re-architects the SDK to mirror
setono/economic-php-sdk, preserving exactly the functionality already supported — no new endpoints. Targets the2.xmajor.Scope preserved:
paymentGateways(list +getById),pickupPoints(search),salesOrders(list/getById/create),shipmentTemplates(list +getById),webhooks(list/create/delete/deleteAll), and theShipmentTemplateResolver.Architecture
Client— collaborators constructor-injected,sandbox: trueflag (replacessetDebug()and all setters), HTTP Basic auth, host/port credential guard,match-based exception mapping.get/post/deletereturn decoded arrays;request()is the PSR-7 escape hatch.Endpoint→ResourceEndpoint→CollectionEndpoint) replace every*EndpointInterfaceand capability trait.Resourceresponses with$raw, stamped byRawStamperoutside Valinor's converter pipeline (avoids the per-node reflection leak).Payloadrequests serialized via the Valinor Normalizer (null-strip + camelCase→snake_case +DateTime→ISO; native enums insrc/Enum/).Collectioncarries theX-*header values;paginate()walks pages).pickupPointsreturns a barelist<PickupPoint>;paymentGateways/shipmentTemplateslook up via?id=.ShipmondoException, exposinggetError()for Shipmondo's{"error": "..."}body.Tooling
max(+ strict-rules / phpunit / webmozart-assert).cuyz/valinorpinned^2.2.2 <2.4— 2.4 drops PHP 8.1 (CI 8.1 matrix guards this).^10.5(attribute-based tests;ScriptedHttpClientfake PSR-18, no mocking framework).psr/log— consumers add logging via PSR-18 middleware.Behavior changes / BC breaks (appropriate for the 2.x major)
get()/post()/delete()returnarray(wasResponseInterface).pickupPoints()->search(PickupPointSearch): list<PickupPoint>(wasget(): Collection).Collection::first()returnsnull(wasfalse);CollectiongainstotalCount.SalesOrder.orderIdnow required.Verification
composer phpunit— 60 tests, 132 assertions (3 live tests skipped)composer analyse— PHPStan level max, no errorscomposer check-style,vendor/bin/rector --dry-run,composer validate --strict,composer normalize— cleanvendor/bin/infection— MSI 58% / covered 69% (thresholds 50/60)