Skip to content

Commit 590855c

Browse files
FanouZeng-TTdamaz91
FanouZeng-TT
authored and
damaz91
committed
fix: enforce additionalProperties:false in generated models
1 parent b0d4924 commit 590855c

5 files changed

Lines changed: 317 additions & 6 deletions

File tree

‎postprocess_models.py‎

Lines changed: 122 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
"""Post-generation fixes for constraints datamodel-code-generator ignores.
1616
17-
Four constraint families are handled:
17+
Six constraint families are handled:
1818
1919
* ``minProperties`` on an object schema WITH declared properties is dropped by
2020
the generator (issue #49): every field is optional, so an empty instance
@@ -74,6 +74,12 @@
7474
then injects a ``model_validator(mode="after")``. More complex conditions are
7575
skipped rather than approximated.
7676
77+
* ``additionalProperties: false`` on an object schema with named properties is
78+
normally overridden by the generator's ``--extra-fields=allow`` flag. The
79+
script detects schemas with ``additionalProperties: false`` and flips their
80+
generated ``model_config`` to ``extra="forbid"`` while preserving
81+
``extra="allow"`` on sibling models in the same module.
82+
7783
Runs from generate_models.sh between generation and formatting; idempotent.
7884
"""
7985

@@ -1004,16 +1010,129 @@ def _patch_unique_items():
10041010
return unique_patched, 0
10051011

10061012

1013+
def find_extra_forbid_class_names(schema_dir):
1014+
"""Map generated class names for objects that forbid unknown keys.
1015+
1016+
The gap this targets: an object schema that declares
1017+
``additionalProperties: false`` AND carries named ``properties`` is still
1018+
emitted by the generator as ``BaseModel(extra="allow")`` (generation runs
1019+
with ``--extra-fields=allow``), so unknown keys are silently retained in
1020+
``model_extra`` instead of being rejected. The rule is mechanical: an
1021+
object node with ``additionalProperties is False`` and non-empty named
1022+
``properties`` maps to its generated class name via its ``title`` (root
1023+
objects) or its property path (untitled nested objects, e.g.
1024+
``allows_multi_destination`` -> ``AllowsMultiDestination``).
1025+
"""
1026+
found = set()
1027+
1028+
def visit(node, class_name):
1029+
if not isinstance(node, dict):
1030+
if isinstance(node, list):
1031+
for item in node:
1032+
visit(item, class_name)
1033+
return
1034+
effective = (
1035+
_alias_name(node["title"]) if node.get("title") else class_name
1036+
)
1037+
if (
1038+
node.get("additionalProperties") is False
1039+
and isinstance(node.get("properties"), dict)
1040+
and node["properties"]
1041+
):
1042+
found.add(effective)
1043+
for name, child in (node.get("properties") or {}).items():
1044+
visit(child, _to_camel_case(name))
1045+
1046+
for path in sorted(Path(schema_dir).rglob("*.json")):
1047+
try:
1048+
schema = json.loads(path.read_text(encoding="utf-8"))
1049+
except (OSError, json.JSONDecodeError):
1050+
continue
1051+
if not isinstance(schema, dict):
1052+
continue
1053+
root_name = (
1054+
_alias_name(schema["title"])
1055+
if schema.get("title")
1056+
else _to_camel_case(path.stem)
1057+
)
1058+
visit(schema, root_name)
1059+
return found
1060+
1061+
1062+
def inject_extra_forbid(source, class_name):
1063+
"""Flip the target class's ``extra="allow"`` config to ``extra="forbid"``.
1064+
1065+
Only the named class's own ``model_config`` is changed (its body, from the
1066+
``class`` statement to the next top-level ``class``/``def``), so sibling
1067+
classes in the same module keep ``extra="allow"``. The source is returned
1068+
unchanged when the class is absent or already ``extra="forbid"``.
1069+
"""
1070+
head = re.search(
1071+
rf"^class {re.escape(class_name)}\(BaseModel\):", source, re.M
1072+
)
1073+
if not head:
1074+
return source
1075+
rest = source[head.end() :]
1076+
next_top = re.search(r"^(?=class |def )", rest, re.M)
1077+
body_end = len(rest) if next_top is None else next_top.start()
1078+
body = rest[:body_end]
1079+
if 'extra="allow"' not in body:
1080+
return source
1081+
new_body = body.replace('extra="allow"', 'extra="forbid"', 1)
1082+
return source[: head.end()] + new_body + rest[body_end:]
1083+
1084+
1085+
def _patch_extra_forbid():
1086+
"""Inject extra="forbid" on models whose schema forbids unknown keys."""
1087+
class_names = find_extra_forbid_class_names(SCHEMA_DIR)
1088+
if not class_names:
1089+
sys.stdout.write(
1090+
"postprocess: no additionalProperties:false models found\n"
1091+
)
1092+
return 0, 0
1093+
patched = 0
1094+
for class_name in sorted(class_names):
1095+
hits = []
1096+
for path in sorted(OUTPUT_DIR.rglob("*.py")):
1097+
source = path.read_text(encoding="utf-8")
1098+
if not re.search(
1099+
rf"^class {re.escape(class_name)}\(", source, re.M
1100+
):
1101+
continue
1102+
updated = inject_extra_forbid(source, class_name)
1103+
if updated != source:
1104+
path.write_text(updated, encoding="utf-8")
1105+
patched += 1
1106+
hits.append(path)
1107+
label = ", ".join(str(h) for h in hits) or "NO GENERATED CLASS FOUND"
1108+
sys.stdout.write(f" extra=forbid on '{class_name}' -> {label}\n")
1109+
if not hits:
1110+
sys.stderr.write(
1111+
f" ! '{class_name}' has no generated class; "
1112+
"constraint not enforced\n"
1113+
)
1114+
return patched, 1
1115+
return patched, 0
1116+
1117+
10071118
def main():
10081119
"""Main entry point to scan schemas and patch generated models."""
10091120
patched_mp, rc_mp = _patch_min_properties()
10101121
patched_pn, rc_pn = _patch_property_names()
10111122
patched_ac, rc_ac = _patch_array_contains()
10121123
patched_cr, rc_cr = _patch_conditional_required()
10131124
patched_ui, rc_ui = _patch_unique_items()
1014-
total = patched_mp + patched_pn + patched_ac + patched_cr + patched_ui
1125+
patched_ef, rc_ef = _patch_extra_forbid()
1126+
total = (
1127+
patched_mp
1128+
+ patched_pn
1129+
+ patched_ac
1130+
+ patched_cr
1131+
+ patched_ui
1132+
+ patched_ef
1133+
)
10151134
sys.stdout.write(f"postprocess: {total} module(s) patched\n")
1016-
return rc_mp or rc_pn or rc_ac or rc_cr or rc_ui
1135+
return rc_mp or rc_pn or rc_ac or rc_cr or rc_ui or rc_ef
10171136

