diff --git a/docs/extending-types.md b/docs/extending-types.md index 840ac081..8cf7b692 100644 --- a/docs/extending-types.md +++ b/docs/extending-types.md @@ -4,13 +4,14 @@ ADCP types represent the standardized protocol schema. However, your implementat This guide shows how to extend ADCP types safely while maintaining protocol compliance. -> **Pydantic v2 serialization note:** Pydantic v2 uses a Rust-backed serializer. When a parent -> model calls `model_dump()`, Pydantic serializes nested child instances using its own compiled -> pipeline — it does **not** call Python-level `model_dump()` overrides on child objects. This -> means that if you override `model_dump()` on a child class, that override will not fire when -> the child is serialized as part of a parent. Use `Field(exclude=True)` for field-level -> exclusion (works automatically at every nesting depth) or `@model_serializer` with -> `serialize_as_any=True` for custom Python logic (covered below). +> **Pydantic v2 serialization note:** Pydantic v2 uses a Rust-backed serializer that +> serializes nested child instances using the declared schema of the parent's field, not the +> child's `model_dump()` override. `AdCPBaseModel.model_dump()` and `model_dump_json()` set +> `serialize_as_any=True` by default so that subclass `@model_serializer` overrides do fire +> through base-typed parent fields, and `Field(exclude=True)` keeps internal fields off the +> wire at every nesting depth. Adopters do **not** need to write parent-side `model_dump` +> overrides to walk children — Pydantic does the walking; this guide covers the two seams +> (`Field(exclude=True)` and `@model_serializer`) that hook into it. ## Field-Level Exclusion with `Field(exclude=True)` — Recommended @@ -61,11 +62,10 @@ For cases where you need Python-level transformation logic beyond field exclusio output, conditional inclusion, derived computed fields — use Pydantic's `@model_serializer(mode='wrap')`. -**Important:** When a parent field is annotated as the base type (`creatives: list[Creative]`), -Pydantic's Rust serializer uses the declared type's schema and the subclass `@model_serializer` -does **not** fire automatically. You must pass `serialize_as_any=True` to the parent's -`model_dump()` call to apply subclass serializers from a parent. If you control the field -annotation you can also declare it as the concrete subclass type. +When the parent extends `AdCPBaseModel` (which all SDK-generated response types do), the +parent's `model_dump()` defaults `serialize_as_any=True`, so subclass `@model_serializer` +overrides fire automatically through base-typed parent fields. No call-site kwarg is +required. ```python from typing import Any @@ -91,20 +91,20 @@ class InternalCreative(Creative): c = InternalCreative(creative_id="c-1", variants=[], source_label="HD_VIDEO") c.model_dump() # {"creative_id": "c-1", "variants": [], "source_label": "hd_video"} -# Nested in a parent with a base-type annotation: +# Nested under an AdCPBaseModel parent with a base-type annotation: class CreativePayload(AdCPBaseModel): creatives: list[Creative] # declared as base type payload = CreativePayload(creatives=[c]) payload.model_dump() -# {"creatives": [{"creative_id": "c-1", "variants": []}]} -# source_label absent, serializer skipped — Pydantic uses Creative's declared schema. - -payload.model_dump(serialize_as_any=True) # {"creatives": [{"creative_id": "c-1", "variants": [], "source_label": "hd_video"}]} -# serializer fired, source_label present and normalized. +# Subclass serializer fired automatically — AdCPBaseModel.model_dump() defaults +# serialize_as_any=True. Pass serialize_as_any=False explicitly to suppress it. ``` +If your parent extends plain `pydantic.BaseModel` (not `AdCPBaseModel`), you must pass +`serialize_as_any=True` yourself — the default kwarg only ships on AdCP types. + ## Migrating from Manual `model_dump()` Dispatch Overrides A common pattern in early SDK integrations is writing a parent override that manually re-calls @@ -139,7 +139,8 @@ class InternalCreative(Creative): ```python # ✅ Move the logic to the child via @model_serializer. -# Call model_dump(serialize_as_any=True) on the parent to apply it. +# AdCPBaseModel parents default serialize_as_any=True so the subclass serializer +# fires automatically — no call-site kwarg needed. class InternalCreative(Creative): @model_serializer(mode="wrap") def _serialize(self, handler: Any, info: SerializationInfo) -> dict[str, Any]: @@ -147,11 +148,18 @@ class InternalCreative(Creative): # ... custom logic here ... return result -# Parent: no model_dump() override; caller passes serialize_as_any=True. +# Parent: no model_dump() override needed. payload = MyCreativePayload(creatives=[InternalCreative(creative_id="c-1", variants=[])]) -wire = payload.model_dump(serialize_as_any=True) +wire = payload.model_dump() ``` +**Adopter migration note (`serialize_as_any` default flip):** If you have subclasses that +add fields *without* `Field(exclude=True)`, those fields previously dropped at the +wire because the parent's base-type annotation acted as an accidental firewall. They will +now appear in `model_dump()` output. Audit each subclass and mark internal fields with +`Field(exclude=True)`; the field is the canonical wire-isolation contract. If you need the +prior behavior at a specific call site, pass `serialize_as_any=False` explicitly. + ## Basic Pattern: Subclassing Response Types ```python diff --git a/src/adcp/types/base.py b/src/adcp/types/base.py index e3e391d9..005bc6bc 100644 --- a/src/adcp/types/base.py +++ b/src/adcp/types/base.py @@ -232,19 +232,24 @@ class AdCPBaseModel(BaseModel): model_config = ConfigDict(extra=_EXTRA_POLICY) def model_dump(self, **kwargs: Any) -> dict[str, Any]: - # NOTE: Pydantic v2 uses a Rust-backed serializer that does NOT call Python-level - # model_dump() overrides on nested child instances. If a child class overrides - # model_dump() for custom serialization logic, that override will not fire when - # the child is serialized as part of a parent model_dump() call. Use - # Field(exclude=True) for field-level exclusion (works at all nesting depths) or - # @model_serializer for custom output logic. See docs/extending-types.md. + # ``serialize_as_any=True`` makes Pydantic dispatch on the runtime type of + # nested values rather than the declared schema, so subclass + # ``@model_serializer`` overrides fire from a base-typed parent field. Combined + # with ``Field(exclude=True)`` on internal fields (which already works at every + # nesting depth), this removes the parent-side ``model_dump`` boilerplate that + # adopters previously needed to write per response type. See + # docs/extending-types.md. if "exclude_none" not in kwargs: kwargs["exclude_none"] = True + if "serialize_as_any" not in kwargs: + kwargs["serialize_as_any"] = True return super().model_dump(**kwargs) def model_dump_json(self, **kwargs: Any) -> str: if "exclude_none" not in kwargs: kwargs["exclude_none"] = True + if "serialize_as_any" not in kwargs: + kwargs["serialize_as_any"] = True return super().model_dump_json(**kwargs) def model_summary(self) -> str: diff --git a/tests/test_serialize_as_any_default.py b/tests/test_serialize_as_any_default.py new file mode 100644 index 00000000..a69094e2 --- /dev/null +++ b/tests/test_serialize_as_any_default.py @@ -0,0 +1,140 @@ +"""AdCPBaseModel defaults ``serialize_as_any=True`` so that subclass +``@model_serializer`` overrides fire when nested under a base-typed parent +field, and ``Field(exclude=True)`` continues to suppress internal fields at +every nesting depth. + +Together these two guarantees mean adopters never need to write parent-side +``model_dump`` overrides that manually walk children — Pydantic does the +walking, ``Field(exclude=True)`` is the wire-isolation contract, and +``@model_serializer`` is the custom-logic seam. The previous default +(``serialize_as_any=False``) silently dropped subclass-only fields and +skipped subclass serializers under nesting; that footgun is what these +tests pin closed. +""" + +from __future__ import annotations + +import json +from typing import Any + +from pydantic import BaseModel, Field, SerializationInfo, model_serializer + +from adcp.types.base import AdCPBaseModel + + +class _SpecChild(BaseModel): + spec_field: str + + +class _ExtendedChildWithExtraField(_SpecChild): + """Subclass that adds a non-excluded field — appears in serialized output + when the parent dispatches via ``serialize_as_any=True``.""" + + seller_extension: str = "exposed" + + +class _ExtendedChildWithExcludedField(_SpecChild): + """Subclass that adds an internal field marked ``exclude=True`` — must + never appear on the wire, regardless of serialize_as_any state.""" + + internal_id: str = Field(default="internal-42", exclude=True) + + +class _ExtendedChildWithSerializer(_SpecChild): + """Subclass with a wrap-mode model serializer — fires under nesting + once serialize_as_any is set.""" + + @model_serializer(mode="wrap") + def _serialize(self, handler: Any, info: SerializationInfo) -> dict[str, Any]: + result: dict[str, Any] = handler(self, info) + result["normalized_by_subclass"] = True + return result + + +class _Parent(AdCPBaseModel): + """Parent declares the field as the spec base type.""" + + child: _SpecChild + children: list[_SpecChild] = Field(default_factory=list) + + +def test_subclass_serializer_fires_on_singular_field() -> None: + parent = _Parent(child=_ExtendedChildWithSerializer(spec_field="ok")) + dumped = parent.model_dump() + assert dumped["child"] == {"spec_field": "ok", "normalized_by_subclass": True} + + +def test_subclass_serializer_fires_on_list_field() -> None: + parent = _Parent( + child=_SpecChild(spec_field="root"), + children=[ + _ExtendedChildWithSerializer(spec_field="a"), + _ExtendedChildWithSerializer(spec_field="b"), + ], + ) + dumped = parent.model_dump() + for entry in dumped["children"]: + assert entry["normalized_by_subclass"] is True + + +def test_subclass_serializer_fires_in_json_dump() -> None: + """``model_dump_json`` carries the same default.""" + parent = _Parent(child=_ExtendedChildWithSerializer(spec_field="ok")) + dumped = json.loads(parent.model_dump_json()) + assert dumped["child"]["normalized_by_subclass"] is True + + +def test_field_exclude_true_still_suppresses_internal_field() -> None: + """The wire-isolation contract: ``Field(exclude=True)`` keeps internal + state off the wire even when serialize_as_any honors subclass schemas.""" + parent = _Parent(child=_ExtendedChildWithExcludedField(spec_field="ok")) + dumped = parent.model_dump() + assert dumped["child"] == {"spec_field": "ok"} + assert "internal_id" not in dumped["child"] + + +def test_field_exclude_true_works_in_list_field() -> None: + parent = _Parent( + child=_SpecChild(spec_field="root"), + children=[ + _ExtendedChildWithExcludedField(spec_field="a"), + _ExtendedChildWithExcludedField(spec_field="b"), + ], + ) + dumped = parent.model_dump() + for entry in dumped["children"]: + assert "internal_id" not in entry + + +def test_subclass_only_field_appears_under_default() -> None: + """Subclasses that add fields without ``Field(exclude=True)`` will see + those fields appear on the wire under the new default. This pins the + behavior change so adopters who relied on the previous accidental + firewall surface a failing test rather than discovering it in + production.""" + parent = _Parent(child=_ExtendedChildWithExtraField(spec_field="ok")) + dumped = parent.model_dump() + assert dumped["child"] == {"spec_field": "ok", "seller_extension": "exposed"} + + +def test_caller_can_opt_out_with_explicit_kwarg() -> None: + """Adopters who want the prior firewall back can pass + ``serialize_as_any=False`` explicitly — the default only kicks in when + the kwarg is unset.""" + parent = _Parent(child=_ExtendedChildWithExtraField(spec_field="ok")) + dumped = parent.model_dump(serialize_as_any=False) + assert dumped["child"] == {"spec_field": "ok"} + assert "seller_extension" not in dumped["child"] + + +def test_caller_can_still_pass_exclude_none_false() -> None: + """The two defaults are independent — overriding one doesn't disturb + the other.""" + + class _ParentWithOptional(AdCPBaseModel): + child: _SpecChild + optional: str | None = None + + parent = _ParentWithOptional(child=_SpecChild(spec_field="ok")) + dumped = parent.model_dump(exclude_none=False) + assert dumped["optional"] is None