Skip to content

Commit 4ec2a57

Browse files
FanouZeng-TTFanouZeng-TT
committed
fix: enforce additionalProperties:false in generated models
1 parent 340a06b commit 4ec2a57

5 files changed

Lines changed: 303 additions & 5 deletions

File tree

‎postprocess_models.py‎

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -815,15 +815,121 @@ def _patch_unique_items():
815815
return unique_patched, 0
816816

817817

818+
def find_extra_forbid_class_names(schema_dir):
819+
"""Map generated class names for objects that forbid unknown keys.
820+
821+
The gap this targets: an object schema that declares
822+
``additionalProperties: false`` AND carries named ``properties`` is still
823+
emitted by the generator as ``BaseModel(extra="allow")`` (generation runs
824+
with ``--extra-fields=allow``), so unknown keys are silently retained in
825+
``model_extra`` instead of being rejected. The rule is mechanical: an
826+
object node with ``additionalProperties is False`` and non-empty named
827+
``properties`` maps to its generated class name via its ``title`` (root
828+
objects) or its property path (untitled nested objects, e.g.
829+
``allows_multi_destination`` -> ``AllowsMultiDestination``).
830+
"""
831+
found = set()
832+
833+
def visit(node, class_name):
834+
if not isinstance(node, dict):
835+
if isinstance(node, list):
836+
for item in node:
837+
visit(item, class_name)
838+
return
839+
effective = (
840+
_alias_name(node["title"]) if node.get("title") else class_name
841+
)
842+
if (
843+
node.get("additionalProperties") is False
844+
and isinstance(node.get("properties"), dict)
845+
and node["properties"]
846+
):
847+
found.add(effective)
848+
for name, child in (node.get("properties") or {}).items():
849+
visit(child, _to_camel_case(name))
850+
851+
for path in sorted(Path(schema_dir).rglob("*.json")):
852+
try:
853+
schema = json.loads(path.read_text(encoding="utf-8"))
854+
except (OSError, json.JSONDecodeError):
855+
continue
856+
if not isinstance(schema, dict):
857+
continue
858+
root_name = (
859+
_alias_name(schema["title"])
860+
if schema.get("title")
861+
else _to_camel_case(path.stem)
862+
)
863+
visit(schema, root_name)
864+
return found
865+
866+
867+
def inject_extra_forbid(source, class_name):
868+
"""Flip the target class's ``extra="allow"`` config to ``extra="forbid"``.
869+
870+
Only the named class's own ``model_config`` is changed (its body, from the
871+
``class`` statement to the next top-level ``class``/``def``), so sibling
872+
classes in the same module keep ``extra="allow"``. The source is returned
873+
unchanged when the class is absent or already ``extra="forbid"``.
874+
"""
875+
head = re.search(
876+
rf"^class {re.escape(class_name)}\(BaseModel\):", source, re.M
877+
)
878+
if not head:
879+
return source
880+
rest = source[head.end() :]
881+
next_top = re.search(r"^(?=class |def )", rest, re.M)
882+
body_end = len(rest) if next_top is None else next_top.start()
883+
body = rest[:body_end]
884+
if 'extra="allow"' not in body:
885+
return source
886+
new_body = body.replace('extra="allow"', 'extra="forbid"', 1)
887+
return source[: head.end()] + new_body + rest[body_end:]
888+
889+
890+
def _patch_extra_forbid():
891+
"""Inject extra="forbid" on models whose schema forbids unknown keys."""
892+
class_names = find_extra_forbid_class_names(SCHEMA_DIR)
893+
if not class_names:
894+
sys.stdout.write(
895+
"postprocess: no additionalProperties:false models found\n"
896+
)
897+
return 0, 0
898+
patched = 0
899+
for class_name in sorted(class_names):
900+
hits = []
901+
for path in sorted(OUTPUT_DIR.rglob("*.py")):
902+
source = path.read_text(encoding="utf-8")
903+
if not re.search(
904+
rf"^class {re.escape(class_name)}\(", source, re.M
905+
):
906+
continue
907+
updated = inject_extra_forbid(source, class_name)
908+
if updated != source:
909+
path.write_text(updated, encoding="utf-8")
910+
patched += 1
911+
hits.append(path)
912+
label = ", ".join(str(h) for h in hits) or "NO GENERATED CLASS FOUND"
913+
sys.stdout.write(f" extra=forbid on '{class_name}' -> {label}\n")
914+
if not hits:
915+
sys.stderr.write(
916+
f" ! '{class_name}' has no generated class; "
917+
"constraint not enforced\n"
918+
)
919+
return patched, 1
920+
return patched, 0
921+
922+
818923
def main():
819924
"""Main entry point to scan schemas and patch generated models."""
820925
patched_mp, rc_mp = _patch_min_properties()
821926
patched_pn, rc_pn = _patch_property_names()
822927
patched_ac, rc_ac = _patch_array_contains()
823928
patched_ui, rc_ui = _patch_unique_items()
824-
total = patched_mp + patched_pn + patched_ac + patched_ui
929+
patched_ef, rc_ef = _patch_extra_forbid()
930+
total = patched_mp + patched_pn + patched_ac + patched_ui + patched_ef
825931
sys.stdout.write(f"postprocess: {total} module(s) patched\n")
826-
return rc_mp or rc_pn or rc_ac or rc_ui
932+
return rc_mp or rc_pn or rc_ac or rc_ui or rc_ef
827933

