Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Monitor OpenAPI schema diffs between git branches, detect breaking changes, gene
|[![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://github.com/Coding-Dev-Tools/api-contract-guardian/blob/main/LICENSE)
|[![Open Source Alternative](https://img.shields.io/badge/Open_Source_Alternative-%E2%87%92-blue?logo=opensourceinitiative)](https://www.opensourcealternative.to/project/api-contract-guardian)
|[![LibHunt](https://img.shields.io/badge/LibHunt-%E2%87%92-blue?logo=codeigniter)](https://www.libhunt.com/r/Coding-Dev-Tools/api-contract-guardian)
|[![PyPI](https://img.shields.io/pypi/v/api-contract-guardian)](https://pypi.org/project/api-contract-guardian/)
|<!-- Not on public PyPI yet — install via git+; see Installation section -->

**Why API Contract Guardian?**

Expand All @@ -24,11 +24,9 @@ Real-world scenarios:

## Installation

```bash
pip install api-contract-guardian
```
> **Note:** `api-contract-guardian` is not yet on public PyPI. Use one of the methods below.

Or install the latest version directly from GitHub:
Install the latest version directly from GitHub:

```bash
pip install git+https://github.com/Coding-Dev-Tools/api-contract-guardian.git
Expand Down
104 changes: 104 additions & 0 deletions src/api_contract_guardian/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,110 @@ def _diff_schema_details(
)
)

# Deep structural diff of NESTED object / array-of-object properties.
# Previously _diff_schema_details only compared a component's DIRECT
# properties (type/format/enum); a nested object property whose sub-fields
# changed (e.g. a required field added deep inside) was reported as "no
# change" -- the silent-green failure class this tool exists to catch.
# Reuse the proven recursive inline-schema walker (conservative: treats all
# required/presence changes as BREAKING, since a component may back a request
# body). Only the nested sub-schemas are walked here, so this never
# double-reports the component's own top-level properties already checked above.
for prop_name in old_props:
if prop_name not in new_props:
continue
old_sub = old_props[prop_name] or {}
new_sub = new_props[prop_name] or {}
if not isinstance(old_sub, dict) or not isinstance(new_sub, dict):
continue
is_nested_object = (
(old_sub.get("type") == "object" and new_sub.get("type") == "object")
or ("properties" in old_sub and "properties" in new_sub)
)
if is_nested_object:
_diff_inline_schema(
f"{schema_path}.properties.{prop_name}",
old_sub,
new_sub,
result,
is_request=True,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve response semantics for component schema walks

When a component is used only by a response (for example, a GET 200 body), removing a nested field or making it optional is breaking for response consumers. Passing is_request=True instead emits request_property_removed / request_property_no_longer_required as non-breaking, so a component response such as Order.address.zip can be removed without tripping the default breaking-change gate.

Useful? React with 👍 / 👎.

depth=1,
)
continue
if old_sub.get("type") == "array" and new_sub.get("type") == "array":
old_items = old_sub.get("items") or {}
new_items = new_sub.get("items") or {}
if isinstance(old_items, dict) and isinstance(new_items, dict) and (
(old_items.get("type") == "object" and new_items.get("type") == "object")
or ("properties" in old_items and "properties" in new_items)
):
_diff_inline_schema(
f"{schema_path}.properties.{prop_name}.items",
old_items,
new_items,
result,
is_request=True,
depth=1,
)

# A property that is a $ref (or array of $ref) into another component was
# previously ignored entirely: retargeting or dropping the ref produced no
# change record. Flag ref-target changes at the property's OWN path. The
# referenced target's internals are already diffed by _diff_schemas at
# components.schemas.<Target>, so this never double-reports them -- it only
# surfaces that THIS property's reference changed.
for prop_name in set(old_props) | set(new_props):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip added properties when checking ref retargets

For a newly added optional component property whose schema is a $ref, this union includes the property even though it has no old counterpart; _flag_schema_ref_prop then reports a schema_ref_changed from None to the ref as DANGEROUS in addition to the existing non-breaking property_added record. This can incorrectly fail CI configurations using --fail-on-dangerous or --max-dangerous 0 for an additive optional field.

Useful? React with 👍 / 👎.

_flag_schema_ref_prop(
schema_path, prop_name, old_props.get(prop_name), new_props.get(prop_name), result
)


def _flag_schema_ref_prop(
schema_path: str,
prop_name: str,
old_prop: Any,
new_prop: Any,
result: DiffResult,
) -> None:
"""Flag a component property whose ``$ref`` target changed or was dropped.

Refs into ``components.schemas`` are intentionally NOT followed into the
target's internals (``_diff_schemas`` already diffs the target at its own
path); we only report that the reference here changed, which is independent
information not otherwise surfaced.
"""

def ref_of(sub: Any) -> Any:
if isinstance(sub, dict):
return sub.get("$ref")
return None

old_ref = ref_of(old_prop)
new_ref = ref_of(new_prop)
# Array whose items are a $ref.
if old_ref is None and isinstance(old_prop, dict) and old_prop.get("type") == "array":
old_ref = ref_of(old_prop.get("items"))
if new_ref is None and isinstance(new_prop, dict) and new_prop.get("type") == "array":
new_ref = ref_of(new_prop.get("items"))

if old_ref is None and new_ref is None:
return

prop_path = f"{schema_path}.properties.{prop_name}"
if old_ref != new_ref:
result.changes.append(
Change(
kind="schema_ref_changed",
severity=Severity.DANGEROUS,
path=prop_path,
description=(
f"$ref property '{prop_name}' target changed from '{old_ref}' to '{new_ref}'"
),
old_value=old_ref,
new_value=new_ref,
)
)


def _diff_security_schemes(
old: dict[str, Any],
Expand Down
153 changes: 153 additions & 0 deletions tests/test_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,159 @@ def test_property_no_longer_required_non_breaking(self):
result = diff_specs(old, new)
assert any(c.kind == "property_no_longer_required" for c in result.changes)

def test_nested_object_property_required_change_is_breaking(self):
# A required field added DEEP inside a nested object property was
# previously reported as "no change" (silent-green gap at the
# component level). It must now surface as a breaking change.
old = _make_spec(
schemas={
"User": {
"type": "object",
"properties": {
"id": {"type": "string"},
"address": {
"type": "object",
"properties": {
"street": {"type": "string"},
"zip": {"type": "string"},
},
},
},
}
}
)
new = _make_spec(
schemas={
"User": {
"type": "object",
"properties": {
"id": {"type": "string"},
"address": {
"type": "object",
"required": ["zip"],
"properties": {
"street": {"type": "string"},
"zip": {"type": "string"},
},
},
},
}
}
)
result = diff_specs(old, new)
became_required = [
c for c in result.changes if c.kind == "request_property_became_required"
]
assert became_required, "nested required change must be detected"
assert any(
"address" in c.path and "zip" in c.path for c in became_required
), "change must be reported at the nested path"
assert all(c.severity.value == "breaking" for c in became_required), (
"nested required change is breaking"
)

def test_nested_array_item_property_required_change_is_breaking(self):
old = _make_spec(
schemas={
"Cart": {
"type": "object",
"properties": {
"items": {
"type": "array",
"items": {
"type": "object",
"properties": {"sku": {"type": "string"}},
},
},
},
}
}
)
new = _make_spec(
schemas={
"Cart": {
"type": "object",
"properties": {
"items": {
"type": "array",
"items": {
"type": "object",
"required": ["sku"],
"properties": {"sku": {"type": "string"}},
},
},
},
}
}
)
result = diff_specs(old, new)
became_required = [
c for c in result.changes if c.kind == "request_property_became_required"
]
assert became_required, "nested array-item required change must be detected"
assert any("items" in c.path and "sku" in c.path for c in became_required)

def test_component_ref_property_target_change_is_dangerous(self):
# A component property whose $ref target was retargeted must be
# flagged as DANGEROUS (it was previously silently ignored at the
# component level).
old = _make_spec(
schemas={
"Order": {
"type": "object",
"properties": {
"customer": {"$ref": "#/components/schemas/CustomerV1"},
},
},
"CustomerV1": {"type": "object"},
"CustomerV2": {"type": "object"},
}
)
new = _make_spec(
schemas={
"Order": {
"type": "object",
"properties": {
"customer": {"$ref": "#/components/schemas/CustomerV2"},
},
},
"CustomerV1": {"type": "object"},
"CustomerV2": {"type": "object"},
}
)
result = diff_specs(old, new)
ref_changed = [c for c in result.changes if c.kind == "schema_ref_changed"]
assert ref_changed, "ref-target change must be detected"
assert any(c.severity.value == "dangerous" for c in ref_changed)
assert any("Order" in c.path and "customer" in c.path for c in ref_changed)

def test_component_ref_property_removed_is_dangerous(self):
old = _make_spec(
schemas={
"Order": {
"type": "object",
"properties": {
"customer": {"$ref": "#/components/schemas/Customer"},
},
},
"Customer": {"type": "object"},
}
)
new = _make_spec(
schemas={
"Order": {
"type": "object",
"properties": {
"customer": {"type": "object"},
},
},
"Customer": {"type": "object"},
}
)
result = diff_specs(old, new)
ref_changed = [c for c in result.changes if c.kind == "schema_ref_changed"]
assert ref_changed, "dropped $ref must be detected"

def test_property_removed_breaking_if_required(self):
old = _make_spec(
schemas={
Expand Down
Loading