10181137

10191138
if __name__ == "__main__":

‎src/ucp_sdk/models/schemas/shopping/types/business_fulfillment_config.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class AllowsMultiDestination(BaseModel):
2929
"""
3030

3131
model_config = ConfigDict(
32-
extra="allow",
32+
extra="forbid",
3333
)
3434
shipping: bool | None = None
3535
"""

‎src/ucp_sdk/models/schemas/shopping/types/error_response.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class ErrorResponse(BaseModel):
3030
"""
3131

3232
model_config = ConfigDict(
33-
extra="allow",
33+
extra="forbid",
3434
)
3535
ucp: ucp_1.UcpMetadata
3636
"""

‎src/ucp_sdk/models/schemas/shopping/types/merchant_fulfillment_config.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class AllowsMultiDestination(BaseModel):
2929
"""
3030

3131
model_config = ConfigDict(
32-
extra="allow",
32+
extra="forbid",
3333
)
3434
shipping: bool | None = None
3535
"""

‎tests/test_codegen_pipeline.py‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,5 +1472,197 @@ def test_brands_accepts_unique_and_none(self) -> None:
14721472
self.assertIsNone(Constraints().brands)
14731473

14741474

1475+
class AdditionalPropertiesForbidFinderTest(unittest.TestCase):
1476+
"""additionalProperties:false objects map to generated class names."""
1477+
1478+
def test_root_titled_object(self) -> None:
1479+
with tempfile.TemporaryDirectory() as tmp:
1480+
Path(tmp, "error_response.json").write_text(
1481+
json.dumps(
1482+
{
1483+
"title": "Error Response",
1484+
"type": "object",
1485+
"additionalProperties": False,
1486+
"properties": {"messages": {"type": "array"}},
1487+
}
1488+
),
1489+
encoding="utf-8",
1490+
)
1491+
names = postprocess_models.find_extra_forbid_class_names(Path(tmp))
1492+
self.assertEqual(names, {"ErrorResponse"})
1493+
1494+
def test_nested_untitled_object_uses_property_path(self) -> None:
1495+
with tempfile.TemporaryDirectory() as tmp:
1496+
Path(tmp, "merchant_fulfillment_config.json").write_text(
1497+
json.dumps(
1498+
{
1499+
"title": "Merchant Fulfillment Config",
1500+
"type": "object",
1501+
"properties": {
1502+
"allows_multi_destination": {
1503+
"type": "object",
1504+
"additionalProperties": False,
1505+
"properties": {"shipping": {"type": "boolean"}},
1506+
}
1507+
},
1508+
}
1509+
),
1510+
encoding="utf-8",
1511+
)
1512+
names = postprocess_models.find_extra_forbid_class_names(Path(tmp))
1513+
self.assertEqual(names, {"AllowsMultiDestination"})
1514+
1515+
def test_loose_and_map_objects_are_excluded(self) -> None:
1516+
with tempfile.TemporaryDirectory() as tmp:
1517+
Path(tmp, "open.json").write_text(
1518+
json.dumps(
1519+
{
1520+
"title": "Open Object",
1521+
"type": "object",
1522+
"properties": {"a": {"type": "string"}},
1523+
}
1524+
),
1525+
encoding="utf-8",
1526+
)
1527+
Path(tmp, "map.json").write_text(
1528+
json.dumps(
1529+
{
1530+
"title": "Map Object",
1531+
"type": "object",
1532+
"additionalProperties": {"type": "string"},
1533+
"properties": {"a": {"type": "string"}},
1534+
}
1535+
),
1536+
encoding="utf-8",
1537+
)
1538+
names = postprocess_models.find_extra_forbid_class_names(Path(tmp))
1539+
self.assertEqual(names, set())
1540+
1541+
1542+
class AdditionalPropertiesForbidInjectorTest(unittest.TestCase):
1543+
"""The injector flips only the target class's model_config to forbid."""
1544+
1545+
SOURCE = '''\
1546+
class AllowsMultiDestination(BaseModel):
1547+
"""
1548+
Permits multiple destinations per method type.
1549+
"""
1550+
1551+
model_config = ConfigDict(
1552+
extra="allow",
1553+
)
1554+
shipping: bool | None = None
1555+
1556+
1557+
class MerchantFulfillmentConfig(BaseModel):
1558+
"""
1559+
Merchant's fulfillment configuration.
1560+
"""
1561+
1562+
model_config = ConfigDict(
1563+
extra="allow",
1564+
)
1565+
allows_multi_destination: AllowsMultiDestination | None = None
1566+
'''
1567+
1568+
def test_flips_only_target_class(self) -> None:
1569+
updated = postprocess_models.inject_extra_forbid(
1570+
self.SOURCE, "AllowsMultiDestination"
1571+
)
1572+
# Target class body now forbids extra keys.
1573+
self.assertIn('extra="forbid"', updated)
1574+
# The sibling class in the same module keeps extra="allow".
1575+
sibling = """class MerchantFulfillmentConfig(BaseModel):
1576+
\"\"\"
1577+
Merchant's fulfillment configuration.
1578+
\"\"\"
1579+
1580+
model_config = ConfigDict(
1581+
extra="allow",
1582+
)"""
1583+
self.assertIn(sibling, updated)
1584+
1585+
def test_idempotent_after_flip(self) -> None:
1586+
once = postprocess_models.inject_extra_forbid(
1587+
self.SOURCE, "AllowsMultiDestination"
1588+
)
1589+
twice = postprocess_models.inject_extra_forbid(
1590+
once, "AllowsMultiDestination"
1591+
)
1592+
self.assertEqual(once, twice)
1593+
1594+
def test_unknown_class_untouched(self) -> None:
1595+
self.assertEqual(
1596+
postprocess_models.inject_extra_forbid(self.SOURCE, "Nope"),
1597+
self.SOURCE,
1598+
)
1599+
1600+
1601+
@unittest.skipUnless(
1602+
HAVE_SDK, "requires the installed package (pip install -e .)"
1603+
)
1604+
class AdditionalPropertiesForbidSemanticTest(unittest.TestCase):
1605+
"""Committed models reject unknown keys on additionalProperties:false."""
1606+
1607+
def test_error_response_rejects_unknown_keys(self) -> None:
1608+
from ucp_sdk.models.schemas.shopping.types.error_response import (
1609+
ErrorResponse,
1610+
)
1611+
1612+
with self.assertRaises(ValidationError):
1613+
ErrorResponse.model_validate(
1614+
{
1615+
"ucp": {"version": "2026-04-08", "status": "error"},
1616+
"messages": [
1617+
{
1618+
"type": "error",
1619+
"code": "not_found",
1620+
"severity": "unrecoverable",
1621+
"content": "boom",
1622+
}
1623+
],
1624+
"bogus": "x",
1625+
}
1626+
)
1627+
1628+
def test_error_response_accepts_declared_fields(self) -> None:
1629+
from ucp_sdk.models.schemas.shopping.types.error_response import (
1630+
ErrorResponse,
1631+
)
1632+
1633+
obj = ErrorResponse.model_validate(
1634+
{
1635+
"ucp": {"version": "2026-04-08", "status": "error"},
1636+
"messages": [
1637+
{
1638+
"type": "error",
1639+
"code": "not_found",
1640+
"severity": "unrecoverable",
1641+
"content": "boom",
1642+
}
1643+
],
1644+
}
1645+
)
1646+
self.assertEqual(obj.messages[0].content, "boom")
1647+
1648+
def test_allows_multi_destination_rejects_unknown_keys(self) -> None:
1649+
from ucp_sdk.models.schemas.shopping.types.merchant_fulfillment_config import (
1650+
AllowsMultiDestination,
1651+
)
1652+
1653+
with self.assertRaises(ValidationError):
1654+
AllowsMultiDestination.model_validate(
1655+
{"shipping": True, "bogus": "x"}
1656+
)
1657+
1658+
def test_sibling_config_keeps_extra_allow(self) -> None:
1659+
from ucp_sdk.models.schemas.shopping.types.merchant_fulfillment_config import (
1660+
MerchantFulfillmentConfig,
1661+
)
1662+
1663+
config = MerchantFulfillmentConfig.model_validate({"bogus": "x"})
1664+
self.assertEqual(config.model_extra, {"bogus": "x"})
1665+
1666+
14751667
if __name__ == "__main__":
14761668
unittest.main()

0 commit comments

Comments
 (0)