828934

829935
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
@@ -1360,5 +1360,197 @@ def test_brands_accepts_unique_and_none(self) -> None:
13601360
self.assertIsNone(Constraints().brands)
13611361

13621362

1363+
class AdditionalPropertiesForbidFinderTest(unittest.TestCase):
1364+
"""additionalProperties:false objects map to generated class names."""
1365+
1366+
def test_root_titled_object(self) -> None:
1367+
with tempfile.TemporaryDirectory() as tmp:
1368+
Path(tmp, "error_response.json").write_text(
1369+
json.dumps(
1370+
{
1371+
"title": "Error Response",
1372+
"type": "object",
1373+
"additionalProperties": False,
1374+
"properties": {"messages": {"type": "array"}},
1375+
}
1376+
),
1377+
encoding="utf-8",
1378+
)
1379+
names = postprocess_models.find_extra_forbid_class_names(Path(tmp))
1380+
self.assertEqual(names, {"ErrorResponse"})
1381+
1382+
def test_nested_untitled_object_uses_property_path(self) -> None:
1383+
with tempfile.TemporaryDirectory() as tmp:
1384+
Path(tmp, "merchant_fulfillment_config.json").write_text(
1385+
json.dumps(
1386+
{
1387+
"title": "Merchant Fulfillment Config",
1388+
"type": "object",
1389+
"properties": {
1390+
"allows_multi_destination": {
1391+
"type": "object",
1392+
"additionalProperties": False,
1393+
"properties": {"shipping": {"type": "boolean"}},
1394+
}
1395+
},
1396+
}
1397+
),
1398+
encoding="utf-8",
1399+
)
1400+
names = postprocess_models.find_extra_forbid_class_names(Path(tmp))
1401+
self.assertEqual(names, {"AllowsMultiDestination"})
1402+
1403+
def test_loose_and_map_objects_are_excluded(self) -> None:
1404+
with tempfile.TemporaryDirectory() as tmp:
1405+
Path(tmp, "open.json").write_text(
1406+
json.dumps(
1407+
{
1408+
"title": "Open Object",
1409+
"type": "object",
1410+
"properties": {"a": {"type": "string"}},
1411+
}
1412+
),
1413+
encoding="utf-8",
1414+
)
1415+
Path(tmp, "map.json").write_text(
1416+
json.dumps(
1417+
{
1418+
"title": "Map Object",
1419+
"type": "object",
1420+
"additionalProperties": {"type": "string"},
1421+
"properties": {"a": {"type": "string"}},
1422+
}
1423+
),
1424+
encoding="utf-8",
1425+
)
1426+
names = postprocess_models.find_extra_forbid_class_names(Path(tmp))
1427+
self.assertEqual(names, set())
1428+
1429+
1430+
class AdditionalPropertiesForbidInjectorTest(unittest.TestCase):
1431+
"""The injector flips only the target class's model_config to forbid."""
1432+
1433+
SOURCE = '''\
1434+
class AllowsMultiDestination(BaseModel):
1435+
"""
1436+
Permits multiple destinations per method type.
1437+
"""
1438+
1439+
model_config = ConfigDict(
1440+
extra="allow",
1441+
)
1442+
shipping: bool | None = None
1443+
1444+
1445+
class MerchantFulfillmentConfig(BaseModel):
1446+
"""
1447+
Merchant's fulfillment configuration.
1448+
"""
1449+
1450+
model_config = ConfigDict(
1451+
extra="allow",
1452+
)
1453+
allows_multi_destination: AllowsMultiDestination | None = None
1454+
'''
1455+
1456+
def test_flips_only_target_class(self) -> None:
1457+
updated = postprocess_models.inject_extra_forbid(
1458+
self.SOURCE, "AllowsMultiDestination"
1459+
)
1460+
# Target class body now forbids extra keys.
1461+
self.assertIn('extra="forbid"', updated)
1462+
# The sibling class in the same module keeps extra="allow".
1463+
sibling = """class MerchantFulfillmentConfig(BaseModel):
1464+
\"\"\"
1465+
Merchant's fulfillment configuration.
1466+
\"\"\"
1467+
1468+
model_config = ConfigDict(
1469+
extra="allow",
1470+
)"""
1471+
self.assertIn(sibling, updated)
1472+
1473+
def test_idempotent_after_flip(self) -> None:
1474+
once = postprocess_models.inject_extra_forbid(
1475+
self.SOURCE, "AllowsMultiDestination"
1476+
)
1477+
twice = postprocess_models.inject_extra_forbid(
1478+
once, "AllowsMultiDestination"
1479+
)
1480+
self.assertEqual(once, twice)
1481+
1482+
def test_unknown_class_untouched(self) -> None:
1483+
self.assertEqual(
1484+
postprocess_models.inject_extra_forbid(self.SOURCE, "Nope"),
1485+
self.SOURCE,
1486+
)
1487+
1488+
1489+
@unittest.skipUnless(
1490+
HAVE_SDK, "requires the installed package (pip install -e .)"
1491+
)
1492+
class AdditionalPropertiesForbidSemanticTest(unittest.TestCase):
1493+
"""Committed models reject unknown keys on additionalProperties:false."""
1494+
1495+
def test_error_response_rejects_unknown_keys(self) -> None:
1496+
from ucp_sdk.models.schemas.shopping.types.error_response import (
1497+
ErrorResponse,
1498+
)
1499+
1500+
with self.assertRaises(ValidationError):
1501+
ErrorResponse.model_validate(
1502+
{
1503+
"ucp": {"version": "2026-04-08", "status": "error"},
1504+
"messages": [
1505+
{
1506+
"type": "error",
1507+
"code": "not_found",
1508+
"severity": "unrecoverable",
1509+
"content": "boom",
1510+
}
1511+
],
1512+
"bogus": "x",
1513+
}
1514+
)
1515+
1516+
def test_error_response_accepts_declared_fields(self) -> None:
1517+
from ucp_sdk.models.schemas.shopping.types.error_response import (
1518+
ErrorResponse,
1519+
)
1520+
1521+
obj = ErrorResponse.model_validate(
1522+
{
1523+
"ucp": {"version": "2026-04-08", "status": "error"},
1524+
"messages": [
1525+
{
1526+
"type": "error",
1527+
"code": "not_found",
1528+
"severity": "unrecoverable",
1529+
"content": "boom",
1530+
}
1531+
],
1532+
}
1533+
)
1534+
self.assertEqual(obj.messages[0].content, "boom")
1535+
1536+
def test_allows_multi_destination_rejects_unknown_keys(self) -> None:
1537+
from ucp_sdk.models.schemas.shopping.types.merchant_fulfillment_config import (
1538+
AllowsMultiDestination,
1539+
)
1540+
1541+
with self.assertRaises(ValidationError):
1542+
AllowsMultiDestination.model_validate(
1543+
{"shipping": True, "bogus": "x"}
1544+
)
1545+
1546+
def test_sibling_config_keeps_extra_allow(self) -> None:
1547+
from ucp_sdk.models.schemas.shopping.types.merchant_fulfillment_config import (
1548+
MerchantFulfillmentConfig,
1549+
)
1550+
1551+
config = MerchantFulfillmentConfig.model_validate({"bogus": "x"})
1552+
self.assertEqual(config.model_extra, {"bogus": "x"})
1553+
1554+
13631555
if __name__ == "__main__":
13641556
unittest.main()

0 commit comments

Comments
 (0)