diff --git a/postprocess_models.py b/postprocess_models.py index 84d35e7..b708a97 100644 --- a/postprocess_models.py +++ b/postprocess_models.py @@ -14,16 +14,23 @@ """Post-generation fixes for constraints datamodel-code-generator ignores. -Seven 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 - passes validation in violation of the schema. (``minProperties`` on a - free-form object property is already handled natively — the generator maps it - to ``Field(min_length=...)`` on the dict field.) The script scans the - preprocessed schemas for root-level ``minProperties`` constraints and injects - a ``model_validator(mode="after")`` into the matching generated classes. - JSON Schema counts the keys present on the object, so the validator counts +Eight constraint families are handled: + +* ``minProperties`` / ``maxProperties`` on an object schema WITH declared + properties are dropped by the generator: every field is optional, so an + empty instance (or, for ``maxProperties``, an over-full one) passes + validation in violation of the schema. ``minProperties`` support (issue + #49, PR #55) never grew a ``maxProperties`` counterpart, so + ``location_serves.json``'s ``maxProperties: 1`` ("the Platform MUST + supply exactly one target form") went unenforced even though its sibling + ``minProperties: 1`` on the same schema was already caught. (Either bound + on a free-form object property is already handled natively — the + generator maps it to ``Field(min_length=..., max_length=...)`` on the + dict field.) The script scans the preprocessed schemas for root-level + ``minProperties``/``maxProperties`` constraints and injects a + ``model_validator(mode="after")`` into the matching generated classes, + one validator per bound so both can coexist on the same class. JSON + Schema counts the keys present on the object, so the validator counts provided fields (``model_fields_set``) unioned with extra keys (``model_extra``) — an explicit null is a present key, and unknown keys on ``extra="allow"`` models count too. @@ -120,6 +127,22 @@ def {marker}(self): return self ''' +_MAX_MARKER = "_enforce_max_properties" + +_MAX_VALIDATOR_TEMPLATE = ''' + @model_validator(mode="after") + def {marker}(self): + """JSON Schema maxProperties: allow at most {maximum} + provided {properties_noun}.""" + provided = self.model_fields_set | set(self.model_extra or {{}}) + if len(provided) > {maximum}: + raise ValueError( + "At most {maximum} {properties_noun} may be provided " + "(schema maxProperties={maximum})" + ) + return self +''' + _PROPNAMES_MARKER = "_enforce_property_names" _PROPNAMES_VALIDATOR_TEMPLATE = ''' @@ -238,6 +261,41 @@ def find_root_min_properties(schema_dir): return found +def find_root_max_properties(schema_dir): + """Map schema title -> maxProperties for root-level object constraints. + + Symmetric twin of find_root_min_properties (see #49/#55, which added + minProperties support but never a maxProperties counterpart): + maxProperties on an object schema WITH declared properties is dropped by + the generator the same way minProperties is, so + location_serves.json's maxProperties: 1 ("the Platform MUST supply + exactly one target form") was silently unenforced. As with the min + side, maxProperties on a free-form object property (no named + properties) is already handled natively by the generator + (Field(max_length=...) on the dict field), so it is out of scope here. + """ + found = {} + 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 + maximum = schema.get("maxProperties") + if not isinstance(maximum, int) or not schema.get("properties"): + continue + title = schema.get("title") + if not title: + sys.stderr.write( + f" ! {path}: root maxProperties but no title; " + "cannot map to a class\n" + ) + continue + found[_alias_name(title)] = maximum + return found + + def _ensure_pydantic_import(source, symbol): """Add ``symbol`` to the ``from pydantic import`` line if absent.""" if re.search( @@ -423,6 +481,35 @@ def inject_min_properties(source, class_name, minimum): return _ensure_pydantic_import(out, "model_validator") +def inject_max_properties(source, class_name, maximum): + """Inject the maxProperties validator at the end of ``class_name``. + + Symmetric twin of inject_min_properties; both validators can be + injected into the same class (location_serves.json declares both + minProperties: 1 and maxProperties: 1), each guarded by its own marker + so neither injection clobbers the other or re-runs on a second pass. + """ + if f"def {_MAX_MARKER}(" in source: + return source + class_re = re.compile(rf"^class {re.escape(class_name)}\(", re.M) + match = class_re.search(source) + if not match: + return source + # The class body ends at the next top-level statement or EOF. + tail = re.compile(r"^\S", re.M) + end_match = tail.search(source, match.end()) + end = end_match.start() if end_match else len(source) + method = _MAX_VALIDATOR_TEMPLATE.format( + marker=_MAX_MARKER, + maximum=maximum, + properties_noun="property" if maximum == 1 else "properties", + ) + 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 _extract_contains_groups(schema, path=None): """Collect every array ``contains`` group from a schema's root + allOf. @@ -1137,6 +1224,37 @@ def _patch_min_properties(): return patched, 0 +def _patch_max_properties(): + """Inject maxProperties validators; return (patched_count, exit_code).""" + constraints = find_root_max_properties(SCHEMA_DIR) + if not constraints: + sys.stdout.write( + "postprocess: no root-level maxProperties constraints found\n" + ) + return 0, 0 + patched = 0 + for title, maximum in sorted(constraints.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(title)}\(", source, re.M): + continue + updated = inject_max_properties(source, title, maximum) + if updated != source: + path.write_text(updated, encoding="utf-8") + patched += 1 + hits.append(path) + label = ", ".join(str(h) for h in hits) or "NO GENERATED CLASS FOUND" + sys.stdout.write(f" maxProperties={maximum} on '{title}' -> {label}\n") + if not hits: + sys.stderr.write( + f" ! '{title}' has no generated class; " + "constraint not enforced\n" + ) + return patched, 1 + return patched, 0 + + def _array_contains_targets(): """Resolve ``title -> groups`` for every model needing a contains bound. @@ -1456,6 +1574,7 @@ def _patch_extra_forbid(): def main(): """Main entry point to scan schemas and patch generated models.""" patched_mp, rc_mp = _patch_min_properties() + patched_xp, rc_xp = _patch_max_properties() patched_pn, rc_pn = _patch_property_names() patched_ac, rc_ac = _patch_array_contains() patched_cr, rc_cr = _patch_conditional_required() @@ -1464,6 +1583,7 @@ def main(): patched_ef, rc_ef = _patch_extra_forbid() total = ( patched_mp + + patched_xp + patched_pn + patched_ac + patched_cr @@ -1472,7 +1592,7 @@ def main(): + 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_xp or rc_pn or rc_ac or rc_cr or rc_cb or rc_ui or rc_ef if __name__ == "__main__": diff --git a/src/ucp_sdk/models/schemas/common/types/location_serves.py b/src/ucp_sdk/models/schemas/common/types/location_serves.py index c5c9f4b..4b70a5a 100644 --- a/src/ucp_sdk/models/schemas/common/types/location_serves.py +++ b/src/ucp_sdk/models/schemas/common/types/location_serves.py @@ -84,3 +84,14 @@ def _enforce_min_properties(self): "At least 1 property must be provided (schema minProperties=1)" ) return self + + @model_validator(mode="after") + def _enforce_max_properties(self): + """JSON Schema maxProperties: allow at most 1 + provided property.""" + provided = self.model_fields_set | set(self.model_extra or {}) + if len(provided) > 1: + raise ValueError( + "At most 1 property may be provided (schema maxProperties=1)" + ) + return self diff --git a/src/ucp_sdk/models/schemas/common/types/location_serves_create_request.py b/src/ucp_sdk/models/schemas/common/types/location_serves_create_request.py index 2c1e4fb..89d4b13 100644 --- a/src/ucp_sdk/models/schemas/common/types/location_serves_create_request.py +++ b/src/ucp_sdk/models/schemas/common/types/location_serves_create_request.py @@ -84,3 +84,14 @@ def _enforce_min_properties(self): "At least 1 property must be provided (schema minProperties=1)" ) return self + + @model_validator(mode="after") + def _enforce_max_properties(self): + """JSON Schema maxProperties: allow at most 1 + provided property.""" + provided = self.model_fields_set | set(self.model_extra or {}) + if len(provided) > 1: + raise ValueError( + "At most 1 property may be provided (schema maxProperties=1)" + ) + return self diff --git a/src/ucp_sdk/models/schemas/common/types/location_serves_update_request.py b/src/ucp_sdk/models/schemas/common/types/location_serves_update_request.py index 99f3dd1..94f4ab5 100644 --- a/src/ucp_sdk/models/schemas/common/types/location_serves_update_request.py +++ b/src/ucp_sdk/models/schemas/common/types/location_serves_update_request.py @@ -84,3 +84,14 @@ def _enforce_min_properties(self): "At least 1 property must be provided (schema minProperties=1)" ) return self + + @model_validator(mode="after") + def _enforce_max_properties(self): + """JSON Schema maxProperties: allow at most 1 + provided property.""" + provided = self.model_fields_set | set(self.model_extra or {}) + if len(provided) > 1: + raise ValueError( + "At most 1 property may be provided (schema maxProperties=1)" + ) + return self diff --git a/tests/test_codegen_pipeline.py b/tests/test_codegen_pipeline.py index eba1f0c..0b83e06 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, ) @@ -1438,6 +1445,176 @@ def test_schema_scan_finds_root_constraints(self): self.assertEqual(found, {"Sample": 2}) +class MaxPropertiesInjectorTest(unittest.TestCase): + """maxProperties is the symmetric twin of minProperties (see #49/#55), + but only minProperties was ever scanned: find_root_min_properties reads + schema.get("minProperties") and there is no find_root_max_properties at + all, so location_serves.json's maxProperties: 1 -- "the Platform MUST + supply exactly one target form" -- is silently dropped. This mirrors + InjectorTest above one for one, for the max side. + """ + + SCHEMA = { + "title": "Sample", + "type": "object", + "maxProperties": 1, + "properties": {"a": {"type": "string"}, "b": {"type": "string"}}, + } + + MODULE = ( + "from __future__ import annotations\n" + "\n" + "from pydantic import BaseModel, ConfigDict\n" + "\n" + "\n" + "class Sample(BaseModel):\n" + ' """A sample."""\n' + "\n" + " model_config = ConfigDict(\n" + ' extra="allow",\n' + " )\n" + " a: str | None = None\n" + " b: str | None = None\n" + ) + + def test_injects_validator_with_declared_maximum(self): + out = postprocess_models.inject_max_properties(self.MODULE, "Sample", 1) + self.assertIn("model_validator", out) + self.assertIn("at most 1", out.lower()) + + @unittest.skipUnless(HAVE_SDK, "executing the module needs pydantic") + def test_injected_validator_enforces_count(self): + out = postprocess_models.inject_max_properties(self.MODULE, "Sample", 1) + namespace: dict = {} + exec(compile(out, "", "exec"), namespace) # noqa: S102 + sample_cls = namespace["Sample"] + with self.assertRaises(ValidationError): + sample_cls(a="one", b="two") + sample_cls(a="only-one") + sample_cls() + + def test_injection_is_idempotent(self): + once = postprocess_models.inject_max_properties( + self.MODULE, "Sample", 1 + ) + twice = postprocess_models.inject_max_properties(once, "Sample", 1) + self.assertEqual(once, twice) + + def test_schema_scan_finds_root_constraints(self): + with tempfile.TemporaryDirectory() as tmp: + sub = Path(tmp) / "sub" + sub.mkdir() + (sub / "sample.json").write_text(json.dumps(self.SCHEMA)) + (sub / "plain.json").write_text( + json.dumps( + {"title": "Plain", "type": "object", "properties": {}} + ) + ) + found = postprocess_models.find_root_max_properties(Path(tmp)) + self.assertEqual(found, {"Sample": 1}) + + def test_schema_scan_ignores_object_without_declared_properties(self): + # Mirrors find_root_min_properties: maxProperties on a free-form + # object property (no named properties) is already handled natively + # by the generator (Field(max_length=...) on the dict field), so a + # bare maxProperties with no properties is out of scope here. + schema = { + "title": "OpenMap", + "type": "object", + "maxProperties": 3, + } + with tempfile.TemporaryDirectory() as tmp: + Path(tmp, "open_map.json").write_text(json.dumps(schema)) + found = postprocess_models.find_root_max_properties(Path(tmp)) + self.assertEqual(found, {}) + + def test_both_bounds_coexist_on_the_same_class(self): + """location_serves.json declares both minProperties: 1 AND + maxProperties: 1 on the same object; both validators must be + injectable into the same class without clobbering each other.""" + module = postprocess_models.inject_min_properties( + self.MODULE, "Sample", 1 + ) + module = postprocess_models.inject_max_properties(module, "Sample", 1) + self.assertIn("_enforce_min_properties", module) + self.assertIn("_enforce_max_properties", module) + if HAVE_SDK: + namespace: dict = {} + exec(compile(module, "", "exec"), namespace) # noqa: S102 + sample_cls = namespace["Sample"] + with self.assertRaises(ValidationError): + sample_cls() + with self.assertRaises(ValidationError): + sample_cls(a="one", b="two") + sample_cls(a="only-one") + + +@unittest.skipUnless( + HAVE_SDK, "requires the installed package (pip install -e .)" +) +class LocationServesMaxPropertiesSemanticTest(unittest.TestCase): + """location_serves.json: "The Platform MUST supply exactly one target + form" -- minProperties: 1 AND maxProperties: 1 together. Only the + minimum was ever enforced (see MaxPropertiesInjectorTest above), so a + map naming both point and address currently validates in violation of + the schema. + """ + + def _location_serves(self): + from ucp_sdk.models.schemas.common.types.location_serves import ( + LocationServes, + ) + + return LocationServes + + def _geo(self): + from ucp_sdk.models.schemas.common.types.geo import Geo + + return Geo + + def _address(self): + from ucp_sdk.models.schemas.common.types.location_serves import ( + Address, + ) + + return Address + + def test_both_point_and_address_rejected(self): + with self.assertRaises(ValidationError): + self._location_serves()( + point=self._geo()(latitude=1.0, longitude=2.0), + address=self._address()(address_country="US"), + ) + + def test_point_only_accepted(self): + location = self._location_serves()( + point=self._geo()(latitude=1.0, longitude=2.0) + ) + self.assertIsNotNone(location.point) + + def test_address_only_accepted(self): + location = self._location_serves()( + address=self._address()(address_country="US") + ) + self.assertIsNotNone(location.address) + + def test_empty_still_rejected_by_the_existing_minimum(self): + # Unaffected by this fix; confirms minProperties: 1 still holds. + with self.assertRaises(ValidationError): + self._location_serves()() + + def test_extension_key_alongside_point_rejected(self): + # extra="allow": an extension form key still counts toward the + # maxProperties=1 total per JSON Schema's key-counting semantics. + with self.assertRaises(ValidationError): + self._location_serves().model_validate( + { + "point": {"latitude": 1.0, "longitude": 2.0}, + "dev.example.custom_target": {"foo": "bar"}, + } + ) + + @unittest.skipUnless( HAVE_SDK, "requires the installed package (pip install -e .)" ) @@ -1870,18 +2047,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 +2224,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 +2245,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 +2264,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 +2295,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"})