diff --git a/src/lib.rs b/src/lib.rs index dbdbe3a..b79bea8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -68,7 +68,9 @@ impl JsonPathStack { } pub fn contains_cycle(&self) -> bool { - self.stack.iter().any(|item| item.starts_with(&self.top)) + self.stack + .iter() + .any(|item| is_path_ancestor_of(&self.top, item)) } pub fn iter(&self) -> impl Iterator { @@ -81,3 +83,77 @@ impl Default for JsonPathStack { Self::new() } } + +/// Check if `ancestor` is a path-segment-aligned prefix of `path`. +/// +/// Returns `true` if either of the following conditions are true: +/// +/// 1. `ancestor` is the same as `path`. +/// 2. `ancestor` is a prefix of `path`, and the character immediately +/// following the prefix is `/`. +/// +/// The second condition prevents false matches where schema names share a +/// common string prefix (e.g., `Item` matching `ItemPage`). +fn is_path_ancestor_of(ancestor: &str, path: &str) -> bool { + path.starts_with(ancestor) + && path + .as_bytes() + .get(ancestor.len()) + .is_none_or(|&b| b == b'/') +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_contains_cycle() { + // A genuine cycle: pushing back to a schema we've already visited. + let stack = JsonPathStack::new() + .push("#/components/schemas/Tree") + .append("properties") + .append("children") + .push("#/components/schemas/Tree"); + + assert!(stack.contains_cycle()); + + // Schemas whose names share a common string prefix must not be treated + // as cycles. e.g. the string "Item" is a prefix of "ItemPage", but they + // are different schemas. + let stack = JsonPathStack::new() + .push("#/components/schemas/ItemPage") + .append("properties") + .append("items") + .append("items") + .push("#/components/schemas/Item"); + + assert!(!stack.contains_cycle()); + } + + #[test] + fn test_is_path_ancestor_of_basics() { + // Exact match. + assert!(is_path_ancestor_of( + "#/components/schemas/Item", + "#/components/schemas/Item" + )); + + // Proper ancestor. + assert!(is_path_ancestor_of( + "#/components/schemas/Item", + "#/components/schemas/Item/properties/value" + )); + + // Shared string prefix but not a path ancestor. + assert!(!is_path_ancestor_of( + "#/components/schemas/Item", + "#/components/schemas/ItemPage" + )); + + // Sibling, not ancestor. + assert!(!is_path_ancestor_of( + "#/components/schemas/Item", + "#/components/schemas/Other" + )); + } +} diff --git a/tests/cases/cycle-shared-prefix/base.json b/tests/cases/cycle-shared-prefix/base.json new file mode 100644 index 0000000..7a409a3 --- /dev/null +++ b/tests/cases/cycle-shared-prefix/base.json @@ -0,0 +1,54 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Cycle shared-prefix detection test fixture", + "version": "1.0.0", + "description": "Item should not be detected as a prefix of ItemPage." + }, + "paths": { + "/items": { + "get": { + "operationId": "list_items", + "responses": { + "200": { + "description": "A page of items", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ItemPage" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "ItemPage": { + "description": "A page of items", + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Item" + } + } + }, + "required": ["items"] + }, + "Item": { + "description": "An item", + "type": "object", + "properties": { + "value": { + "type": "string" + } + }, + "required": ["value"] + } + } + } +} diff --git a/tests/cases/cycle-shared-prefix/output/change-item-value-type.out b/tests/cases/cycle-shared-prefix/output/change-item-value-type.out new file mode 100644 index 0000000..b7f0017 --- /dev/null +++ b/tests/cases/cycle-shared-prefix/output/change-item-value-type.out @@ -0,0 +1,32 @@ +--- change-item-value-type.json ++++ patched +@@ + "description": "An item", + "properties": { + "value": { +- "type": "string" ++ "type": "integer" + } + }, + "required": [ + + +Result for patch: +[ + Change { + message: "schema types changed", + old_path: [ + "#/components/schemas/Item/properties/value", + "#/components/schemas/ItemPage/properties/items/items/$ref", + "#/paths/~1items/get/responses/200/content/application~1json/schema/$ref", + ], + new_path: [ + "#/components/schemas/Item/properties/value", + "#/components/schemas/ItemPage/properties/items/items/$ref", + "#/paths/~1items/get/responses/200/content/application~1json/schema/$ref", + ], + comparison: Output, + class: Incompatible, + details: UnknownDifference, + }, +] diff --git a/tests/cases/cycle-shared-prefix/patch/change-item-value-type.json b/tests/cases/cycle-shared-prefix/patch/change-item-value-type.json new file mode 100644 index 0000000..b4d012b --- /dev/null +++ b/tests/cases/cycle-shared-prefix/patch/change-item-value-type.json @@ -0,0 +1,7 @@ +[ + { + "op": "replace", + "path": "/components/schemas/Item/properties/value/type", + "value": "integer" + } +]