From 0da179752ce2da2e8a69464884e04f6beb9f5ceb Mon Sep 17 00:00:00 2001 From: Vishal Katyal Date: Wed, 26 Aug 2026 21:33:59 -0400 Subject: [PATCH 1/2] fix(rest/python): build request variant destinations in the checkout test helper _create_checkout_payload built shipping_destination.ShippingDestination for FulfillmentMethodCreateRequest.destinations, a field typed as the FulfillmentDestinationCreateRequest union. ucp-sdk 0.4.5 resolved that union to the response variants (ShippingDestination, RetailLocation), so the construction validated. 0.4.6 corrected the oneOf/anyOf codegen (python-sdk#83) and the union now resolves to the matching *CreateRequest variants, so the response class no longer validates against a request field. Since ucp-sdk is an unpinned dependency here, a fresh install now resolves 0.4.6 and the shared helper fails for every caller (integration_test.py, cart_test.py, signature_integration_test.py). Swap the import and construction to shipping_destination_create_request.ShippingDestinationCreateRequest, matching the corrected union. --- rest/python/server/integration_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest/python/server/integration_test.py b/rest/python/server/integration_test.py index 55b0ba6..1e55400 100644 --- a/rest/python/server/integration_test.py +++ b/rest/python/server/integration_test.py @@ -81,7 +81,7 @@ line_item_create_request as line_item_create_req, ) from ucp_sdk.models.schemas.shopping.types import ( - shipping_destination as shipping_destination_req, + shipping_destination_create_request as shipping_destination_req, ) FLAGS = flags.FLAGS @@ -250,7 +250,7 @@ def _create_checkout_payload( payment = payment_create_req.PaymentCreateRequest(instruments=[]) # Hierarchical Fulfillment Construction - destination = shipping_destination_req.ShippingDestination( + destination = shipping_destination_req.ShippingDestinationCreateRequest( id="dest_1", address_country="US" ) group = fulfillment_group_create_req.FulfillmentGroupCreateRequest( From 202bc13b5b7f6d8759b3556ca333c97b638df9f3 Mon Sep 17 00:00:00 2001 From: Vishal Katyal Date: Wed, 26 Aug 2026 22:27:57 -0400 Subject: [PATCH 2/2] fix(rest/python): apply update request payment instruments instead of failing construction update_checkout built the response payment as PaymentResponse(instruments=checkout_req.payment.instruments), handing the response model a list of payment_instrument_update_request. SelectedPaymentInstrument instances. PaymentResponse declares that field as list[payment_instrument.SelectedPaymentInstrument], a sibling response class with the same shape but a different identity, so pydantic rejects the construction with an unhandled ValidationError: a bare 500, not the UCP error envelope, on any update whose payment carries a non-empty instruments array. create_checkout builds PaymentResponse the same way from the create request and collapses the same way; every existing test that touches payment sends instruments: [], so neither site was ever exercised with a real instrument. checkout.json marks payment ucp_request: {create: optional, update: optional}, and checkout.md documents that submitting payment populates payment.instruments with the collected instrument data, so update is a normal place for a platform to submit payment and the server must apply it, not 500. Fix both sites by dumping the request-side payment to a dict and letting PaymentResponse parse that, mirroring how buyer, context, signals, and discounts already cross this same request to response boundary elsewhere in this file. Adds test_update_applies_payment_instruments and test_create_applies_payment_instruments to integration_test.py, which signature_integration_test.py and cart_test.py also pick up, giving 8 passing instances across the create and cart-to-checkout paths in both signed and unsigned harness modes. Kill tested: reverting the service change with the tests kept turns all 8 red with the same ValidationError; restoring turns them green. Builds on fix/test-helper-request-variants (samples 9c0ebef), cherry picked here as its own commit, since a fresh ucp-sdk install resolves 0.4.6 and the shared checkout test helper needs that fix to import. --- rest/python/server/integration_test.py | 105 ++++++++++++++++++ .../server/services/checkout_service.py | 23 +++- 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/rest/python/server/integration_test.py b/rest/python/server/integration_test.py index 1e55400..9b17c81 100644 --- a/rest/python/server/integration_test.py +++ b/rest/python/server/integration_test.py @@ -1772,6 +1772,111 @@ def test_validation_failure_answers_with_ucp_envelope(self) -> None: "content must name the offending member", ) + def test_update_applies_payment_instruments(self) -> None: + """An update carrying payment.instruments must apply them, not 500. + + checkout.json marks `payment` as `ucp_request: {update: "optional"}`, and + checkout.md says submitting payment populates payment.instruments with + the collected instrument data -- update is a normal place for a platform + to submit payment. update_checkout built the response instead as + `PaymentResponse(instruments=checkout_req.payment.instruments)`, handing + the response model a list of + payment_instrument_update_request.SelectedPaymentInstrument instances. + The response model is typed for the sibling response class + payment_instrument.SelectedPaymentInstrument, so pydantic rejects the + construction as an unhandled ValidationError -- a bare 500, not the UCP + error envelope, whenever a request supplies a non-empty instruments + array. An update that omits payment (or sends instruments: []) never + exercises the mismatch, which is why this shipped unnoticed. + """ + with self.client: + created = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="pay_upd_1", request_id="pay_upd_1" + ), + json={"line_items": [{"item": {"id": "rose"}, "quantity": 1}]}, + ) + self.assertEqual(created.status_code, 201, f"Response: {created.text}") + checkout_id = self.get_resource_id(created.json()["id"]) + + instrument = { + "id": "instr_upd_1", + "handler_id": "mock_payment_handler", + "type": "card", + "display": {"brand": "Visa", "last_digits": "4242"}, + "selected": True, + } + updated = self.client.put( + f"/checkout-sessions/{checkout_id}", + headers=self._get_headers( + idempotency_key="pay_upd_2", request_id="pay_upd_2" + ), + json={ + "line_items": [{"item": {"id": "rose"}, "quantity": 1}], + "payment": {"instruments": [instrument]}, + }, + ) + self.assertEqual( + updated.status_code, + 200, + f"an update carrying payment.instruments must not 500: {updated.text}", + ) + returned = (updated.json().get("payment") or {}).get("instruments") or [] + self.assertEqual( + len(returned), 1, "the submitted instrument must be applied" + ) + self.assertEqual(returned[0].get("id"), "instr_upd_1") + self.assertEqual(returned[0].get("handler_id"), "mock_payment_handler") + self.assertEqual(returned[0].get("type"), "card") + self.assertEqual( + returned[0].get("display"), + {"brand": "Visa", "last_digits": "4242"}, + ) + self.assertTrue(returned[0].get("selected")) + + def test_create_applies_payment_instruments(self) -> None: + """A create carrying payment.instruments must apply them, not 500. + + Same class as test_update_applies_payment_instruments: create_checkout + builds `PaymentResponse(instruments=checkout_req.payment.instruments)` + from the create request, handing the response model a list of + payment_instrument_create_request.SelectedPaymentInstrument instances + instead of the response class it declares. Every existing create test + that touches payment sends `instruments: []`, so the mismatch never + triggers pydantic's model-type check. + """ + with self.client: + instrument = { + "id": "instr_create_1", + "handler_id": "mock_payment_handler", + "type": "card", + "display": {"brand": "Visa", "last_digits": "4242"}, + "selected": True, + } + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers( + idempotency_key="pay_create_1", request_id="pay_create_1" + ), + json={ + "line_items": [{"item": {"id": "rose"}, "quantity": 1}], + "payment": {"instruments": [instrument]}, + }, + ) + self.assertEqual( + response.status_code, + 201, + f"a create carrying payment.instruments must not 500: {response.text}", + ) + returned = (response.json().get("payment") or {}).get("instruments") or [] + self.assertEqual( + len(returned), 1, "the submitted instrument must be applied" + ) + self.assertEqual(returned[0].get("id"), "instr_create_1") + self.assertEqual(returned[0].get("handler_id"), "mock_payment_handler") + self.assertEqual(returned[0].get("type"), "card") + if __name__ == "__main__": absltest.main() diff --git a/rest/python/server/services/checkout_service.py b/rest/python/server/services/checkout_service.py index ad7f376..5f8cc51 100644 --- a/rest/python/server/services/checkout_service.py +++ b/rest/python/server/services/checkout_service.py @@ -392,10 +392,14 @@ async def create_checkout( {"type": "total", "amount": 0}, ], links=[], + # Same request/response class collision as update_checkout below: + # checkout_req.payment.instruments holds + # payment_instrument_create_request.SelectedPaymentInstrument + # instances, not the payment_instrument.SelectedPaymentInstrument the + # response field declares. Dump to a dict first so PaymentResponse + # parses it rather than rejecting a foreign model instance. payment=PaymentResponse( - instruments=checkout_req.payment.instruments - if checkout_req.payment - else None, + **checkout_req.payment.model_dump(exclude_none=True) ) if checkout_req.payment else None, @@ -529,8 +533,19 @@ async def update_checkout( # is the same defect as the create path. if checkout_req.payment: + # checkout_req.payment.instruments holds + # payment_instrument_update_request.SelectedPaymentInstrument + # instances, a sibling of the response class + # payment_instrument.SelectedPaymentInstrument that PaymentResponse + # declares for the same field. Passing the request instances straight + # through fails pydantic's model-type check (a different class, not a + # dict), which raised an unhandled ValidationError -- a bare 500 -- + # whenever an update carried a non-empty instruments array. Dumping to + # a dict first and letting PaymentResponse parse it mirrors how buyer, + # context, signals, and discounts already cross this same request to + # response boundary below and in create_checkout. existing.payment = PaymentResponse( - instruments=checkout_req.payment.instruments, + **checkout_req.payment.model_dump(exclude_none=True) ) if checkout_req.buyer: