Skip to content

fix(rest/python): stop null-padding unset optional fields in order responses - #224

Open
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:fix/order-null-fix
Open

fix(rest/python): stop null-padding unset optional fields in order responses#224
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:fix/order-null-fix

Conversation

@vishkaty

Copy link
Copy Markdown
Contributor

Fixes #223

Problem

Order responses null-pad unset optional fields the same way checkout responses did before #117: the order schemas type these fields as bare non-nullable string/object/array, so an explicit JSON null fails validation the way an omitted key does not.

Fix (three coordinated sites, same idiom as #117)

  1. services/checkout_service.py:885, inside complete_checkout: the model_dump that persists a freshly created order.
  2. routes/order.py:89, the PUT /orders/{id} update handler: the model_dump that persists an updated order.
  3. routes/ucp_implementation.py:321, order_event_webhook: the model_dump that persists an inbound partner order-event payload.

All three now pass exclude_none=True. The third site is not named in the reproduction in the linked issue (which only drives GET); it surfaced during the class sweep below and is included here rather than filed separately, since it is the same one-line idiom on the same defect class.

No route-level response_model_exclude_none change is needed here (unlike the #117 checkout routes): the order routes already declare response_model=dict[str, Any], so FastAPI does no field-level filtering on the way out either way; the fix has to live at the point the dict is built, which is exactly where these three sites are.

Verification

Wire-level, ucp-sdk 0.4.6 (the version this PR targets; see Testing note below). Fresh server boot, simple_happy_path_client.py to completion, then drove all three write sites directly (GET, PUT round-trip, and a POST to the order-event webhook receiver using the just-fetched order body as a stand-in partner payload). Every resulting body validated against source/schemas/shopping/order.json with two independent validators (the official ucp-schema Rust CLI, and an independent Draft 2020-12 jsonschema referee built over the full vendored schema set) and both spec corpora (2026-04-08, the version this server serves, and 2026-08-25, the current release). All 4 validator x corpus combinations agree on the same 34 violating paths before the fix, 0 after, on all three surfaces.

The 34 violating paths on the pre-fix GET (identical set from both validators, both corpora)
/adjustments
/attribution
/fulfillment/expectations/0/destination/extended_address
/fulfillment/expectations/0/destination/first_name
/fulfillment/expectations/0/destination/last_name
/fulfillment/expectations/0/destination/phone_number
/fulfillment/expectations/0/fulfillable_on
/label
/line_items/0/item/image_url
/line_items/0/parent_id
/line_items/0/quantity/original
/line_items/0/totals/0/display_text
/line_items/0/totals/1/display_text
/line_items/1/item/image_url
/line_items/1/parent_id
/line_items/1/quantity/original
/line_items/1/totals/0/display_text
/line_items/1/totals/1/display_text
/messages
/totals/0/display_text
/totals/0/lines
/totals/1/display_text
/totals/1/lines
/totals/2/display_text
/totals/2/lines
/totals/3/display_text
/totals/3/lines
/ucp/capabilities/dev.ucp.shopping.checkout/0/config
/ucp/capabilities/dev.ucp.shopping.checkout/0/extends
/ucp/capabilities/dev.ucp.shopping.checkout/0/id
/ucp/capabilities/dev.ucp.shopping.checkout/0/schema
/ucp/capabilities/dev.ucp.shopping.checkout/0/spec
/ucp/payment_handlers
/ucp/services

Class sweep

Every order-adjacent response surface, checked with both validators against both corpora:

Surface Result
Order GET, PUT, inbound webhook persist Fixed, this PR (the three sites above)
Outbound order-event webhook body Fixed by the same sites: _notify_webhook delivers the stored row these sites write
Checkout responses Not applicable, fixed by #117; re-driven on the wire today, 0 nulls
Cart responses Not applicable, built over the #117 idiom in #159; re-driven today, 0 nulls

Testing

  • Three new tests in integration_test.py, one per write surface, each written first and watched fail on the null emission, then made green by the fix. Through the classes cart_test.py and signature_integration_test.py reuse, the 3 methods run as 12 additional test instances.
  • Full suite: 245 pre-existing plus the new instances, 257 passed and 12 subtests, 0 failures.
  • Kill test per site: reverting any one site alone, with the tests kept, turns exactly that surface red; restoring returns the suite to green.
  • Pinned pre-commit run on all four changed files: every hook passes.
  • Testing note: the full pytest run used ucp-sdk==0.4.5 because 0.4.6 makes the shared checkout fixture in integration_test.py fail at test runtime on an unrelated discriminated union strictness change, already adapted by the open test(rest/python): adapt checkout fixture to ucp-sdk 0.4.6 #219 (and fix(rest/python): apply update request payment instruments instead of failing construction #220 carries the same adaptation). All wire-level verification above used 0.4.6. The fix itself is version independent, one keyword argument at three call sites.

…sponses

The order schemas type unset optional fields as bare non-nullable
string/object/array, the same shape Universal-Commerce-Protocol#115/Universal-Commerce-Protocol#117 fixed for checkout responses
against the 2026-01-23 schema. The order route has three independent write
sites with the identical gap: the initial persist in complete_checkout, the order
update route, and the order-event webhook receiver each model_dump an Order
without exclude_none, so GET/PUT /orders/{id} and the outbound order-event
webhook body (which reads the same stored row) all emit explicit null for
every field a client or partner left unset.

Adds exclude_none=True to all three model_dump call sites, following the
Universal-Commerce-Protocol#117 idiom exactly. Adds three new tests asserting no null anywhere in the
order GET, order PUT, and post-webhook-receiver bodies; each was watched
red before the corresponding fix and confirmed to go red again with that
fix excised (kill-tested individually).
@damaz91 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Aug 30, 2026
@carolinerg1 carolinerg1 added status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Order responses serialize unset optional fields as null, failing validation against the official ucp-schema

4 participants