From bb4fef9d9d92008db705e8378c2ea883b7d3e748 Mon Sep 17 00:00:00 2001 From: Vishal Katyal Date: Wed, 22 Jul 2026 11:12:36 -0400 Subject: [PATCH 1/2] fix(rest/python): deliver the order object as the webhook body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The order webhook (orderEvent) is defined in rest.openapi.json with a request body of $ref: order — the delivered body must be an order object. The server instead posted a custom envelope {event_type, checkout_id, order}, and posted it even when there was no order (body = null). Post the order object as the JSON body, carry the event type in an X-Event-Type header (per the reporter's suggestion), and skip delivery when there is no order. Adds two tests that capture the delivered request: the body validates as an Order with all required top-level fields and no envelope keys, the event type is in X-Event-Type, and no delivery occurs without an order. Fixes #135. --- rest/python/server/integration_test.py | 147 ++++++++++++++++++ .../server/services/checkout_service.py | 31 +++- 2 files changed, 170 insertions(+), 8 deletions(-) diff --git a/rest/python/server/integration_test.py b/rest/python/server/integration_test.py index d91fc56d..afab73d7 100644 --- a/rest/python/server/integration_test.py +++ b/rest/python/server/integration_test.py @@ -19,6 +19,7 @@ import shutil import tempfile from collections.abc import AsyncGenerator +from unittest import mock import uuid from absl import flags @@ -26,7 +27,10 @@ import db import dependencies from fastapi.testclient import TestClient +from models import UnifiedCheckout from server.server import app +from services.checkout_service import CheckoutService +from services.fulfillment_service import FulfillmentService from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import create_async_engine from sqlalchemy.orm import sessionmaker @@ -55,6 +59,7 @@ from ucp_sdk.models.schemas.shopping.fulfillment import ( Checkout as FulfillmentCheckout, ) +from ucp_sdk.models.schemas.shopping.order import Order from ucp_sdk.models.schemas.shopping.order import PlatformSchema from ucp_sdk.models.schemas.shopping.types import ( fulfillment_group_create_request as fulfillment_group_create_req, @@ -75,6 +80,34 @@ FLAGS = flags.FLAGS +class _CapturingAsyncClient: + """Fake httpx.AsyncClient that records outbound webhook POSTs for tests.""" + + def __init__(self, sink: list[dict]) -> None: + """Record captured requests into the provided sink list.""" + self._sink = sink + + async def __aenter__(self) -> "_CapturingAsyncClient": + """Enter the async context, returning this capturing client.""" + return self + + async def __aexit__(self, *exc_info: object) -> bool: + """Exit the async context without suppressing exceptions.""" + return False + + async def post( + self, + url: str, + json: object = None, + headers: dict[str, str] | None = None, + timeout: float | None = None, + ) -> None: + """Capture a POST (URL, JSON body, headers) instead of sending it.""" + self._sink.append( + {"url": url, "json": json, "headers": dict(headers or {})} + ) + + class TestCheckout( BuyerConsentCheckoutResp, FulfillmentCheckout, @@ -591,6 +624,120 @@ def test_cancel_checkout(self) -> None: self.assertEqual(response.status_code, 409) self.assertIn("Cannot cancel checkout", response.json()["detail"]) + def _notify_and_capture( + self, checkout: UnifiedCheckout, event_type: str + ) -> list[dict]: + """Fire _notify_webhook with httpx stubbed and return captured POSTs.""" + captured: list[dict] = [] + + async def run() -> None: + async with ( + self.products_session_factory() as products_session, + self.transactions_session_factory() as transactions_session, + ): + service = CheckoutService( + FulfillmentService(), + products_session, + transactions_session, + "http://testserver", + ) + with mock.patch( + "services.checkout_service.httpx.AsyncClient", + lambda *args, **kwargs: _CapturingAsyncClient(captured), + ): + await service._notify_webhook(checkout, event_type) + + asyncio.run(run()) + return captured + + def test_webhook_delivers_the_bare_order_as_body(self) -> None: + """The order-event webhook body is the order object, per rest.openapi.json. + + webhooks.orderEvent.post.requestBody references #/components/schemas/order, + so the delivered JSON must be the order itself (every required top-level + field present) with the event type carried in the X-Event-Type header -- + never a custom {event_type, checkout_id, order} envelope. + """ + with self.client: + # Drive a real create + complete so the server persists a real order. + payload = self._create_checkout_payload( + "wh_order_placed", [("rose", "Red Rose", 1000, 1)] + ) + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers(idempotency_key="wh1", request_id="wh1"), + json=payload.model_dump(mode="json", exclude_none=True), + ) + self.assertEqual(response.status_code, 201, response.text) + + payment_payload = self._create_payment_payload() + response = self.client.post( + "/checkout-sessions/wh_order_placed/complete", + headers=self._get_headers(idempotency_key="wh2", request_id="wh2"), + json=payment_payload, + ) + self.assertEqual(response.status_code, 200, response.text) + + checkout = UnifiedCheckout.model_validate(response.json()) + self.assertIsNotNone( + checkout.order, "completed checkout must carry an order" + ) + checkout.platform = PlatformSchema( + webhook_url="https://platform.example/ucp-webhook" + ) + + captured = self._notify_and_capture(checkout, "order_placed") + + self.assertEqual(len(captured), 1, "exactly one webhook must be delivered") + delivered = captured[0] + self.assertEqual(delivered["url"], "https://platform.example/ucp-webhook") + # The event type travels in the header, not the body. + self.assertEqual(delivered["headers"].get("X-Event-Type"), "order_placed") + + body = delivered["json"] + # The body IS an order: it validates and carries every required field. + Order.model_validate(body) + for field in ( + "ucp", + "id", + "checkout_id", + "permalink_url", + "line_items", + "fulfillment", + "currency", + "totals", + ): + self.assertIn(field, body, f"order body missing required '{field}'") + # And it is NOT the old {event_type, checkout_id, order} envelope. + self.assertNotIn("event_type", body) + self.assertNotIn("order", body) + + def test_webhook_is_skipped_when_there_is_no_order(self) -> None: + """No webhook is delivered when the checkout has no order to send. + + The body must always be a valid order, so an absent order must never be + posted (the old envelope posted a body of {"order": null}). + """ + with self.client: + payload = self._create_checkout_payload( + "wh_no_order", [("rose", "Red Rose", 1000, 1)] + ) + response = self.client.post( + "/checkout-sessions", + headers=self._get_headers(idempotency_key="wh3", request_id="wh3"), + json=payload.model_dump(mode="json", exclude_none=True), + ) + self.assertEqual(response.status_code, 201, response.text) + # A created-but-not-completed checkout has no order yet. + checkout = UnifiedCheckout.model_validate(response.json()) + self.assertIsNone(checkout.order) + checkout.platform = PlatformSchema( + webhook_url="https://platform.example/ucp-webhook" + ) + + captured = self._notify_and_capture(checkout, "order_placed") + self.assertEqual(captured, [], "no webhook may be sent without an order") + if __name__ == "__main__": absltest.main() diff --git a/rest/python/server/services/checkout_service.py b/rest/python/server/services/checkout_service.py index 9b07a3da..f2ed1c53 100644 --- a/rest/python/server/services/checkout_service.py +++ b/rest/python/server/services/checkout_service.py @@ -807,26 +807,41 @@ async def complete_checkout( return checkout async def _notify_webhook(self, checkout: Checkout, event_type: str) -> None: - """Notifies the configured webhook of an event.""" + """Notifies the configured webhook of an order event. + + Per the UCP REST OpenAPI (``webhooks.orderEvent``), the request body is the + order object itself (``#/components/schemas/order``); the event type is + conveyed out of band in the ``X-Event-Type`` header. The body must always + be a valid order, so no notification is sent when there is no order to + deliver. + """ if not checkout.platform or not checkout.platform.webhook_url: return - webhook_url = str(checkout.platform.webhook_url) order_data = None if checkout.order and checkout.order.id: order_data = await db.get_order( self.transactions_session, checkout.order.id ) - payload = { - "event_type": event_type, - "checkout_id": checkout.id, - "order": order_data, - } + if not order_data: + logger.warning( + "Skipping %s webhook for checkout %s: no order to deliver", + event_type, + checkout.id, + ) + return + + webhook_url = str(checkout.platform.webhook_url) try: async with httpx.AsyncClient() as client: - await client.post(webhook_url, json=payload, timeout=5.0) + await client.post( + webhook_url, + json=order_data, + headers={"X-Event-Type": event_type}, + timeout=5.0, + ) except Exception as e: # pylint: disable=broad-exception-caught logger.error("Failed to notify webhook at %s: %s", webhook_url, e) From 128b6e14881c0882f4a0e521594c5529a5d947fe Mon Sep 17 00:00:00 2001 From: damaz91 Date: Thu, 23 Jul 2026 13:31:38 +0000 Subject: [PATCH 2/2] Refactor integration tests to use respx Address review comment suggesting using respx to simplify mocking of httpx. Removed custom _CapturingAsyncClient and updated _notify_and_capture to use respx.mock. TAG=agy CONV=94ff3e71-a493-4097-9827-22e9aeda899c --- rest/python/server/integration_test.py | 50 +++++++++----------------- rest/python/server/pyproject.toml | 1 + 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/rest/python/server/integration_test.py b/rest/python/server/integration_test.py index 10743fb0..6250da72 100644 --- a/rest/python/server/integration_test.py +++ b/rest/python/server/integration_test.py @@ -15,11 +15,11 @@ """Integration tests for the UCP SDK Server.""" import asyncio +from collections.abc import AsyncGenerator +import json from pathlib import Path import shutil import tempfile -from collections.abc import AsyncGenerator -from unittest import mock import uuid from absl import flags @@ -27,6 +27,7 @@ import db import dependencies from fastapi.testclient import TestClient +import respx from models import UnifiedCheckout from server.server import app from services.checkout_service import CheckoutService @@ -80,34 +81,6 @@ FLAGS = flags.FLAGS -class _CapturingAsyncClient: - """Fake httpx.AsyncClient that records outbound webhook POSTs for tests.""" - - def __init__(self, sink: list[dict]) -> None: - """Record captured requests into the provided sink list.""" - self._sink = sink - - async def __aenter__(self) -> "_CapturingAsyncClient": - """Enter the async context, returning this capturing client.""" - return self - - async def __aexit__(self, *exc_info: object) -> bool: - """Exit the async context without suppressing exceptions.""" - return False - - async def post( - self, - url: str, - json: object = None, - headers: dict[str, str] | None = None, - timeout: float | None = None, - ) -> None: - """Capture a POST (URL, JSON body, headers) instead of sending it.""" - self._sink.append( - {"url": url, "json": json, "headers": dict(headers or {})} - ) - - class TestCheckout( BuyerConsentCheckoutResp, FulfillmentCheckout, @@ -641,11 +614,20 @@ async def run() -> None: transactions_session, "http://testserver", ) - with mock.patch( - "services.checkout_service.httpx.AsyncClient", - lambda *args, **kwargs: _CapturingAsyncClient(captured), - ): + with respx.mock: + route = respx.post().respond(200) await service._notify_webhook(checkout, event_type) + if route.called: + for call in route.calls: + request = call.request + body = json.loads(request.content) if request.content else None + captured.append( + { + "url": str(request.url), + "json": body, + "headers": request.headers, + } + ) asyncio.run(run()) return captured diff --git a/rest/python/server/pyproject.toml b/rest/python/server/pyproject.toml index 00ce80c0..d9818dc1 100644 --- a/rest/python/server/pyproject.toml +++ b/rest/python/server/pyproject.toml @@ -27,6 +27,7 @@ dev = [ "pytest>=8.0.0", "httpx>=0.26.0", "ruff>=0.14.11", + "respx>=0.21.0", ] [build-system]