Skip to content

Commit 3d1e382

Browse files
author
cowork-bot
committed
cowork-bot: deep-diff nested component properties + flag $ref retargets
1 parent 5e055b1 commit 3d1e382

2 files changed

Lines changed: 257 additions & 0 deletions

File tree

‎src/api_contract_guardian/diff.py‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,110 @@ def _diff_schema_details(
985985
)
986986
)
987987

988+
# Deep structural diff of NESTED object / array-of-object properties.
989+
# Previously _diff_schema_details only compared a component's DIRECT
990+
# properties (type/format/enum); a nested object property whose sub-fields
991+
# changed (e.g. a required field added deep inside) was reported as "no
992+
# change" -- the silent-green failure class this tool exists to catch.
993+
# Reuse the proven recursive inline-schema walker (conservative: treats all
994+
# required/presence changes as BREAKING, since a component may back a request
995+
# body). Only the nested sub-schemas are walked here, so this never
996+
# double-reports the component's own top-level properties already checked above.
997+
for prop_name in old_props:
998+
if prop_name not in new_props:
999+
continue
1000+
old_sub = old_props[prop_name] or {}
1001+
new_sub = new_props[prop_name] or {}
1002+
if not isinstance(old_sub, dict) or not isinstance(new_sub, dict):
1003+
continue
1004+
is_nested_object = (
1005+
(old_sub.get("type") == "object" and new_sub.get("type") == "object")
1006+
or ("properties" in old_sub and "properties" in new_sub)
1007+
)
1008+
if is_nested_object:
1009+
_diff_inline_schema(
1010+
f"{schema_path}.properties.{prop_name}",
1011+
old_sub,
1012+
new_sub,
1013+
result,
1014+
is_request=True,
1015+
depth=1,
1016+
)
1017+
continue
1018+
if old_sub.get("type") == "array" and new_sub.get("type") == "array":
1019+
old_items = old_sub.get("items") or {}
1020+
new_items = new_sub.get("items") or {}
1021+
if isinstance(old_items, dict) and isinstance(new_items, dict) and (
1022+
(old_items.get("type") == "object" and new_items.get("type") == "object")
1023+
or ("properties" in old_items and "properties" in new_items)
1024+
):
1025+
_diff_inline_schema(
1026+
f"{schema_path}.properties.{prop_name}.items",
1027+
old_items,
1028+
new_items,
1029+
result,
1030+
is_request=True,
1031+
depth=1,
1032+
)
1033+
1034+
# A property that is a $ref (or array of $ref) into another component was
1035+
# previously ignored entirely: retargeting or dropping the ref produced no
1036+
# change record. Flag ref-target changes at the property's OWN path. The
1037+
# referenced target's internals are already diffed by _diff_schemas at
1038+
# components.schemas.<Target>, so this never double-reports them -- it only
1039+
# surfaces that THIS property's reference changed.
1040+
for prop_name in set(old_props) | set(new_props):
1041+
_flag_schema_ref_prop(
1042+
schema_path, prop_name, old_props.get(prop_name), new_props.get(prop_name), result
1043+
)
1044+
1045+
1046+
def _flag_schema_ref_prop(
1047+
schema_path: str,
1048+
prop_name: str,
1049+
old_prop: Any,
1050+
new_prop: Any,
1051+
result: DiffResult,
1052+
) -> None:
1053+
"""Flag a component property whose ``$ref`` target changed or was dropped.
1054+
1055+
Refs into ``components.schemas`` are intentionally NOT followed into the
1056+
target's internals (``_diff_schemas`` already diffs the target at its own
1057+
path); we only report that the reference here changed, which is independent
1058+
information not otherwise surfaced.
1059+
"""
1060+
1061+
def ref_of(sub: Any) -> Any:
1062+
if isinstance(sub, dict):
1063+
return sub.get("$ref")
1064+
return None
1065+
1066+
old_ref = ref_of(old_prop)
1067+
new_ref = ref_of(new_prop)
1068+
# Array whose items are a $ref.
1069+
if old_ref is None and isinstance(old_prop, dict) and old_prop.get("type") == "array":
1070+
old_ref = ref_of(old_prop.get("items"))
1071+
if new_ref is None and isinstance(new_prop, dict) and new_prop.get("type") == "array":
1072+
new_ref = ref_of(new_prop.get("items"))
1073+
1074+
if old_ref is None and new_ref is None:
1075+
return
1076+
1077+
prop_path = f"{schema_path}.properties.{prop_name}"
1078+
if old_ref != new_ref:
1079+
result.changes.append(
1080+
Change(
1081+
kind="schema_ref_changed",
1082+
severity=Severity.DANGEROUS,
1083+
path=prop_path,
1084+
description=(
1085+
f"$ref property '{prop_name}' target changed from '{old_ref}' to '{new_ref}'"
1086+
),
1087+
old_value=old_ref,
1088+
new_value=new_ref,
1089+
)
1090+
)
1091+
9881092

