fix(rest/python): stop null-padding unset optional fields in order responses - #224
Open
vishkaty wants to merge 1 commit into
Open
fix(rest/python): stop null-padding unset optional fields in order responses#224vishkaty wants to merge 1 commit into
vishkaty wants to merge 1 commit into
Conversation
…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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
nullfails validation the way an omitted key does not.Fix (three coordinated sites, same idiom as #117)
services/checkout_service.py:885, insidecomplete_checkout: the model_dump that persists a freshly created order.routes/order.py:89, thePUT /orders/{id}update handler: the model_dump that persists an updated order.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_nonechange is needed here (unlike the #117 checkout routes): the order routes already declareresponse_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.pyto 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 againstsource/schemas/shopping/order.jsonwith two independent validators (the officialucp-schemaRust CLI, and an independent Draft 2020-12jsonschemareferee 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)
Class sweep
Every order-adjacent response surface, checked with both validators against both corpora:
_notify_webhookdelivers the stored row these sites writeTesting
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 classescart_test.pyandsignature_integration_test.pyreuse, the 3 methods run as 12 additional test instances.pre-commit runon all four changed files: every hook passes.ucp-sdk==0.4.5because 0.4.6 makes the shared checkout fixture inintegration_test.pyfail 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.