From 3944fc129280cac129ad1de6b80689ffc0728796 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Thu, 25 Jun 2026 14:48:02 +0000 Subject: [PATCH] detect scalar request narrowing --- codex-rs/schema-evolution/src/compare.rs | 201 ++++++++++++++++++ .../schema-evolution/src/compare/value.rs | 126 +++++++++++ .../src/compare/value_tests.rs | 110 ++++++++++ .../schema-evolution/src/compare_tests.rs | 60 ++++++ codex-rs/schema-evolution/src/lib.rs | 23 ++ codex-rs/schema-evolution/src/model.rs | 12 ++ .../schema-evolution/src/model/constraints.rs | 15 ++ codex-rs/schema-evolution/src/model/method.rs | 7 + codex-rs/schema-evolution/src/model/value.rs | 28 +++ codex-rs/schema-evolution/src/parse.rs | 2 + codex-rs/schema-evolution/src/test_support.rs | 45 ++++ codex-rs/schema-evolution/src/violation.rs | 175 +++++++++++++++ 12 files changed, 804 insertions(+) create mode 100644 codex-rs/schema-evolution/src/compare.rs create mode 100644 codex-rs/schema-evolution/src/compare/value.rs create mode 100644 codex-rs/schema-evolution/src/compare/value_tests.rs create mode 100644 codex-rs/schema-evolution/src/compare_tests.rs create mode 100644 codex-rs/schema-evolution/src/test_support.rs diff --git a/codex-rs/schema-evolution/src/compare.rs b/codex-rs/schema-evolution/src/compare.rs new file mode 100644 index 0000000000..1c035f358e --- /dev/null +++ b/codex-rs/schema-evolution/src/compare.rs @@ -0,0 +1,201 @@ +mod value; + +use crate::ApiSchema; +use crate::Arguments; +use crate::Location; +use crate::Method; +use crate::SchemaId; +use crate::SchemaNode; +use crate::SchemaPath; +use crate::SchemaRules; +use crate::SchemaSnapshot; +use crate::Violation; +use anyhow::Result; +use std::collections::BTreeMap; +use std::collections::HashSet; + +pub(crate) fn compare_api_schemas(base: &ApiSchema, current: &ApiSchema) -> Result> { + let mut retained_methods = 0; + let mut violations = Vec::new(); + for (name, base_method) in &base.methods { + let Some(current_method) = current.methods.get(name) else { + violations.push(Violation::MethodRemoved { + method: name.clone(), + }); + continue; + }; + retained_methods += 1; + let mut cx = CompareCx::new(base, current, name); + base_method.compare(current_method, &mut cx, &SchemaPath::default())?; + violations.extend(cx.violations); + } + sort_and_dedup(&mut violations); + Ok(collapse_shared_envelope_changes( + violations, + retained_methods, + )) +} + +/// Recursively compares typed schema nodes and records values accepted by the base only. +/// +/// Implementations should delegate to child schema types and add narrowly scoped violations to +/// the comparison context rather than interpreting unrelated schema keywords. +pub(super) trait CompareNarrowing { + fn compare(&self, current: &Rhs, cx: &mut CompareCx<'_>, path: &SchemaPath) -> Result<()>; +} + +pub(super) struct CompareCx<'a> { + pub(super) base: &'a ApiSchema, + pub(super) current: &'a ApiSchema, + pub(super) method: &'a str, + active: HashSet<(SchemaId, SchemaId)>, + pub(super) violations: Vec, +} + +impl<'a> CompareCx<'a> { + fn new(base: &'a ApiSchema, current: &'a ApiSchema, method: &'a str) -> Self { + Self { + base, + current, + method, + active: HashSet::new(), + violations: Vec::new(), + } + } + + pub(super) fn location(&self, path: &SchemaPath) -> Location { + Location::method(self.method, path.clone()) + } + + pub(super) fn constraint_changed( + &mut self, + path: &SchemaPath, + before: serde_json::Value, + after: serde_json::Value, + ) { + self.violations.push(Violation::ConstraintChanged { + at: self.location(path), + before: SchemaSnapshot(before), + after: SchemaSnapshot(after), + }); + } +} + +impl CompareNarrowing for Method { + fn compare(&self, current: &Self, cx: &mut CompareCx<'_>, path: &SchemaPath) -> Result<()> { + self.request.compare(¤t.request, cx, path)?; + self.arguments + .compare(¤t.arguments, cx, &path.property("params")) + } +} + +impl CompareNarrowing for Arguments { + fn compare(&self, current: &Self, cx: &mut CompareCx<'_>, path: &SchemaPath) -> Result<()> { + match (self.argument(), current.argument()) { + (None, None) => {} + (Some(_), None) => cx.violations.push(Violation::PropertyRemoved { + at: cx.location(path), + }), + (None, Some(current)) if current.required => { + cx.violations.push(Violation::RequiredPropertyAdded { + at: cx.location(path), + }); + } + (None, Some(_)) => {} + (Some(base), Some(current)) => { + if !base.required && current.required { + cx.violations.push(Violation::RequiredPropertyAdded { + at: cx.location(path), + }); + } + base.schema.compare(¤t.schema, cx, path)?; + } + } + Ok(()) + } +} + +impl CompareNarrowing for SchemaId { + fn compare(&self, current: &Self, cx: &mut CompareCx<'_>, path: &SchemaPath) -> Result<()> { + let (base_id, base) = cx.base.resolve(*self)?; + let (current_id, current) = cx.current.resolve(*current)?; + if !cx.active.insert((base_id, current_id)) { + return Ok(()); + } + let result = match (base, current) { + (SchemaNode::Never, _) | (_, SchemaNode::Any) => Ok(()), + (SchemaNode::Any, _) | (_, SchemaNode::Never) => { + cx.constraint_changed( + path, + cx.base.snapshot(base_id)?, + cx.current.snapshot(current_id)?, + ); + Ok(()) + } + (SchemaNode::Rules(base), SchemaNode::Rules(current)) => { + compare_rules(base, current, cx, path) + } + (SchemaNode::Reference(_), _) | (_, SchemaNode::Reference(_)) => { + unreachable!("resolve returns concrete schema nodes") + } + }; + cx.active.remove(&(base_id, current_id)); + result + } +} + +fn compare_rules( + base: &SchemaRules, + current: &SchemaRules, + cx: &mut CompareCx<'_>, + path: &SchemaPath, +) -> Result<()> { + value::compare_values(base.values.as_ref(), current.values.as_ref(), cx, path); + value::compare_types(base.types.as_ref(), current.types.as_ref(), cx, path); + value::compare_constraints(&base.constraints, ¤t.constraints, cx, path); + Ok(()) +} + +fn collapse_shared_envelope_changes( + violations: Vec, + retained_methods: usize, +) -> Vec { + let mut groups = BTreeMap::>::new(); + for violation in violations { + let mut breakage = violation.breakage(); + breakage.method.clear(); + groups + .entry(serde_json::to_string(&breakage).unwrap_or_default()) + .or_default() + .push(violation); + } + + let mut collapsed = Vec::new(); + for mut group in groups.into_values() { + let shared = retained_methods > 0 + && group.len() == retained_methods + && group + .first() + .and_then(Violation::location) + .is_some_and(|location| !location.path.starts_with_params()); + if shared { + let mut violation = group.remove(0); + violation.set_shared_envelope(); + collapsed.push(violation); + } else { + collapsed.extend(group); + } + } + sort_and_dedup(&mut collapsed); + collapsed +} + +fn sort_and_dedup(violations: &mut Vec) { + violations + .sort_by_key(|violation| serde_json::to_string(&violation.breakage()).unwrap_or_default()); + violations.dedup(); +} + +#[cfg(test)] +#[path = "compare_tests.rs"] +mod tests; diff --git a/codex-rs/schema-evolution/src/compare/value.rs b/codex-rs/schema-evolution/src/compare/value.rs new file mode 100644 index 0000000000..c9b55adf87 --- /dev/null +++ b/codex-rs/schema-evolution/src/compare/value.rs @@ -0,0 +1,126 @@ +use super::CompareCx; +use crate::ConstraintSet; +use crate::SchemaPath; +use crate::TypeSet; +use crate::ValueSet; +use crate::Violation; +use serde_json::Number; +use std::cmp::Ordering; + +pub(super) fn compare_values( + base: Option<&ValueSet>, + current: Option<&ValueSet>, + cx: &mut CompareCx<'_>, + path: &SchemaPath, +) { + let Some(current) = current else { + return; + }; + if base.is_none_or(|base| { + base.values + .iter() + .any(|value| !current.values.contains(value)) + }) { + cx.violations.push(Violation::EnumNarrowed { + at: cx.location(path), + before: base.cloned(), + after: current.clone(), + }); + } +} + +pub(super) fn compare_types( + base: Option<&TypeSet>, + current: Option<&TypeSet>, + cx: &mut CompareCx<'_>, + path: &SchemaPath, +) { + let Some(current) = current else { + return; + }; + if base.is_none_or(|base| !base.accepted_types().is_subset(¤t.accepted_types())) { + cx.violations.push(Violation::TypeNarrowed { + at: cx.location(path), + before: base.cloned(), + after: current.clone(), + }); + } +} + +pub(super) fn compare_constraints( + base: &ConstraintSet, + current: &ConstraintSet, + cx: &mut CompareCx<'_>, + path: &SchemaPath, +) { + let mut before = ConstraintSet::default(); + let mut after = ConstraintSet::default(); + for (kind, current_value) in ¤t.lower_bounds { + let base_value = base.lower_bounds.get(kind); + if base_value.is_none_or(|base_value| { + compare_numbers(current_value, base_value) == Ordering::Greater + }) { + if let Some(base_value) = base_value { + before.lower_bounds.insert(*kind, base_value.clone()); + } + after.lower_bounds.insert(*kind, current_value.clone()); + } + } + for (kind, current_value) in ¤t.upper_bounds { + let base_value = base.upper_bounds.get(kind); + if base_value + .is_none_or(|base_value| compare_numbers(current_value, base_value) == Ordering::Less) + { + if let Some(base_value) = base_value { + before.upper_bounds.insert(*kind, base_value.clone()); + } + after.upper_bounds.insert(*kind, current_value.clone()); + } + } + if current.unique_items == Some(true) && base.unique_items != Some(true) { + before.unique_items = base.unique_items; + after.unique_items = current.unique_items; + } + for (key, current_value) in ¤t.opaque { + let base_value = base.opaque.get(key); + if base_value != Some(current_value) { + if let Some(base_value) = base_value { + before.opaque.insert(key.clone(), base_value.clone()); + } + after.opaque.insert(key.clone(), current_value.clone()); + } + } + if after != ConstraintSet::default() { + cx.constraint_changed(path, before.to_json(), after.to_json()); + } +} + +fn compare_numbers(left: &Number, right: &Number) -> Ordering { + if let (Some(left), Some(right)) = (left.as_i64(), right.as_i64()) { + return left.cmp(&right); + } + if let (Some(left), Some(right)) = (left.as_u64(), right.as_u64()) { + return left.cmp(&right); + } + if let (Some(left), Some(right)) = (left.as_i64(), right.as_u64()) { + return if left.is_negative() { + Ordering::Less + } else { + (left as u64).cmp(&right) + }; + } + if let (Some(left), Some(right)) = (left.as_u64(), right.as_i64()) { + return if right.is_negative() { + Ordering::Greater + } else { + left.cmp(&(right as u64)) + }; + } + left.as_f64() + .partial_cmp(&right.as_f64()) + .unwrap_or(Ordering::Equal) +} + +#[cfg(test)] +#[path = "value_tests.rs"] +mod tests; diff --git a/codex-rs/schema-evolution/src/compare/value_tests.rs b/codex-rs/schema-evolution/src/compare/value_tests.rs new file mode 100644 index 0000000000..08f651892b --- /dev/null +++ b/codex-rs/schema-evolution/src/compare/value_tests.rs @@ -0,0 +1,110 @@ +use crate::ViolationKind; +use crate::test_support::breakage; +use crate::test_support::compare; +use crate::test_support::request_schema; +use pretty_assertions::assert_eq; +use serde_json::json; + +#[test] +fn detects_enum_type_and_bound_narrowing() -> anyhow::Result<()> { + assert_eq!( + compare( + &request_schema(json!({ "enum": ["a", "b"], "type": "string" })), + &request_schema(json!({ "enum": ["a"], "type": "string" })), + )?, + vec![breakage( + ViolationKind::EnumNarrowed, + "params", + json!(["a", "b"]), + json!(["a"]), + )] + ); + assert_eq!( + compare( + &request_schema(json!({ "type": "number" })), + &request_schema(json!({ "type": "integer" })), + )?, + vec![breakage( + ViolationKind::TypeNarrowed, + "params", + json!("number"), + json!("integer"), + )] + ); + assert_eq!( + compare( + &request_schema(json!({ "type": ["string", "null"] })), + &request_schema(json!({ "type": "string" })), + )?, + vec![breakage( + ViolationKind::TypeNarrowed, + "params", + json!(["null", "string"]), + json!("string"), + )] + ); + assert_eq!( + compare( + &request_schema(json!({ "minLength": 1, "type": "string" })), + &request_schema(json!({ "minLength": 2, "type": "string" })), + )?, + vec![breakage( + ViolationKind::ConstraintChanged, + "params", + json!({ "minLength": 1 }), + json!({ "minLength": 2 }), + )] + ); + Ok(()) +} + +#[test] +fn allows_enum_type_and_bound_widening() -> anyhow::Result<()> { + let base = request_schema(json!({ "enum": ["a"], "minLength": 2, "type": "string" })); + let current = + request_schema(json!({ "enum": ["a", "b"], "minLength": 1, "type": ["string", "null"] })); + assert_eq!(compare(&base, ¤t)?, vec![]); + Ok(()) +} + +#[test] +fn compares_large_integer_bounds_without_losing_precision() -> anyhow::Result<()> { + let base = request_schema(json!({ + "minimum": 9_007_199_254_740_992_u64, + "type": "integer" + })); + let current = request_schema(json!({ + "minimum": 9_007_199_254_740_993_u64, + "type": "integer" + })); + assert_eq!( + compare(&base, ¤t)?, + vec![breakage( + ViolationKind::ConstraintChanged, + "params", + json!({ "minimum": 9_007_199_254_740_992_u64 }), + json!({ "minimum": 9_007_199_254_740_993_u64 }), + )] + ); + Ok(()) +} + +#[test] +fn intersects_enum_and_const_constraints() -> anyhow::Result<()> { + let base = request_schema(json!({ "enum": ["a", "b"], "type": "string" })); + let current = request_schema(json!({ + "const": "a", + "enum": ["a", "b"], + "type": "string" + })); + assert_eq!( + compare(&base, ¤t)?, + vec![breakage( + ViolationKind::EnumNarrowed, + "params", + json!(["a", "b"]), + json!(["a"]), + )] + ); + Ok(()) +} diff --git a/codex-rs/schema-evolution/src/compare_tests.rs b/codex-rs/schema-evolution/src/compare_tests.rs new file mode 100644 index 0000000000..1698fc1d7d --- /dev/null +++ b/codex-rs/schema-evolution/src/compare_tests.rs @@ -0,0 +1,60 @@ +use super::*; +use crate::ViolationKind; +use crate::test_support::breakage; +use crate::test_support::compare; +use crate::test_support::request_schema; +use pretty_assertions::assert_eq; +use serde_json::json; + +#[test] +fn detects_method_removal() -> Result<()> { + assert_eq!( + compare( + &request_schema(json!({ "type": "null" })), + &json!({ "oneOf": [] }), + )?, + vec![crate::SchemaBreakage { + kind: ViolationKind::MethodRemoved, + method: "test/method".to_string(), + path: "request".to_string(), + before: json!(true), + after: json!(false), + }] + ); + Ok(()) +} + +#[test] +fn compares_typed_arguments_with_the_full_request_schema() -> Result<()> { + let mut base = request_schema(json!({ "type": "null" })); + base["oneOf"][0]["required"] = json!(["id", "method"]); + let current = request_schema(json!({ "type": "null" })); + assert_eq!( + compare(&base, ¤t)?, + vec![breakage( + ViolationKind::RequiredPropertyAdded, + "params", + json!(false), + json!(true), + )] + ); + + Ok(()) +} + +#[test] +fn retains_request_level_constraints_in_the_typed_envelope() -> Result<()> { + let base = request_schema(json!({ "type": "null" })); + let mut current = request_schema(json!({ "type": "null" })); + current["oneOf"][0]["minProperties"] = json!(3); + + let mut expected = breakage( + ViolationKind::ConstraintChanged, + "request", + json!({}), + json!({ "minProperties": 3 }), + ); + expected.method = "*".to_string(); + assert_eq!(compare(&base, ¤t)?, vec![expected]); + Ok(()) +} diff --git a/codex-rs/schema-evolution/src/lib.rs b/codex-rs/schema-evolution/src/lib.rs index c2e7c1a15c..fd4c606ecb 100644 --- a/codex-rs/schema-evolution/src/lib.rs +++ b/codex-rs/schema-evolution/src/lib.rs @@ -1,12 +1,18 @@ +mod compare; mod model; mod parse; mod violation; +#[cfg(test)] +mod test_support; + +use compare::compare_api_schemas; pub use model::ApiSchema; pub(crate) use model::Arguments; pub(crate) use model::ArraySchema; pub(crate) use model::ConstraintSet; pub(crate) use model::JsonType; +pub(crate) use model::Method; pub(crate) use model::ObjectSchema; pub(crate) use model::SchemaId; pub(crate) use model::SchemaNode; @@ -15,5 +21,22 @@ pub(crate) use model::TypeSet; pub(crate) use model::UnionKind; pub(crate) use model::UnionSchema; pub(crate) use model::ValueSet; +pub(crate) use violation::Location; pub use violation::SchemaBreakage; +pub(crate) use violation::SchemaPath; +pub(crate) use violation::SchemaSnapshot; +pub(crate) use violation::Violation; pub use violation::ViolationKind; + +use anyhow::Result; + +/// Finds values that the current request schema no longer accepts. +pub fn find_request_narrowing( + base: &ApiSchema, + current: &ApiSchema, +) -> Result> { + Ok(compare_api_schemas(base, current)? + .iter() + .map(Violation::breakage) + .collect()) +} diff --git a/codex-rs/schema-evolution/src/model.rs b/codex-rs/schema-evolution/src/model.rs index db45ffc3c8..a1f2e9b0fb 100644 --- a/codex-rs/schema-evolution/src/model.rs +++ b/codex-rs/schema-evolution/src/model.rs @@ -8,6 +8,7 @@ mod value; pub(crate) use array::ArraySchema; pub(crate) use constraints::ConstraintSet; pub(crate) use constraints::annotation; +pub(crate) use constraints::normalized; #[cfg(test)] pub(crate) use method::Argument; pub(crate) use method::Arguments; @@ -53,6 +54,7 @@ pub struct SchemaRules { pub any_of: Option, pub one_of: Option, pub constraints: ConstraintSet, + pub(crate) source: Value, } impl ApiSchema { @@ -118,6 +120,16 @@ impl ApiSchema { } } } + + pub(crate) fn snapshot(&self, id: SchemaId) -> Result { + let (_, node) = self.resolve(id)?; + Ok(match node { + SchemaNode::Any => Value::Bool(true), + SchemaNode::Never => Value::Bool(false), + SchemaNode::Reference(_) => unreachable!("resolve returns a concrete node"), + SchemaNode::Rules(rules) => rules.source.clone(), + }) + } } fn pointer_escape(value: &str) -> String { diff --git a/codex-rs/schema-evolution/src/model/constraints.rs b/codex-rs/schema-evolution/src/model/constraints.rs index 3a27e6d21a..d649abbd4c 100644 --- a/codex-rs/schema-evolution/src/model/constraints.rs +++ b/codex-rs/schema-evolution/src/model/constraints.rs @@ -97,6 +97,21 @@ impl ConstraintSet { Ok(constraints) } + pub(crate) fn to_json(&self) -> Value { + let mut values = serde_json::Map::new(); + for (kind, value) in &self.lower_bounds { + values.insert(kind.as_str().to_string(), Value::Number(value.clone())); + } + for (kind, value) in &self.upper_bounds { + values.insert(kind.as_str().to_string(), Value::Number(value.clone())); + } + if let Some(value) = self.unique_items { + values.insert("uniqueItems".to_string(), Value::Bool(value)); + } + values.extend(self.opaque.clone()); + Value::Object(values) + } + fn insert_lower(&mut self, kind: LowerBound, value: &Value) -> Result<()> { self.lower_bounds.insert( kind, diff --git a/codex-rs/schema-evolution/src/model/method.rs b/codex-rs/schema-evolution/src/model/method.rs index ffecc9f663..481373000e 100644 --- a/codex-rs/schema-evolution/src/model/method.rs +++ b/codex-rs/schema-evolution/src/model/method.rs @@ -91,6 +91,13 @@ impl Method { } impl Arguments { + pub(crate) fn argument(&self) -> Option { + match self { + Self::None => None, + Self::Map(argument) | Self::List(argument) | Self::Value(argument) => Some(*argument), + } + } + fn classify(parser: &Parser<'_>, argument: Argument) -> Result { let (_, node) = parser.schema.resolve(argument.schema)?; let SchemaNode::Rules(rules) = node else { diff --git a/codex-rs/schema-evolution/src/model/value.rs b/codex-rs/schema-evolution/src/model/value.rs index 2e371b5238..51228674e8 100644 --- a/codex-rs/schema-evolution/src/model/value.rs +++ b/codex-rs/schema-evolution/src/model/value.rs @@ -29,6 +29,18 @@ impl JsonType { _ => Err(anyhow!("unsupported JSON Schema type {value}")), } } + + fn as_str(self) -> &'static str { + match self { + Self::Array => "array", + Self::Boolean => "boolean", + Self::Integer => "integer", + Self::Null => "null", + Self::Number => "number", + Self::Object => "object", + Self::String => "string", + } + } } #[derive(Clone, Debug, Eq, PartialEq)] @@ -64,6 +76,18 @@ impl TypeSet { } accepted } + + pub fn to_json(&self) -> Value { + let values = self + .declared + .iter() + .map(|value| Value::String(value.as_str().to_string())) + .collect::>(); + match values.as_slice() { + [value] => value.clone(), + _ => Value::Array(values), + } + } } #[derive(Clone, Debug, Eq, PartialEq)] @@ -96,4 +120,8 @@ impl ValueSet { values.dedup(); Ok(Some(Self { values })) } + + pub fn to_json(&self) -> Value { + Value::Array(self.values.clone()) + } } diff --git a/codex-rs/schema-evolution/src/parse.rs b/codex-rs/schema-evolution/src/parse.rs index 9785209258..0ddf1a9305 100644 --- a/codex-rs/schema-evolution/src/parse.rs +++ b/codex-rs/schema-evolution/src/parse.rs @@ -10,6 +10,7 @@ use crate::UnionKind; use crate::UnionSchema; use crate::ValueSet; use crate::model::annotation; +use crate::model::normalized; use anyhow::Result; use anyhow::anyhow; use anyhow::bail; @@ -67,6 +68,7 @@ impl<'a> Parser<'a> { any_of, one_of, constraints, + source: normalized(value), }))) } diff --git a/codex-rs/schema-evolution/src/test_support.rs b/codex-rs/schema-evolution/src/test_support.rs new file mode 100644 index 0000000000..1fff6af22b --- /dev/null +++ b/codex-rs/schema-evolution/src/test_support.rs @@ -0,0 +1,45 @@ +use crate::ApiSchema; +use crate::SchemaBreakage; +use crate::find_request_narrowing; +use anyhow::Result; +use serde_json::Value; +use serde_json::json; + +pub(crate) fn request_schema(params: Value) -> Value { + request_schema_for("test/method", params) +} + +pub(crate) fn request_schema_for(method: &str, params: Value) -> Value { + json!({ + "oneOf": [{ + "properties": { + "id": { "type": ["string", "integer"] }, + "method": { "enum": [method], "type": "string" }, + "params": params + }, + "required": ["id", "method", "params"], + "type": "object" + }] + }) +} + +pub(crate) fn compare(base: &Value, current: &Value) -> Result> { + let base = ApiSchema::parse(base)?; + let current = ApiSchema::parse(current)?; + find_request_narrowing(&base, ¤t) +} + +pub(crate) fn breakage( + kind: crate::ViolationKind, + path: &str, + before: Value, + after: Value, +) -> SchemaBreakage { + SchemaBreakage { + kind, + method: "test/method".to_string(), + path: path.to_string(), + before, + after, + } +} diff --git a/codex-rs/schema-evolution/src/violation.rs b/codex-rs/schema-evolution/src/violation.rs index 60947a2718..c7a2f1f95f 100644 --- a/codex-rs/schema-evolution/src/violation.rs +++ b/codex-rs/schema-evolution/src/violation.rs @@ -1,6 +1,9 @@ +use crate::TypeSet; +use crate::ValueSet; use serde::Deserialize; use serde::Serialize; use serde_json::Value; +use std::fmt; #[derive(Clone, Debug, Deserialize, Eq, Ord, PartialEq, PartialOrd, Serialize)] #[serde(rename_all = "camelCase")] @@ -15,6 +18,57 @@ pub enum ViolationKind { UnionVariantRemoved, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum Violation { + MethodRemoved { + method: String, + }, + PropertyRemoved { + at: Location, + }, + RequiredPropertyAdded { + at: Location, + }, + TypeNarrowed { + at: Location, + before: Option, + after: TypeSet, + }, + EnumNarrowed { + at: Location, + before: Option, + after: ValueSet, + }, + ConstraintChanged { + at: Location, + before: SchemaSnapshot, + after: SchemaSnapshot, + }, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Location { + pub scope: ViolationScope, + pub path: SchemaPath, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum ViolationScope { + Method(String), + SharedEnvelope, +} + +#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] +pub struct SchemaPath(Vec); + +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +enum PathSegment { + Property(String), +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct SchemaSnapshot(pub Value); + #[derive(Clone, Debug, Eq, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct SchemaBreakage { @@ -24,3 +78,124 @@ pub struct SchemaBreakage { pub before: Value, pub after: Value, } + +impl Location { + pub(crate) fn method(method: &str, path: SchemaPath) -> Self { + Self { + scope: ViolationScope::Method(method.to_string()), + path, + } + } +} + +impl SchemaPath { + pub(crate) fn property(&self, name: impl Into) -> Self { + let mut path = self.clone(); + path.0.push(PathSegment::Property(name.into())); + path + } + + pub(crate) fn starts_with_params(&self) -> bool { + matches!(self.0.first(), Some(PathSegment::Property(name)) if name == "params") + } +} + +impl fmt::Display for SchemaPath { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + for (index, segment) in self.0.iter().enumerate() { + match segment { + PathSegment::Property(name) if index == 0 => formatter.write_str(name)?, + PathSegment::Property(name) => write!(formatter, ".{name}")?, + } + } + Ok(()) + } +} + +impl Violation { + pub fn breakage(&self) -> SchemaBreakage { + match self { + Self::MethodRemoved { method } => SchemaBreakage { + kind: ViolationKind::MethodRemoved, + method: method.clone(), + path: "request".to_string(), + before: Value::Bool(true), + after: Value::Bool(false), + }, + Self::PropertyRemoved { at } => at_location( + ViolationKind::PropertyRemoved, + at, + Value::Bool(true), + Value::Bool(false), + ), + Self::RequiredPropertyAdded { at } => at_location( + ViolationKind::RequiredPropertyAdded, + at, + Value::Bool(false), + Value::Bool(true), + ), + Self::TypeNarrowed { at, before, after } => at_location( + ViolationKind::TypeNarrowed, + at, + before.as_ref().map_or(Value::Null, TypeSet::to_json), + after.to_json(), + ), + Self::EnumNarrowed { at, before, after } => at_location( + ViolationKind::EnumNarrowed, + at, + before.as_ref().map_or(Value::Null, ValueSet::to_json), + after.to_json(), + ), + Self::ConstraintChanged { at, before, after } => at_location( + ViolationKind::ConstraintChanged, + at, + before.0.clone(), + after.0.clone(), + ), + } + } + + pub(crate) fn location(&self) -> Option<&Location> { + match self { + Self::MethodRemoved { .. } => None, + Self::PropertyRemoved { at } + | Self::RequiredPropertyAdded { at } + | Self::TypeNarrowed { at, .. } + | Self::EnumNarrowed { at, .. } + | Self::ConstraintChanged { at, .. } => Some(at), + } + } + + pub(crate) fn set_shared_envelope(&mut self) { + if let Some(location) = self.location_mut() { + location.scope = ViolationScope::SharedEnvelope; + } + } + + fn location_mut(&mut self) -> Option<&mut Location> { + match self { + Self::MethodRemoved { .. } => None, + Self::PropertyRemoved { at } + | Self::RequiredPropertyAdded { at } + | Self::TypeNarrowed { at, .. } + | Self::EnumNarrowed { at, .. } + | Self::ConstraintChanged { at, .. } => Some(at), + } + } +} + +fn at_location(kind: ViolationKind, at: &Location, before: Value, after: Value) -> SchemaBreakage { + SchemaBreakage { + kind, + method: match &at.scope { + ViolationScope::Method(method) => method.clone(), + ViolationScope::SharedEnvelope => "*".to_string(), + }, + path: match at.path.to_string() { + path if path.is_empty() => "request".to_string(), + path => path, + }, + before, + after, + } +}