diff --git a/rest/python/client/flower_shop/sample_output/happy_path_dialog.md b/rest/python/client/flower_shop/sample_output/happy_path_dialog.md index 4b482ca..a9b4b35 100644 --- a/rest/python/client/flower_shop/sample_output/happy_path_dialog.md +++ b/rest/python/client/flower_shop/sample_output/happy_path_dialog.md @@ -1353,9 +1353,9 @@ export RESPONSE=$(curl -s -X POST $SERVER_URL/checkout-sessions/$CHECKOUT_ID/com -H 'request-id: f6478591-3679-4459-baa7-7ecddb6a5c95' \ -H 'Content-Type: application/json' \ -d '{ - "risk_signals": { - "ip": "127.0.0.1", - "browser": "python-httpx" + "signals": { + "com.example.ip": "127.0.0.1", + "com.example.browser": "python-httpx" }, "payment": { "instruments": [ diff --git a/rest/python/client/flower_shop/simple_happy_path_client.py b/rest/python/client/flower_shop/simple_happy_path_client.py index 3527b3b..2913b8e 100644 --- a/rest/python/client/flower_shop/simple_happy_path_client.py +++ b/rest/python/client/flower_shop/simple_happy_path_client.py @@ -861,9 +861,9 @@ def main() -> int: } ] }, - "risk_signals": { - "ip": "127.0.0.1", - "browser": "python-httpx", + "signals": { + "com.example.ip": "127.0.0.1", + "com.example.browser": "python-httpx", }, } diff --git a/rest/python/server/integration_test.py b/rest/python/server/integration_test.py index 6f9d095..465d991 100644 --- a/rest/python/server/integration_test.py +++ b/rest/python/server/integration_test.py @@ -319,7 +319,6 @@ def _create_payment_payload(self) -> dict: } ] ), - risk_signals={}, ) return payload.model_dump(mode="json", exclude_none=True) @@ -2224,6 +2223,132 @@ def test_checkout_readiness_lifecycle(self) -> None: self.assertEqual(complete_res.status_code, 200, complete_res.text) self.assertEqual(complete_res.json()["status"], "completed") + def _create_and_get_id(self, checkout_id: str, key: str) -> str: + """Create a checkout and return the server-assigned id.""" + payload = self._create_checkout_payload( + checkout_id, [("rose", "Red Rose", 1000, 1)] + ) + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers(idempotency_key=key, request_id=key), + json=payload.model_dump(mode="json", exclude_none=True), + ) + self.assertEqual(response.status_code, 201, response.text) + return self.get_resource_id(TestCheckout.model_validate(response.json()).id) + + def test_complete_accepts_the_spec_shaped_body(self) -> None: + """Complete Checkout takes the body the spec defines. + + checkout.json marks `payment` complete: required and `signals` optional. + There is no `risk_signals` member anywhere in the specification; it was + replaced by `signals` before the 2026-04-08 release. The handler declared + `risk_signals` as a required body member, so a spec-shaped request was + rejected with 422. + """ + with self.client: + checkout_sid = self._create_and_get_id("spec_body_1", "sb1") + body = { + "payment": { + "instruments": [ + { + "id": "instr_1", + "handler_id": "mock_payment_handler", + "type": "card", + "display": {"brand": "Visa", "last_digits": "1234"}, + "credential": {"type": "token", "token": "success_token"}, + } + ] + }, + "signals": {"com.example.session": "xts-marker-2030"}, + } + response = self.client.post( + f"/checkout-sessions/{checkout_sid}/complete", + headers=self._get_headers(idempotency_key="sb2", request_id="sb2"), + json=body, + ) + self.assertEqual(response.status_code, 200, response.text) + self.assertEqual( + TestCheckout.model_validate(response.json()).status, "completed" + ) + + def test_complete_rejects_a_malformed_payment_with_4xx(self) -> None: + """A malformed payment is a client error, not a server fault. + + The handler built PaymentCreateRequest inside its own body, so pydantic + raised a ValidationError that was not a RequestValidationError and the + request surfaced as a bare 500. + """ + with self.client: + checkout_sid = self._create_and_get_id("bad_payment_1", "bp1") + response = self.client.post( + f"/checkout-sessions/{checkout_sid}/complete", + headers=self._get_headers(idempotency_key="bp2", request_id="bp2"), + json={ + # risk_signals is sent so the request reaches the payment parsing + # rather than being turned away for the missing member. It is a + # tolerated extra once the body binds to the SDK model. + "risk_signals": {}, + "payment": { + "instruments": [ + {"id": "instr_1", "handler_id": 12345, "type": "card"} + ] + }, + }, + ) + self.assertEqual(response.status_code, 422, response.text) + self.assertEqual(response.json()["ucp"]["status"], "error") + + def test_complete_rejects_a_non_list_instruments_with_4xx(self) -> None: + """The same holds when instruments is not an array at all.""" + with self.client: + checkout_sid = self._create_and_get_id("bad_payment_2", "bp3") + response = self.client.post( + f"/checkout-sessions/{checkout_sid}/complete", + headers=self._get_headers(idempotency_key="bp4", request_id="bp4"), + json={ + "risk_signals": {}, + "payment": {"instruments": "xts-not-a-list"}, + }, + ) + self.assertEqual(response.status_code, 422, response.text) + + def test_complete_carries_signals_into_the_request_identity(self) -> None: + """`signals` is part of the request, so it must reach the service. + + The handler bound only `payment` and `risk_signals`, so a top-level + `signals` member was silently discarded. Reusing one idempotency key with + a different `signals` value is then indistinguishable from a replay. With + the body bound to the SDK model it is a conflict. + """ + with self.client: + checkout_sid = self._create_and_get_id("signals_1", "sg1") + base = { + "payment": { + "instruments": [ + { + "id": "instr_1", + "handler_id": "mock_payment_handler", + "type": "card", + "display": {"brand": "Visa", "last_digits": "1234"}, + "credential": {"type": "token", "token": "success_token"}, + } + ] + }, + } + first = self.client.post( + f"/checkout-sessions/{checkout_sid}/complete", + headers=self._get_headers(idempotency_key="sg2", request_id="sg2"), + json=dict(base, signals={"com.example.session": "first-xts"}), + ) + self.assertEqual(first.status_code, 200, first.text) + + second = self.client.post( + f"/checkout-sessions/{checkout_sid}/complete", + headers=self._get_headers(idempotency_key="sg2", request_id="sg3"), + json=dict(base, signals={"com.example.session": "second-xts"}), + ) + self.assertEqual(second.status_code, 409, second.text) + if __name__ == "__main__": absltest.main() diff --git a/rest/python/server/routes/ucp_implementation.py b/rest/python/server/routes/ucp_implementation.py index 1a0460e..77ae161 100644 --- a/rest/python/server/routes/ucp_implementation.py +++ b/rest/python/server/routes/ucp_implementation.py @@ -207,8 +207,7 @@ async def update_checkout( async def complete_checkout( checkout_id: Annotated[str, Path(..., alias="id")], - payment: Annotated[dict[str, Any], Body(...)], - risk_signals: Annotated[dict[str, Any], Body(...)], + checkout_complete: Annotated[CheckoutCompleteRequest, Body(...)], common_headers: Annotated[ dependencies.CommonHeaders, Depends(dependencies.common_headers) ], @@ -216,18 +215,23 @@ async def complete_checkout( checkout_service: Annotated[ CheckoutService, Depends(dependencies.get_checkout_service) ], - checkout_complete: Annotated[CheckoutCompleteRequest | None, Body()] = None, ) -> models.UnifiedCheckout: - """Complete Checkout Implementation.""" + """Complete Checkout Implementation. + + The body is the checkout complete projection: `payment` is required and + `signals` and `attribution` are optional. Binding it to the SDK model means + a malformed payment is answered by the framework as a 422 in the UCP + envelope, rather than raising inside the handler as a 500. + """ del common_headers # Unused - # Parse payment into PaymentCreateRequest - payment_req = PaymentCreateRequest(**payment) + payment_req = PaymentCreateRequest( + **checkout_complete.payment.model_dump(mode="json", exclude_none=True) + ) return await checkout_service.complete_checkout( checkout_id, payment_req, - risk_signals, idempotency_key, checkout_complete=checkout_complete, ) diff --git a/rest/python/server/services/checkout_service.py b/rest/python/server/services/checkout_service.py index f6c1669..4cc90fe 100644 --- a/rest/python/server/services/checkout_service.py +++ b/rest/python/server/services/checkout_service.py @@ -716,7 +716,6 @@ async def complete_checkout( self, checkout_id: str, payment: PaymentCreateRequest, - risk_signals: dict[str, Any], idempotency_key: str, checkout_complete: CheckoutCompleteRequest | None = None, ) -> Checkout: @@ -724,10 +723,10 @@ async def complete_checkout( logger.info("Completing checkout session %s", checkout_id) # Idempotency Check - # Include risk_signals and checkout_complete in the hash + # The whole request body is part of the request identity, so signals and + # attribution are included through checkout_complete. combined_data = { "payment": payment.model_dump(mode="json"), - "risk_signals": risk_signals, "checkout_complete": checkout_complete.model_dump(mode="json") if checkout_complete else None,