9891093
def _diff_security_schemes(
9901094
old: dict[str, Any],

‎tests/test_diff.py‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,159 @@ def test_property_no_longer_required_non_breaking(self):
663663
result = diff_specs(old, new)
664664
assert any(c.kind == "property_no_longer_required" for c in result.changes)
665665

666+
def test_nested_object_property_required_change_is_breaking(self):
667+
# A required field added DEEP inside a nested object property was
668+
# previously reported as "no change" (silent-green gap at the
669+
# component level). It must now surface as a breaking change.
670+
old = _make_spec(
671+
schemas={
672+
"User": {
673+
"type": "object",
674+
"properties": {
675+
"id": {"type": "string"},
676+
"address": {
677+
"type": "object",
678+
"properties": {
679+
"street": {"type": "string"},
680+
"zip": {"type": "string"},
681+
},
682+
},
683+
},
684+
}
685+
}
686+
)
687+
new = _make_spec(
688+
schemas={
689+
"User": {
690+
"type": "object",
691+
"properties": {
692+
"id": {"type": "string"},
693+
"address": {
694+
"type": "object",
695+
"required": ["zip"],
696+
"properties": {
697+
"street": {"type": "string"},
698+
"zip": {"type": "string"},
699+
},
700+
},
701+
},
702+
}
703+
}
704+
)
705+
result = diff_specs(old, new)
706+
became_required = [
707+
c for c in result.changes if c.kind == "request_property_became_required"
708+
]
709+
assert became_required, "nested required change must be detected"
710+
assert any(
711+
"address" in c.path and "zip" in c.path for c in became_required
712+
), "change must be reported at the nested path"
713+
assert all(c.severity.value == "breaking" for c in became_required), (
714+
"nested required change is breaking"
715+
)
716+
717+
def test_nested_array_item_property_required_change_is_breaking(self):
718+
old = _make_spec(
719+
schemas={
720+
"Cart": {
721+
"type": "object",
722+
"properties": {
723+
"items": {
724+
"type": "array",
725+
"items": {
726+
"type": "object",
727+
"properties": {"sku": {"type": "string"}},
728+
},
729+
},
730+
},
731+
}
732+
}
733+
)
734+
new = _make_spec(
735+
schemas={
736+
"Cart": {
737+
"type": "object",
738+
"properties": {
739+
"items": {
740+
"type": "array",
741+
"items": {
742+
"type": "object",
743+
"required": ["sku"],
744+
"properties": {"sku": {"type": "string"}},
745+
},
746+
},
747+
},
748+
}
749+
}
750+
)
751+
result = diff_specs(old, new)
752+
became_required = [
753+
c for c in result.changes if c.kind == "request_property_became_required"
754+
]
755+
assert became_required, "nested array-item required change must be detected"
756+
assert any("items" in c.path and "sku" in c.path for c in became_required)
757+
758+
def test_component_ref_property_target_change_is_dangerous(self):
759+
# A component property whose $ref target was retargeted must be
760+
# flagged as DANGEROUS (it was previously silently ignored at the
761+
# component level).
762+
old = _make_spec(
763+
schemas={
764+
"Order": {
765+
"type": "object",
766+
"properties": {
767+
"customer": {"$ref": "#/components/schemas/CustomerV1"},
768+
},
769+
},
770+
"CustomerV1": {"type": "object"},
771+
"CustomerV2": {"type": "object"},
772+
}
773+
)
774+
new = _make_spec(
775+
schemas={
776+
"Order": {
777+
"type": "object",
778+
"properties": {
779+
"customer": {"$ref": "#/components/schemas/CustomerV2"},
780+
},
781+
},
782+
"CustomerV1": {"type": "object"},
783+
"CustomerV2": {"type": "object"},
784+
}
785+
)
786+
result = diff_specs(old, new)
787+
ref_changed = [c for c in result.changes if c.kind == "schema_ref_changed"]
788+
assert ref_changed, "ref-target change must be detected"
789+
assert any(c.severity.value == "dangerous" for c in ref_changed)
790+
assert any("Order" in c.path and "customer" in c.path for c in ref_changed)
791+
792+
def test_component_ref_property_removed_is_dangerous(self):
793+
old = _make_spec(
794+
schemas={
795+
"Order": {
796+
"type": "object",
797+
"properties": {
798+
"customer": {"$ref": "#/components/schemas/Customer"},
799+
},
800+
},
801+
"Customer": {"type": "object"},
802+
}
803+
)
804+
new = _make_spec(
805+
schemas={
806+
"Order": {
807+
"type": "object",
808+
"properties": {
809+
"customer": {"type": "object"},
810+
},
811+
},
812+
"Customer": {"type": "object"},
813+
}
814+
)
815+
result = diff_specs(old, new)
816+
ref_changed = [c for c in result.changes if c.kind == "schema_ref_changed"]
817+
assert ref_changed, "dropped $ref must be detected"
818+
666819
def test_property_removed_breaking_if_required(self):
667820
old = _make_spec(
668821
schemas={

0 commit comments

Comments
 (0)