diff --git a/postprocess_models.py b/postprocess_models.py index 84d35e7..4e96632 100644 --- a/postprocess_models.py +++ b/postprocess_models.py @@ -14,7 +14,7 @@ """Post-generation fixes for constraints datamodel-code-generator ignores. -Seven constraint families are handled: +Eight constraint families are handled: * ``minProperties`` on an object schema WITH declared properties is dropped by the generator (issue #49): every field is optional, so an empty instance @@ -83,6 +83,24 @@ fields were stripped by request-variant projection is inapplicable rather than malformed and is skipped silently. +* A discriminator retyping an array PROPERTY's items to a schema file + different from the property's own base ``$ref`` is dropped entirely, a + third if/then shape distinct from the required-fields and numeric-bounds + families above. ``fulfillment_method.json``'s ``destinations`` stays typed + to the base ``FulfillmentDestination`` regardless of ``type``, even though + a `shipping` method's destinations are really ``ShippingDestination`` + (postal address fields, `type` const `shipping_address`) and a `pickup` + method's are really ``LocationDestination`` (`type` const + `business_location`) — so a `shipping` method can currently list a + destination typed `business_location` and it validates. Pydantic has no + clean way to retype a field's item type from a source-text splice, so + this is enforced with a runtime check instead of a static type change: + each item is checked against the referenced schema's own (root-level, + post-merge) required keys and const-pinned properties — an approximation, + not a full re-derivation of the retyped type (a schema the retyped file + itself ``allOf``-references, e.g. ``postal_address.json``, is not + inspected). + * ``additionalProperties: false`` on an object schema with named properties is normally overridden by the generator's ``--extra-fields=allow`` flag. The script detects schemas with ``additionalProperties: false`` and flips their @@ -197,6 +215,47 @@ def {marker}(self): return self ''' +_RETYPE_MARKER = "_enforce_conditional_item_retyping" + +_RETYPE_TEMPLATE = ''' + @model_validator(mode="after") + def {marker}(self): + """JSON Schema if/then: approximate a discriminator's array-item + retyping to a different referenced schema, via that schema's own + required keys and const-pinned fields.""" + rules = {rules!r} + for rule in rules: + actual = getattr(self, rule["discriminator"], None) + if actual not in rule["values"]: + continue + for _item in getattr(self, rule["field"], None) or []: + _provided = ( + set(_item.keys()) + if isinstance(_item, dict) + else _item.model_fields_set | set(_item.model_extra or {{}}) + ) + for _required in rule["required"]: + if _required not in _provided: + raise ValueError( + f"Field {{_required!r}} is required for " + f"{{rule['field']}} items when " + f"{{rule['discriminator']}} is {{actual!r}}" + ) + for _const_field, _const_value in rule["consts"].items(): + _actual_value = ( + _item.get(_const_field) + if isinstance(_item, dict) + else getattr(_item, _const_field, None) + ) + if _actual_value != _const_value: + raise ValueError( + f"Field {{_const_field!r}} must equal " + f"{{_const_value!r}} for {{rule['field']}} items " + f"when {{rule['discriminator']}} is {{actual!r}}" + ) + return self +''' + _UNIQUE_VALIDATOR_TEMPLATE = ''' @field_validator("{field}", mode="after") def {marker}_{field}(cls, value): # noqa: N805 @@ -987,6 +1046,207 @@ def inject_conditional_bounds(source, class_name, rules): return _ensure_pydantic_import(out, "model_validator") +def _resolve_referenced_shape(ref, schema_path): + """Load ``ref`` (relative to ``schema_path``) and return its own + (root-level, post-merge) required keys and const-pinned properties. + + Returns ``None`` if the file cannot be loaded. Deliberately shallow: it + reads only the referenced schema's own ``required``/``properties``, not + anything it in turn ``allOf``-references (e.g. shipping_destination.json + ``allOf``-refs postal_address.json, whose fields are not inspected) -- + an approximation, not a full re-derivation of the retyped shape. + """ + file_part = ref.split("#", 1)[0] + if not file_part: + return None + target_path = (Path(schema_path).parent / file_part).resolve() + try: + referenced = json.loads(target_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + sys.stderr.write( + f" ! {schema_path}: retyped $ref {ref!r} could not be loaded; " + "rule skipped\n" + ) + return None + if not isinstance(referenced, dict): + return None + required = sorted( + name for name in referenced.get("required", []) if isinstance(name, str) + ) + consts = { + name: prop["const"] + for name, prop in (referenced.get("properties") or {}).items() + if isinstance(prop, dict) and "const" in prop + } + return {"required": required, "consts": consts} + + +def _is_ref_array(node): + """True when ``node`` is an array property typed via ``items.$ref``.""" + return ( + isinstance(node, dict) + and node.get("type") == "array" + and isinstance(node.get("items"), dict) + and isinstance(node["items"].get("$ref"), str) + ) + + +def _describe_retyping_branch(branch, properties, schema_path): + """Describe one allOf if/then branch that retypes an array property's + items to a schema file different from the property's own base ``$ref``. + + Mechanical and narrow by design, mirroring the other conditional + scanners in this module: a single-key ``const``/``enum`` discriminator + naming a property present on the enclosing object, and a ``then`` that + narrows exactly one array property (also present on the enclosing + object) to a different ``items.$ref``. Anything else -- multiple + discriminators, a non-array or non-$ref field, a `then` naming a field + absent from the enclosing object (a request variant that omits it, as + fulfillment_method_create_request.json does for ``destinations``) -- + returns ``None`` silently: those are either a different rule shape + (left to find_conditional_required/find_conditional_bounds, which scan + the same branches) or legitimately inapplicable, not malformed. + """ + if not isinstance(branch, dict) or set(branch) != {"if", "then"}: + return None + condition = branch["if"] + consequence = branch["then"] + if ( + not isinstance(condition, dict) + or set(condition) != {"properties", "required"} + or not isinstance(consequence, dict) + or set(consequence) != {"properties"} + ): + return None + condition_props = condition["properties"] + condition_required = condition["required"] + if ( + not isinstance(condition_props, dict) + or len(condition_props) != 1 + or not isinstance(condition_required, list) + or len(condition_required) != 1 + ): + return None + discriminator, predicate = next(iter(condition_props.items())) + if condition_required != [discriminator] or not isinstance(predicate, dict): + return None + if set(predicate) == {"const"}: + values = [predicate["const"]] + elif ( + set(predicate) == {"enum"} + and isinstance(predicate["enum"], list) + and predicate["enum"] + ): + values = predicate["enum"] + else: + return None + if discriminator not in properties or any( + not isinstance(value, (str, int, float, bool)) for value in values + ): + return None + consequence_props = consequence["properties"] + if not isinstance(consequence_props, dict) or len(consequence_props) != 1: + return None + field, field_schema = next(iter(consequence_props.items())) + if field not in properties or not _is_ref_array(field_schema): + return None + base_field_schema = properties[field] + if not _is_ref_array(base_field_schema): + return None + base_ref = base_field_schema["items"]["$ref"] + new_ref = field_schema["items"]["$ref"] + if new_ref == base_ref: + return None + target = _resolve_referenced_shape(new_ref, schema_path) + if target is None: + return None + return { + "discriminator": discriminator, + "values": values, + "field": field, + "required": target["required"], + "consts": target["consts"], + } + + +def find_conditional_array_retyping(schema_dir): + """Map generated class names to array-item retyping rules. + + Complements find_conditional_required/find_conditional_bounds, which + only handle a ``then`` that adds required fields or narrows a numeric + range. A ``then`` that instead retypes an array PROPERTY's items to a + schema file different from the property's own base ``$ref`` is a third + shape the generator drops entirely: fulfillment_method.json's + ``destinations`` stays typed to the base FulfillmentDestination + regardless of ``type``, even though a `shipping` method's destinations + are really ShippingDestination (postal address fields, `type` const + `shipping_address`) and a `pickup` method's are really + LocationDestination (`type` const `business_location`). Pydantic has no + clean way to retype a field's item type from a source-text splice, so + this is enforced with a runtime check instead of a static type change: + each item is checked against the referenced schema's own required keys + and const-pinned fields (see _resolve_referenced_shape), an + approximation rather than a full re-derivation of the retyped type. + """ + rules_by_class = {} + + def walk(node, current_class_name, schema_path): + if not isinstance(node, dict): + return + if isinstance(node.get("title"), str): + current_class_name = _alias_name(node["title"]) + properties = node.get("properties") + allof = node.get("allOf") + if isinstance(properties, dict) and isinstance(allof, list): + for branch in allof: + rule = _describe_retyping_branch( + branch, properties, schema_path + ) + if rule is not None and current_class_name is not None: + rules_by_class.setdefault(current_class_name, []).append( + rule + ) + if isinstance(properties, dict): + for name, prop in properties.items(): + walk(prop, _to_camel_case(name), schema_path) + defs = node.get("$defs") + if isinstance(defs, dict): + for def_name, def_node in defs.items(): + walk(def_node, _to_camel_case(def_name), schema_path) + + for path in sorted(Path(schema_dir).rglob("*.json")): + try: + schema = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + continue + if not isinstance(schema, dict): + continue + root_title = schema.get("title") + initial_class = ( + _alias_name(root_title) if root_title else _to_camel_case(path.stem) + ) + walk(schema, initial_class, path) + return rules_by_class + + +def inject_conditional_array_retyping(source, class_name, rules): + """Inject array-item retyping checks into one generated class.""" + class_re = re.compile(rf"^class {re.escape(class_name)}\(", re.M) + match = class_re.search(source) + if not match: + return source + tail = re.compile(r"^\S", re.M) + end_match = tail.search(source, match.end()) + end = end_match.start() if end_match else len(source) + if f"def {_RETYPE_MARKER}(" in source[match.start() : end]: + return source + method = _RETYPE_TEMPLATE.format(marker=_RETYPE_MARKER, rules=rules) + body = source[:end].rstrip("\n") + rest = source[end:] + out = body + "\n" + method + ("\n" + rest if rest else "") + return _ensure_pydantic_import(out, "model_validator") + + def find_unique_items_fields(schema_dir): """Map generated class names to fields carrying ``uniqueItems``. @@ -1320,6 +1580,41 @@ def _patch_conditional_bounds(): return patched, 0 +def _patch_conditional_array_retyping(): + """Inject array-item retyping checks; return counts and status.""" + rules_by_class = find_conditional_array_retyping(SCHEMA_DIR) + if not rules_by_class: + sys.stdout.write( + "postprocess: no conditional array-item retyping rules found\n" + ) + return 0, 0 + patched = 0 + for class_name, rules in sorted(rules_by_class.items()): + hits = [] + for path in sorted(OUTPUT_DIR.rglob("*.py")): + source = path.read_text(encoding="utf-8") + if not re.search( + rf"^class {re.escape(class_name)}\(", source, re.M + ): + continue + updated = inject_conditional_array_retyping( + source, class_name, rules + ) + if updated != source: + path.write_text(updated, encoding="utf-8") + patched += 1 + hits.append(path) + label = ( + ", ".join(str(path) for path in hits) or "NO GENERATED CLASS FOUND" + ) + sys.stdout.write( + f" conditional array-item retyping on '{class_name}' -> {label}\n" + ) + if not hits: + return patched, 1 + return patched, 0 + + def _patch_unique_items(): """Inject uniqueItems validators; return (patched_count, exit_code).""" unique_fields_by_class = find_unique_items_fields(SCHEMA_DIR) @@ -1460,6 +1755,7 @@ def main(): patched_ac, rc_ac = _patch_array_contains() patched_cr, rc_cr = _patch_conditional_required() patched_cb, rc_cb = _patch_conditional_bounds() + patched_rt, rc_rt = _patch_conditional_array_retyping() patched_ui, rc_ui = _patch_unique_items() patched_ef, rc_ef = _patch_extra_forbid() total = ( @@ -1468,11 +1764,12 @@ def main(): + patched_ac + patched_cr + patched_cb + + patched_rt + patched_ui + patched_ef ) sys.stdout.write(f"postprocess: {total} module(s) patched\n") - return rc_mp or rc_pn or rc_ac or rc_cr or rc_cb or rc_ui or rc_ef + return rc_mp or rc_pn or rc_ac or rc_cr or rc_cb or rc_rt or rc_ui or rc_ef if __name__ == "__main__": diff --git a/src/ucp_sdk/models/schemas/shopping/types/fulfillment_method.py b/src/ucp_sdk/models/schemas/shopping/types/fulfillment_method.py index b9f7c06..820f5c5 100644 --- a/src/ucp_sdk/models/schemas/shopping/types/fulfillment_method.py +++ b/src/ucp_sdk/models/schemas/shopping/types/fulfillment_method.py @@ -18,7 +18,7 @@ from __future__ import annotations -from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, model_validator from . import fulfillment_destination, fulfillment_group @@ -57,3 +57,55 @@ class FulfillmentMethod(BaseModel): """ Fulfillment groups for selecting options. Agent sets selected_option_id on groups to choose shipping method. """ + + @model_validator(mode="after") + def _enforce_conditional_item_retyping(self): + """JSON Schema if/then: approximate a discriminator's array-item + retyping to a different referenced schema, via that schema's own + required keys and const-pinned fields.""" + rules = [ + { + "discriminator": "type", + "values": ["shipping"], + "field": "destinations", + "required": ["id", "type"], + "consts": {"type": "shipping_address"}, + }, + { + "discriminator": "type", + "values": ["pickup"], + "field": "destinations", + "required": ["type"], + "consts": {"type": "business_location"}, + }, + ] + for rule in rules: + actual = getattr(self, rule["discriminator"], None) + if actual not in rule["values"]: + continue + for _item in getattr(self, rule["field"], None) or []: + _provided = ( + set(_item.keys()) + if isinstance(_item, dict) + else _item.model_fields_set | set(_item.model_extra or {}) + ) + for _required in rule["required"]: + if _required not in _provided: + raise ValueError( + f"Field {_required!r} is required for " + f"{rule['field']} items when " + f"{rule['discriminator']} is {actual!r}" + ) + for _const_field, _const_value in rule["consts"].items(): + _actual_value = ( + _item.get(_const_field) + if isinstance(_item, dict) + else getattr(_item, _const_field, None) + ) + if _actual_value != _const_value: + raise ValueError( + f"Field {_const_field!r} must equal " + f"{_const_value!r} for {rule['field']} items " + f"when {rule['discriminator']} is {actual!r}" + ) + return self diff --git a/tests/test_codegen_pipeline.py b/tests/test_codegen_pipeline.py index eba1f0c..eb3b7e6 100644 --- a/tests/test_codegen_pipeline.py +++ b/tests/test_codegen_pipeline.py @@ -31,12 +31,19 @@ try: from pydantic import TypeAdapter, ValidationError - from ucp_sdk.models.schemas.shopping.types.description import Description - from ucp_sdk.models.schemas.shopping.types.totals import Totals - from ucp_sdk.models.schemas.shopping.types.totals_create_request import ( + # NOTE(root-cause-0): these paths moved from shopping.types to + # common.types when #87 (2026-08-25 UCP release regen) restructured the + # schema tree. The old paths silently raise ModuleNotFoundError here, + # which the except clause below swallows as HAVE_SDK = False -- so every + # semantic test gated on HAVE_SDK skips instead of running, and CI is + # green on a suite that mostly never executed. See the sibling fixes to + # the other stale shopping.types.* imports later in this file. + from ucp_sdk.models.schemas.common.types.description import Description + from ucp_sdk.models.schemas.common.types.totals import Totals + from ucp_sdk.models.schemas.common.types.totals_create_request import ( TotalsCreateRequest, ) - from ucp_sdk.models.schemas.shopping.types.totals_update_request import ( + from ucp_sdk.models.schemas.common.types.totals_update_request import ( TotalsUpdateRequest, ) @@ -1014,7 +1021,7 @@ class SignalsPropertyNamesTest(unittest.TestCase): """ def _signals(self): - from ucp_sdk.models.schemas.shopping.types.signals import Signals + from ucp_sdk.models.schemas.common.types.signals import Signals return Signals @@ -1053,13 +1060,13 @@ def test_known_named_fields_still_work(self): def test_request_variants_enforce_property_names(self): # The gap and its fix travel to the generated request variants too. - from ucp_sdk.models.schemas.shopping.types.signals_complete_request import ( + from ucp_sdk.models.schemas.common.types.signals_complete_request import ( SignalsCompleteRequest, ) - from ucp_sdk.models.schemas.shopping.types.signals_create_request import ( + from ucp_sdk.models.schemas.common.types.signals_create_request import ( SignalsCreateRequest, ) - from ucp_sdk.models.schemas.shopping.types.signals_update_request import ( + from ucp_sdk.models.schemas.common.types.signals_update_request import ( SignalsUpdateRequest, ) @@ -1376,6 +1383,268 @@ def test_injected_validator_enforces_conditional_bounds(self): total(type="total", amount=-5) +class ConditionalArrayRetypingInjectorTest(unittest.TestCase): + """A discriminator retyping an array property's items to a different + referenced schema file is a third if/then shape, distinct from + conditional-required and conditional-bounds above. This mirrors + fulfillment_method.json: the base `destinations` property is typed via + `items.$ref` to fulfillment_destination.json, but a `shipping` method's + destinations should really be shipping_destination.json items (postal + address fields, `type` const `shipping_address`) and a `pickup` + method's should really be location_destination.json items (`type` + const `business_location`). The generator drops both retyping branches + entirely -- no scanner in this module ever looked for this shape. + """ + + MODULE = ( + "from __future__ import annotations\n" + "\n" + "from pydantic import BaseModel, ConfigDict\n" + "\n" + "from . import destination\n" + "\n" + "\n" + "class Method(BaseModel):\n" + ' model_config = ConfigDict(extra="allow")\n' + " type: str\n" + " destinations: list[destination.Destination] | None = None\n" + ) + RULES = [ + { + "discriminator": "type", + "values": ["shipping"], + "field": "destinations", + "required": ["id", "type"], + "consts": {"type": "shipping_address"}, + }, + { + "discriminator": "type", + "values": ["pickup"], + "field": "destinations", + "required": ["type"], + "consts": {"type": "business_location"}, + }, + ] + + def _schema(self): + return { + "title": "Method", + "type": "object", + "properties": { + "type": {"type": "string"}, + "destinations": { + "type": "array", + "items": {"$ref": "destination.json"}, + }, + }, + "allOf": [ + { + "if": { + "properties": {"type": {"const": "shipping"}}, + "required": ["type"], + }, + "then": { + "properties": { + "destinations": { + "type": "array", + "items": {"$ref": "shipping_destination.json"}, + } + } + }, + }, + { + "if": { + "properties": {"type": {"const": "pickup"}}, + "required": ["type"], + }, + "then": { + "properties": { + "destinations": { + "type": "array", + "items": {"$ref": "location_destination.json"}, + } + } + }, + }, + ], + } + + def _write_schema_tree(self, tmp): + Path(tmp, "method.json").write_text(json.dumps(self._schema())) + Path(tmp, "destination.json").write_text( + json.dumps( + { + "title": "Destination", + "type": "object", + "required": ["id", "type"], + "properties": { + "id": {"type": "string"}, + "type": {"type": "string"}, + }, + } + ) + ) + Path(tmp, "shipping_destination.json").write_text( + json.dumps( + { + "title": "Shipping Destination", + "type": "object", + "required": ["id", "type"], + "properties": { + "id": {"type": "string"}, + "type": {"type": "string", "const": "shipping_address"}, + }, + "allOf": [{"$ref": "postal_address.json"}], + } + ) + ) + Path(tmp, "location_destination.json").write_text( + json.dumps( + { + "title": "Business Location Destination", + "type": "object", + "required": ["type"], + "properties": { + "type": {"type": "string", "const": "business_location"} + }, + "allOf": [{"$ref": "location_summary.json"}], + } + ) + ) + + def test_schema_scan_reads_both_retyping_branches(self): + with tempfile.TemporaryDirectory() as tmp: + self._write_schema_tree(tmp) + found = postprocess_models.find_conditional_array_retyping( + Path(tmp) + ) + self.assertEqual(found, {"Method": self.RULES}) + + def test_schema_scan_ignores_branch_matching_the_base_ref(self): + # A then.properties..items.$ref identical to the base ref is + # not a retype -- nothing to approximate. The sibling pickup branch + # (still a genuine retype) is unaffected. + schema = self._schema() + schema["allOf"][0]["then"]["properties"]["destinations"]["items"][ + "$ref" + ] = "destination.json" + with tempfile.TemporaryDirectory() as tmp: + self._write_schema_tree(tmp) + Path(tmp, "method.json").write_text(json.dumps(schema)) + found = postprocess_models.find_conditional_array_retyping( + Path(tmp) + ) + self.assertEqual(found, {"Method": [self.RULES[1]]}) + + def test_schema_scan_skips_rule_whose_field_was_stripped(self): + # A request variant that omits `destinations` entirely (as + # fulfillment_method_create_request.json does) makes the rule + # inapplicable, not malformed -- no warning, and no rule recorded + # for the variant's own class. + schema = self._schema() + schema["title"] = "Method Create Request" + del schema["properties"]["destinations"] + with tempfile.TemporaryDirectory() as tmp: + self._write_schema_tree(tmp) + Path(tmp, "method_create_request.json").write_text( + json.dumps(schema) + ) + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): + found = postprocess_models.find_conditional_array_retyping( + Path(tmp) + ) + self.assertNotIn("MethodCreateRequest", found) + self.assertEqual(found, {"Method": self.RULES}) + self.assertNotIn("unsupported", stderr.getvalue()) + + def test_schema_scan_warns_when_retyped_ref_cannot_be_loaded(self): + schema = self._schema() + with tempfile.TemporaryDirectory() as tmp: + Path(tmp, "method.json").write_text(json.dumps(schema)) + Path(tmp, "destination.json").write_text( + json.dumps({"title": "Destination", "type": "object"}) + ) + # shipping_destination.json / location_destination.json are + # deliberately absent. + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): + found = postprocess_models.find_conditional_array_retyping( + Path(tmp) + ) + self.assertEqual(found, {}) + self.assertIn("could not be loaded", stderr.getvalue()) + + def test_injection_is_idempotent(self): + once = postprocess_models.inject_conditional_array_retyping( + self.MODULE, "Method", self.RULES + ) + twice = postprocess_models.inject_conditional_array_retyping( + once, "Method", self.RULES + ) + self.assertEqual(once, twice) + + @unittest.skipUnless(HAVE_SDK, "executing the module needs pydantic") + def test_injected_validator_enforces_retyping(self): + module = ( + "from __future__ import annotations\n" + "\n" + "from pydantic import BaseModel, ConfigDict\n" + "\n" + "\n" + "class Destination(BaseModel):\n" + ' model_config = ConfigDict(extra="allow")\n' + " type: str\n" + " id: str\n" + "\n" + "\n" + "class Method(BaseModel):\n" + ' model_config = ConfigDict(extra="allow")\n' + " type: str\n" + " destinations: list[Destination] | None = None\n" + ) + out = postprocess_models.inject_conditional_array_retyping( + module, "Method", self.RULES + ) + namespace: dict = {} + exec(compile(out, "", "exec"), namespace) # noqa: S102 + # Forward reference (from __future__ import annotations): Method's + # "destinations: list[Destination]" annotation resolves once both + # classes exist in the exec'd namespace. + namespace["Method"].model_rebuild(_types_namespace=namespace) + method_cls = namespace["Method"] + destination_cls = namespace["Destination"] + with self.assertRaises(ValidationError): + method_cls( + type="shipping", + destinations=[ + destination_cls(type="business_location", id="d1") + ], + ) + with self.assertRaises(ValidationError): + method_cls( + type="pickup", + destinations=[ + destination_cls(type="shipping_address", id="d1") + ], + ) + method_cls( + type="shipping", + destinations=[destination_cls(type="shipping_address", id="d1")], + ) + method_cls( + type="pickup", + destinations=[destination_cls(type="business_location", id="d1")], + ) + # A type carrying no rule is unconstrained (open vocabulary). + method_cls( + type="courier", + destinations=[destination_cls(type="anything", id="d1")], + ) + # No destinations at all is unconstrained regardless of type. + method_cls(type="shipping") + + class InjectorTest(unittest.TestCase): """The post-generation injector's own behavior.""" @@ -1870,18 +2139,41 @@ def test_injected_validator_rejects_duplicates(self) -> None: class UniqueItemsSemanticTest(unittest.TestCase): """Committed models enforce uniqueItems on declared array fields.""" + # NOTE(root-cause-0): card_payment_instrument.json no longer declares a + # Constraints.brands field (uniqueItems) as of the pinned 2026-08-25 UCP + # schema -- the module now generates only Display, ConstraintTarget and + # CardPaymentInstrument (verified against + # src/ucp_sdk/models/schemas/common/types/card_payment_instrument.py). + # These two tests exercised a schema shape that no longer exists; the + # HAVE_SDK import gate bug (see the top of this file) had been hiding + # that they could not pass, not just that they were unrelated to SDK + # availability. Documented skip rather than silent deletion: the + # uniqueItems mechanism itself stays covered by UniqueItemsInjectorTest + # (injector unit tests) and by other committed models with uniqueItems + # fields (e.g. common.types.constraint_expression, context, + # location_filter, request_constraints). + @unittest.skip( + "card_payment_instrument.Constraints.brands (uniqueItems) was " + "removed from the schema before the pinned 2026-08-25 UCP release; " + "no current committed model at this path carries a brands field" + ) def test_brands_rejects_duplicates(self) -> None: """card_payment_instrument brands rejects duplicate entries.""" - from ucp_sdk.models.schemas.shopping.types.card_payment_instrument import ( + from ucp_sdk.models.schemas.common.types.card_payment_instrument import ( Constraints, ) with self.assertRaisesRegex(ValidationError, "[Uu]nique"): Constraints(brands=["visa", "visa"]) + @unittest.skip( + "card_payment_instrument.Constraints.brands (uniqueItems) was " + "removed from the schema before the pinned 2026-08-25 UCP release; " + "no current committed model at this path carries a brands field" + ) def test_brands_accepts_unique_and_none(self) -> None: """Unique lists and missing values are accepted.""" - from ucp_sdk.models.schemas.shopping.types.card_payment_instrument import ( + from ucp_sdk.models.schemas.common.types.card_payment_instrument import ( Constraints, ) @@ -2024,7 +2316,7 @@ class AdditionalPropertiesForbidSemanticTest(unittest.TestCase): """Committed models reject unknown keys on additionalProperties:false.""" def test_error_response_rejects_unknown_keys(self) -> None: - from ucp_sdk.models.schemas.shopping.types.error_response import ( + from ucp_sdk.models.schemas.common.types.error_response import ( ErrorResponse, ) @@ -2045,7 +2337,7 @@ def test_error_response_rejects_unknown_keys(self) -> None: ) def test_error_response_accepts_declared_fields(self) -> None: - from ucp_sdk.models.schemas.shopping.types.error_response import ( + from ucp_sdk.models.schemas.common.types.error_response import ( ErrorResponse, ) @@ -2064,8 +2356,29 @@ def test_error_response_accepts_declared_fields(self) -> None: ) self.assertEqual(obj.messages[0].content, "boom") + # NOTE(root-cause-0): merchant_fulfillment_config.json was renamed and + # restructured to business_fulfillment_config.json before the pinned + # 2026-08-25 UCP release. The nested additionalProperties:false object + # these tests targeted (allows_multi_destination -> AllowsMultiDestination) + # is gone; the current schema's multi_destination field is a list of + # MultiDestinationItem (extra="allow", no nested forbid object) -- + # verified against + # src/ucp_sdk/models/schemas/shopping/types/business_fulfillment_config.py. + # The HAVE_SDK import gate bug (see the top of this file) had been + # hiding that these two tests could not pass at all, not just that they + # were unrelated to SDK availability. Documented skip rather than silent + # deletion: the additionalProperties:false -> extra="forbid" mechanism + # itself stays covered by test_error_response_rejects_unknown_keys above + # and by AdditionalPropertiesForbidInjectorTest/FinderTest. + @unittest.skip( + "merchant_fulfillment_config.AllowsMultiDestination was removed " + "when the schema was restructured to " + "business_fulfillment_config.MultiDestinationItem before the " + "pinned 2026-08-25 UCP release; no current committed model at " + "this path carries a nested additionalProperties:false object" + ) def test_allows_multi_destination_rejects_unknown_keys(self) -> None: - from ucp_sdk.models.schemas.shopping.types.merchant_fulfillment_config import ( + from ucp_sdk.models.schemas.shopping.types.business_fulfillment_config import ( AllowsMultiDestination, ) @@ -2074,12 +2387,19 @@ def test_allows_multi_destination_rejects_unknown_keys(self) -> None: {"shipping": True, "bogus": "x"} ) + @unittest.skip( + "merchant_fulfillment_config.MerchantFulfillmentConfig was renamed " + "and restructured to business_fulfillment_config." + "BusinessFulfillmentConfig before the pinned 2026-08-25 UCP " + "release; see test_allows_multi_destination_rejects_unknown_keys " + "above" + ) def test_sibling_config_keeps_extra_allow(self) -> None: - from ucp_sdk.models.schemas.shopping.types.merchant_fulfillment_config import ( - MerchantFulfillmentConfig, + from ucp_sdk.models.schemas.shopping.types.business_fulfillment_config import ( + BusinessFulfillmentConfig, ) - config = MerchantFulfillmentConfig.model_validate({"bogus": "x"}) + config = BusinessFulfillmentConfig.model_validate({"bogus": "x"}) self.assertEqual(config.model_extra, {"bogus": "x"}) @@ -2114,5 +2434,97 @@ def test_payment_handler_base_rejects_invalid_version(self) -> None: Base.model_validate({"version": {"not": "a version"}}) +@unittest.skipUnless( + HAVE_SDK, "requires the installed package (pip install -e .)" +) +class FulfillmentMethodDestinationRetypingSemanticTest(unittest.TestCase): + """fulfillment_method.json retypes `destinations` per `type`: a + `shipping` method's destinations are shipping_destination.json items + (`type` const `shipping_address`), a `pickup` method's are + location_destination.json items (`type` const `business_location`). + The committed FulfillmentMethod model, before this fix, accepted any + FulfillmentDestination (bare `type: str`, `id: str`) regardless of the + method's own type, so a `shipping` method could list a + `business_location` destination and it would validate. + """ + + def _method(self): + from ucp_sdk.models.schemas.shopping.types.fulfillment_method import ( + FulfillmentMethod, + ) + + return FulfillmentMethod + + def _destination(self): + from ucp_sdk.models.schemas.shopping.types.fulfillment_destination import ( + FulfillmentDestination, + ) + + return FulfillmentDestination + + def test_shipping_method_with_business_location_destination_rejected( + self, + ): + with self.assertRaises(ValidationError): + self._method()( + id="m1", + type="shipping", + line_item_ids=["li1"], + destinations=[ + self._destination()(type="business_location", id="d1") + ], + ) + + def test_pickup_method_with_shipping_address_destination_rejected(self): + with self.assertRaises(ValidationError): + self._method()( + id="m2", + type="pickup", + line_item_ids=["li1"], + destinations=[ + self._destination()(type="shipping_address", id="d1") + ], + ) + + def test_shipping_method_with_shipping_address_destination_accepted( + self, + ): + method = self._method()( + id="m1", + type="shipping", + line_item_ids=["li1"], + destinations=[ + self._destination()(type="shipping_address", id="d1") + ], + ) + self.assertEqual(method.destinations[0].type, "shipping_address") + + def test_pickup_method_with_business_location_destination_accepted(self): + method = self._method()( + id="m2", + type="pickup", + line_item_ids=["li1"], + destinations=[ + self._destination()(type="business_location", id="d1") + ], + ) + self.assertEqual(method.destinations[0].type, "business_location") + + def test_method_type_outside_the_pinned_vocabulary_is_unconstrained( + self, + ): + # type is an open vocabulary ("Businesses MAY use additional + # values"); only shipping/pickup carry a retyping rule. + self._method()( + id="m3", + type="curbside", + line_item_ids=["li1"], + destinations=[self._destination()(type="anything", id="d1")], + ) + + def test_method_without_destinations_is_unconstrained(self): + self._method()(id="m4", type="shipping", line_item_ids=["li1"]) + + if __name__ == "__main__": unittest.main()