diff --git a/crates/dropshot-api-manager/src/cmd/generate.rs b/crates/dropshot-api-manager/src/cmd/generate.rs index 1c0b2e4..382f96b 100644 --- a/crates/dropshot-api-manager/src/cmd/generate.rs +++ b/crates/dropshot-api-manager/src/cmd/generate.rs @@ -3,11 +3,13 @@ use crate::{ FAILURE_EXIT_CODE, apis::ManagedApis, + compatibility::CompatIssueLocation, environment::{BlessedSource, GeneratedSource, ResolvedEnv}, output::{ - CheckResult, OutputOpts, Styles, display_api_spec_version, - display_load_problems, display_non_version_problems, - display_resolution, display_version_problems, + CheckResult, CompatDisplayContext, OutputOpts, Styles, + display_api_spec_version, display_load_problems, + display_non_version_problems, display_resolution, + display_version_problems, headers::{self, *}, plural, }, @@ -216,6 +218,8 @@ fn generate_impl_inner( display_load_problems(writer, &errors, styles)?; let resolved = Resolved::new(env, apis, &blessed, &generated, &local_files_recheck); + let dedup = resolved.build_compat_dedup_map(); + let orphaned_and_unparseable: Vec<_> = resolved.orphaned_and_unparseable().collect(); nproblems += orphaned_and_unparseable.len(); @@ -237,7 +241,13 @@ fn generate_impl_inner( (this is a bug)", ident, version )?; - display_version_problems(writer, env, problems, styles)?; + let compat_ctx = CompatDisplayContext { + dedup: &dedup, + current: CompatIssueLocation { api: ident, version }, + }; + display_version_problems( + writer, env, problems, styles, compat_ctx, + )?; } } @@ -259,6 +269,7 @@ fn generate_impl_inner( // Release borrows held by `resolved`, then drop all source // collections in parallel. Each contains many parsed OpenAPI // documents whose sequential drops are costly. + drop(dedup); drop(resolved); std::thread::scope(|s| { s.spawn(|| drop(blessed)); diff --git a/crates/dropshot-api-manager/src/compatibility/detect.rs b/crates/dropshot-api-manager/src/compatibility/detect.rs index b48f00b..d402ac3 100644 --- a/crates/dropshot-api-manager/src/compatibility/detect.rs +++ b/crates/dropshot-api-manager/src/compatibility/detect.rs @@ -6,8 +6,9 @@ //! [`super::types`]; this module just bridges drift's output into them. use super::types::{ - ApiCompatIssue, DocumentBasePath, DocumentPath, OperationIdMap, PathTree, - PathTreeKey, SubpathChange, unescape_pointer_component, + ApiCompatIssue, CompatIssueLocation, CompatRenderStatus, DocumentBasePath, + DocumentPath, OperationIdMap, PathTree, PathTreeKey, SubpathChange, + unescape_pointer_component, }; use drift::{Change, ChangeClass, ChangeInfo, ChangePath}; @@ -66,6 +67,128 @@ impl ApiCompatIssue { } } +/// Tracks compatibility issues to deduplicate them. +/// +/// Call [`Self::insert`] once per `(location, issue)` pair, then +/// [`Self::finalize`] to enable lookups. The two-phase design lets anchor +/// numbers be compact: only multi-site entries get a number, and the +/// numbering is contiguous. +#[derive(Debug, Default)] +pub(crate) struct CompatDedupMap<'a> { + // This is a `Vec` rather than a `HashMap` because `serde_json::Value` + // doesn't implement `Hash` (floats), and the total entry count per run is + // expected to be small. + entries: Vec>, +} + +#[derive(Debug)] +struct RawEntry<'a> { + issue: &'a ApiCompatIssue, + first_occurrence: CompatIssueLocation<'a>, + count: usize, +} + +impl<'a> CompatDedupMap<'a> { + pub(crate) fn insert( + &mut self, + location: CompatIssueLocation<'a>, + issue: &'a ApiCompatIssue, + ) { + if let Some(entry) = + self.entries.iter_mut().find(|e| e.issue.is_same_change_as(issue)) + { + entry.count += 1; + } else { + self.entries.push(RawEntry { + issue, + first_occurrence: location, + count: 1, + }); + } + } + + /// Finalize the map and assign 1-indexed anchor numbers to duplicated + /// entries. + pub(crate) fn finalize(self) -> FinalizedCompatDedupMap<'a> { + let mut next_anchor = 1; + let entries = self + .entries + .into_iter() + .map(|raw| { + if raw.count > 1 { + let anchor = next_anchor; + next_anchor += 1; + FinalizedEntry::MultiSite { + issue: raw.issue, + first_occurrence: raw.first_occurrence, + anchor, + } + } else { + FinalizedEntry::Singleton { issue: raw.issue } + } + }) + .collect(); + FinalizedCompatDedupMap { entries } + } +} + +/// Lookup-phase dedup map. Returned by [`CompatDedupMap::finalize`]. +#[derive(Debug)] +pub(crate) struct FinalizedCompatDedupMap<'a> { + entries: Vec>, +} + +#[derive(Debug)] +enum FinalizedEntry<'a> { + Singleton { + issue: &'a ApiCompatIssue, + }, + MultiSite { + issue: &'a ApiCompatIssue, + first_occurrence: CompatIssueLocation<'a>, + anchor: usize, + }, +} + +impl<'a> FinalizedEntry<'a> { + fn issue(&self) -> &'a ApiCompatIssue { + match self { + Self::Singleton { issue } | Self::MultiSite { issue, .. } => issue, + } + } +} + +impl FinalizedCompatDedupMap<'_> { + /// Returns how `issue` at `current` should be rendered. + /// + /// Panics if `issue` was never inserted. + pub(crate) fn status_for( + &self, + issue: &ApiCompatIssue, + current: CompatIssueLocation<'_>, + ) -> CompatRenderStatus { + let entry = self + .entries + .iter() + .find(|e| e.issue().is_same_change_as(issue)) + .expect("every issue passed to status_for was inserted"); + match entry { + FinalizedEntry::Singleton { .. } => { + CompatRenderStatus::FirstOccurrence { anchor: None } + } + FinalizedEntry::MultiSite { first_occurrence, anchor, .. } => { + if *first_occurrence == current { + CompatRenderStatus::FirstOccurrence { + anchor: Some(*anchor), + } + } else { + CompatRenderStatus::Duplicate { anchor: *anchor } + } + } + } + } +} + impl SubpathChange { fn from_info(info: ChangeInfo) -> Self { Self { @@ -265,7 +388,7 @@ fn normalize_old_websocket_responses( } } -pub fn api_compatible( +pub(crate) fn api_compatible( blessed: &serde_json::Value, generated: &serde_json::Value, ) -> anyhow::Result> { @@ -311,12 +434,21 @@ pub fn api_compatible( &generated_op_ids, )); } + // Sort by base to ensure a deterministic iteration order, independent of + // whatever drift returns. The JSON values are derived from the base, and + // `serde_json::Value` isn't `Ord`, so we don't include them in the key. + issues.sort_by(|a, b| { + (&a.blessed_base, &a.generated_base) + .cmp(&(&b.blessed_base, &b.generated_base)) + }); Ok(issues) } #[cfg(test)] mod tests { use super::*; + use dropshot_api_manager_types::ApiIdent; + use std::collections::BTreeSet; #[test] fn test_normalize_old_websocket_responses() { @@ -697,4 +829,383 @@ mod tests { "websocket-to-HTTP change should be detected as incompatible", ); } + + /// Shorthand for a component-shaped [`DocumentBasePath`] from a JSON + /// Pointer string. + fn component_base(p: &str) -> DocumentBasePath { + DocumentBasePath::Component(DocumentPath::parse(p)) + } + + /// Build an `ApiCompatIssue` directly. + fn synthetic_issue( + base: &str, + message: &str, + blessed_value: serde_json::Value, + generated_value: serde_json::Value, + ) -> ApiCompatIssue { + ApiCompatIssue { + blessed_base: component_base(base), + generated_base: component_base(base), + changes: BTreeSet::from([SubpathChange { + class: ChangeClass::Incompatible, + message: message.into(), + old_subpath: DocumentPath::parse("properties/value"), + new_subpath: DocumentPath::parse("properties/value"), + }]), + // Leave this empty since it's ignored by the dedup logic. + tree: PathTree::default(), + blessed_value: Some(blessed_value), + generated_value: Some(generated_value), + } + } + + /// Owns the data a [`CompatIssueLocation`] borrows from, so a test can + /// hold it alive while passing the location around. + struct OwnedLoc { + api: ApiIdent, + version: semver::Version, + } + + impl OwnedLoc { + fn new(api: &str, version: &str) -> Self { + Self { + api: ApiIdent::from(api.to_string()), + version: version.parse().unwrap(), + } + } + + fn as_loc(&self) -> CompatIssueLocation<'_> { + CompatIssueLocation { api: &self.api, version: &self.version } + } + } + + #[track_caller] + fn assert_status( + dedup: &FinalizedCompatDedupMap<'_>, + issue: &ApiCompatIssue, + current: CompatIssueLocation<'_>, + expected: CompatRenderStatus, + ) { + let actual = dedup.status_for(issue, current); + assert_eq!(actual, expected); + } + + #[test] + fn test_dedup_basic() { + let issue_a = synthetic_issue( + "#/components/schemas/Error", + "schema types changed", + serde_json::json!({"Error": {"type": "string"}}), + serde_json::json!({"Error": {"type": "integer"}}), + ); + let issue_b = synthetic_issue( + "#/components/schemas/Error", + "schema types changed", + serde_json::json!({"Error": {"type": "string"}}), + serde_json::json!({"Error": {"type": "integer"}}), + ); + + let foo = OwnedLoc::new("foo", "1.0.0"); + let bar = OwnedLoc::new("bar", "1.0.0"); + let mut dedup = CompatDedupMap::default(); + dedup.insert(foo.as_loc(), &issue_a); + dedup.insert(bar.as_loc(), &issue_b); + let dedup = dedup.finalize(); + + assert_status( + &dedup, + &issue_a, + foo.as_loc(), + CompatRenderStatus::FirstOccurrence { anchor: Some(1) }, + ); + assert_status( + &dedup, + &issue_b, + bar.as_loc(), + CompatRenderStatus::Duplicate { anchor: 1 }, + ); + } + + /// Two issues with the same name and message but different underlying + /// values are not duplicates: an `Error` schema in one API may be a + /// completely different type from `Error` in another. + #[test] + fn test_dedup_distinguishes_by_value() { + let issue_a = synthetic_issue( + "#/components/schemas/Error", + "schema types changed", + serde_json::json!({"Error": {"type": "string"}}), + serde_json::json!({"Error": {"type": "integer"}}), + ); + // Same name, same change message, different concrete schema. + let issue_b = synthetic_issue( + "#/components/schemas/Error", + "schema types changed", + serde_json::json!({"Error": {"type": "object"}}), + serde_json::json!({"Error": {"type": "array"}}), + ); + + let foo = OwnedLoc::new("foo", "1.0.0"); + let bar = OwnedLoc::new("bar", "1.0.0"); + let mut dedup = CompatDedupMap::default(); + dedup.insert(foo.as_loc(), &issue_a); + dedup.insert(bar.as_loc(), &issue_b); + let dedup = dedup.finalize(); + + // Both issues are reported by exactly one (api, version), so finalize + // didn't assign them anchors. + assert_status( + &dedup, + &issue_a, + foo.as_loc(), + CompatRenderStatus::FirstOccurrence { anchor: None }, + ); + assert_status( + &dedup, + &issue_b, + bar.as_loc(), + CompatRenderStatus::FirstOccurrence { anchor: None }, + ); + } + + /// The same issue reported under multiple versions of the same API + /// dedups the second version, not the first. + #[test] + fn test_dedup_across_versions_of_same_api() { + let issue = synthetic_issue( + "#/components/schemas/Error", + "schema types changed", + serde_json::json!({"Error": {"type": "string"}}), + serde_json::json!({"Error": {"type": "integer"}}), + ); + + let v1 = OwnedLoc::new("foo", "1.0.0"); + let v2 = OwnedLoc::new("foo", "2.0.0"); + let mut dedup = CompatDedupMap::default(); + dedup.insert(v1.as_loc(), &issue); + dedup.insert(v2.as_loc(), &issue); + let dedup = dedup.finalize(); + + assert_status( + &dedup, + &issue, + v1.as_loc(), + CompatRenderStatus::FirstOccurrence { anchor: Some(1) }, + ); + assert_status( + &dedup, + &issue, + v2.as_loc(), + CompatRenderStatus::Duplicate { anchor: 1 }, + ); + } + + #[test] + fn test_dedup_asymmetric_versions_across_apis() { + let issue = synthetic_issue( + "#/components/schemas/Error", + "schema types changed", + serde_json::json!({"Error": {"type": "string"}}), + serde_json::json!({"Error": {"type": "integer"}}), + ); + + let a_v1 = OwnedLoc::new("api_a", "1.0.0"); + let a_v2 = OwnedLoc::new("api_a", "2.0.0"); + let a_v3 = OwnedLoc::new("api_a", "3.0.0"); + let b_v2 = OwnedLoc::new("api_b", "2.0.0"); + + let mut dedup = CompatDedupMap::default(); + dedup.insert(a_v1.as_loc(), &issue); + dedup.insert(a_v2.as_loc(), &issue); + dedup.insert(a_v3.as_loc(), &issue); + dedup.insert(b_v2.as_loc(), &issue); + let dedup = dedup.finalize(); + + // a@v1 is the canonical occurrence; everyone else is a duplicate + // pointing at anchor 1. + assert_status( + &dedup, + &issue, + a_v1.as_loc(), + CompatRenderStatus::FirstOccurrence { anchor: Some(1) }, + ); + for loc in [&a_v2, &a_v3, &b_v2] { + assert_status( + &dedup, + &issue, + loc.as_loc(), + CompatRenderStatus::Duplicate { anchor: 1 }, + ); + } + } + + #[test] + fn test_dedup_ignores_tree() { + let ops = OperationIdMap::new(); + let mut tree_a = PathTree::default(); + tree_a.insert([PathTreeKey::parse( + "#/components/schemas/Wrapper/properties/a/$ref", + &ops, + )]); + let mut tree_b = PathTree::default(); + tree_b.insert([PathTreeKey::parse( + "#/components/schemas/Wrapper/properties/b/$ref", + &ops, + )]); + + let make_issue = |tree: PathTree| ApiCompatIssue { + blessed_base: component_base("#/components/schemas/SubType"), + generated_base: component_base("#/components/schemas/SubType"), + changes: BTreeSet::from([SubpathChange { + class: ChangeClass::Incompatible, + message: "schema types changed".into(), + old_subpath: DocumentPath::parse("properties/value"), + new_subpath: DocumentPath::parse("properties/value"), + }]), + tree, + blessed_value: Some( + serde_json::json!({"SubType": {"type": "string"}}), + ), + generated_value: Some( + serde_json::json!({"SubType": {"type": "integer"}}), + ), + }; + + let issue_a = make_issue(tree_a); + let issue_b = make_issue(tree_b); + + assert!( + issue_a.is_same_change_as(&issue_b), + "issues identical except for tree should dedup", + ); + } + + #[test] + fn test_anchor_numbering_skips_single_occurrence() { + let multi_a = synthetic_issue( + "#/components/schemas/MultiA", + "schema types changed", + serde_json::json!({"MultiA": {"type": "string"}}), + serde_json::json!({"MultiA": {"type": "integer"}}), + ); + let solo = synthetic_issue( + "#/components/schemas/Solo", + "schema types changed", + serde_json::json!({"Solo": {"type": "string"}}), + serde_json::json!({"Solo": {"type": "integer"}}), + ); + let multi_b = synthetic_issue( + "#/components/schemas/MultiB", + "schema types changed", + serde_json::json!({"MultiB": {"type": "string"}}), + serde_json::json!({"MultiB": {"type": "integer"}}), + ); + + let foo = OwnedLoc::new("foo", "1.0.0"); + let bar = OwnedLoc::new("bar", "1.0.0"); + let mut dedup = CompatDedupMap::default(); + // Insert order: multi_a (twice), solo (once), multi_b (twice). Solo + // sits between the multis in insert order, so a non-compact scheme + // would assign it anchor 2 and skip to 3 for multi_b. + dedup.insert(foo.as_loc(), &multi_a); + dedup.insert(bar.as_loc(), &multi_a); + dedup.insert(foo.as_loc(), &solo); + dedup.insert(foo.as_loc(), &multi_b); + dedup.insert(bar.as_loc(), &multi_b); + let dedup = dedup.finalize(); + + assert_status( + &dedup, + &multi_a, + foo.as_loc(), + CompatRenderStatus::FirstOccurrence { anchor: Some(1) }, + ); + assert_status( + &dedup, + &solo, + foo.as_loc(), + CompatRenderStatus::FirstOccurrence { anchor: None }, + ); + assert_status( + &dedup, + &multi_b, + foo.as_loc(), + // multi_b must be anchor 2, not 3 — the solo issue between the + // two multis takes no slot in the visible numbering. + CompatRenderStatus::FirstOccurrence { anchor: Some(2) }, + ); + } + + /// Two issues at the same base with the same change set but reported in + /// different orders should dedup. The `changes` field is a `BTreeSet`, + /// so the comparison is order-independent regardless of what order + /// drift happened to emit the inner changes in. + #[test] + fn test_dedup_change_order_independent() { + fn make_change(message: &str, subpath: &str) -> SubpathChange { + SubpathChange { + class: ChangeClass::Incompatible, + message: message.into(), + old_subpath: DocumentPath::parse(subpath), + new_subpath: DocumentPath::parse(subpath), + } + } + + fn issue_with_changes( + changes: impl IntoIterator, + ) -> ApiCompatIssue { + ApiCompatIssue { + blessed_base: component_base("#/components/schemas/User"), + generated_base: component_base("#/components/schemas/User"), + changes: changes.into_iter().collect(), + tree: PathTree::default(), + blessed_value: Some(serde_json::json!({"User": {}})), + generated_value: Some(serde_json::json!({"User": {}})), + } + } + + let a_changes = [ + make_change("a changed", "properties/a"), + make_change("b changed", "properties/b"), + ]; + let b_changes = [ + make_change("b changed", "properties/b"), + make_change("a changed", "properties/a"), + ]; + + let issue_a = issue_with_changes(a_changes); + let issue_b = issue_with_changes(b_changes); + + assert!( + issue_a.is_same_change_as(&issue_b), + "issues with same change set in different order should dedup", + ); + + let foo = OwnedLoc::new("foo", "1.0.0"); + let bar = OwnedLoc::new("bar", "1.0.0"); + let mut dedup = CompatDedupMap::default(); + dedup.insert(foo.as_loc(), &issue_a); + dedup.insert(bar.as_loc(), &issue_b); + let dedup = dedup.finalize(); + assert_status( + &dedup, + &issue_b, + bar.as_loc(), + CompatRenderStatus::Duplicate { anchor: 1 }, + ); + } + + #[test] + #[should_panic(expected = "every issue passed to status_for was inserted")] + fn test_status_for_panics_on_uninserted() { + let issue = synthetic_issue( + "#/components/schemas/Error", + "schema types changed", + serde_json::json!({"Error": {"type": "string"}}), + serde_json::json!({"Error": {"type": "integer"}}), + ); + let foo = OwnedLoc::new("foo", "1.0.0"); + let dedup = CompatDedupMap::default().finalize(); + let _ = dedup.status_for(&issue, foo.as_loc()); + } } diff --git a/crates/dropshot-api-manager/src/compatibility/display.rs b/crates/dropshot-api-manager/src/compatibility/display.rs index 5d3c5d4..2eade6a 100644 --- a/crates/dropshot-api-manager/src/compatibility/display.rs +++ b/crates/dropshot-api-manager/src/compatibility/display.rs @@ -7,8 +7,8 @@ //! user sees on `cargo openapi check`. use super::types::{ - ApiCompatIssue, DocumentBasePath, DocumentPath, PathTree, PathTreeKey, - SubpathChange, + ApiCompatIssue, CompatRenderStatus, DocumentBasePath, DocumentPath, + PathTree, PathTreeKey, SubpathChange, }; use crate::output::Styles; use drift::ChangeClass; @@ -27,11 +27,12 @@ use std::{ /// within the same issue are all used by the same set of endpoints (by the /// structure of how OpenAPI works), so we combine those changes. /// -/// Separately, `output.rs` emits a single JSON diff per issue which is treated -/// as part of the block. +/// Separately, if `status` is `FirstOccurrence`, `output.rs` emits a single +/// JSON diff per issue. pub(crate) struct ApiCompatIssueDisplay<'a> { pub(super) issue: &'a ApiCompatIssue, pub(super) styles: &'a Styles, + pub(super) status: CompatRenderStatus, } impl fmt::Display for ApiCompatIssueDisplay<'_> { @@ -43,11 +44,13 @@ impl fmt::Display for ApiCompatIssueDisplay<'_> { // rendered block always ends mid-line regardless of whether a // `used by:` section follows. The caller supplies the trailing // newline (typically via `eprintln!` or `format!("{}\n", ...)`). + let mut marker = ChangeAnchor::from_status(self.status); for (i, change) in self.issue.changes.iter().enumerate() { if i > 0 { writeln!(f)?; } - self.write_change_header(f, change, label_width)?; + self.write_change_header(f, change, label_width, marker)?; + marker = None; } if !groups.is_empty() { writeln!(f)?; @@ -57,6 +60,29 @@ impl fmt::Display for ApiCompatIssueDisplay<'_> { } } +/// Marker rendered after the change message. +#[derive(Clone, Copy, Debug)] +enum ChangeAnchor { + Define(usize), + Reference(usize), +} + +impl ChangeAnchor { + /// Derive the marker (if any) for this issue's first severity line from + /// its [`CompatRenderStatus`]. + fn from_status(status: CompatRenderStatus) -> Option { + match status { + CompatRenderStatus::FirstOccurrence { anchor: Some(n) } => { + Some(Self::Define(n)) + } + CompatRenderStatus::FirstOccurrence { anchor: None } => None, + CompatRenderStatus::Duplicate { anchor } => { + Some(Self::Reference(anchor)) + } + } + } +} + impl ApiCompatIssueDisplay<'_> { /// Compute the label column width for this block. /// @@ -83,12 +109,13 @@ impl ApiCompatIssueDisplay<'_> { f: &mut fmt::Formatter<'_>, change: &SubpathChange, label_width: usize, + anchor: Option, ) -> fmt::Result { let styles = self.styles; - // Severity line: `: `. The surrounding cargo - // Failure header already names the API/version, so this line just - // describes the change itself. + // Severity line, with an optional `[#N]` / `(see #N)` anchor suffix. + // The number is shared so a terminal search for `#N` jumps between + // sites in scrollback. let severity = class_label(change.class); write_label( f, @@ -97,6 +124,13 @@ impl ApiCompatIssueDisplay<'_> { label_width, )?; f.write_str(&change.message)?; + if let Some(anchor) = anchor { + let text = match anchor { + ChangeAnchor::Define(n) => format!("[#{n}]"), + ChangeAnchor::Reference(n) => format!("(see #{n})"), + }; + write!(f, " {}", text.style(styles.warning))?; + } writeln!(f)?; write_label(f, "at", styles.dimmed, label_width)?; @@ -115,8 +149,8 @@ impl ApiCompatIssueDisplay<'_> { // This map is used to elide shared subtrees with `(*)` back-references, // similar to `cargo tree`. // - // Note that the same `base` with a different `subpath` is not deduped - // at that level, though it's likely to be deduped at the level above. + // Note that the same `base` with a different `subpath` doesn't dedup + // at that level, though it likely will at the level above. let mut seen = HashSet::new(); for (i, group) in groups.iter().enumerate() { if i > 0 { @@ -608,8 +642,8 @@ fn component_name(base: &DocumentPath) -> &str { mod tests { use super::{ super::types::{ - ApiCompatIssue, DocumentBasePath, DocumentPath, OperationIdMap, - PathTree, PathTreeKey, SubpathChange, + ApiCompatIssue, CompatRenderStatus, DocumentBasePath, DocumentPath, + OperationIdMap, PathTree, PathTreeKey, SubpathChange, }, *, }; @@ -651,7 +685,13 @@ mod tests { } let mut out = issues .iter() - .map(|i| i.display(styles).to_string()) + .map(|i| { + i.display( + styles, + CompatRenderStatus::FirstOccurrence { anchor: None }, + ) + .to_string() + }) .collect::>() .join("\n\n"); out.push('\n'); @@ -685,13 +725,14 @@ mod tests { fn assert_issue_snapshots(name: &str, issue: &ApiCompatIssue) { let mut colorized = Styles::default(); colorized.colorize(); + let status = CompatRenderStatus::FirstOccurrence { anchor: None }; expectorate::assert_contents( output_path(&format!("{name}.txt")), - &format!("{}\n", issue.display(&Styles::default())), + &format!("{}\n", issue.display(&Styles::default(), status)), ); expectorate::assert_contents( output_path(&format!("{name}.ansi")), - &format!("{}\n", issue.display(&colorized)), + &format!("{}\n", issue.display(&colorized, status)), ); } @@ -1064,6 +1105,121 @@ mod tests { ); } + /// Abbreviated rendering of an issue with multiple [`SubpathChange`]s. + /// + /// The `(see #N)` back-reference must appear *only* on the first severity + /// line. + #[test] + fn test_render_abbreviated_multi_change() { + let ops = op_ids(&[("#/paths/~1bar1/get", "get_bar1")]); + let make_chain = |route: &str| { + vec![ + PathTreeKey::parse( + "#/components/schemas/Wrapper/properties/value/$ref", + &ops, + ), + PathTreeKey::parse( + &format!( + "#/paths/~1{route}/get/responses/200/content/application~1json/schema/$ref" + ), + &ops, + ), + ] + }; + let mut tree = PathTree::default(); + tree.insert(make_chain("bar1")); + + let issue = ApiCompatIssue { + blessed_base: component("#/components/schemas/SubType"), + generated_base: component("#/components/schemas/SubType"), + changes: BTreeSet::from([ + SubpathChange { + class: ChangeClass::Incompatible, + message: "schema kind changed".into(), + old_subpath: DocumentPath::parse("properties/value"), + new_subpath: DocumentPath::parse("properties/value"), + }, + SubpathChange { + class: ChangeClass::Unhandled, + message: "object properties changed".into(), + old_subpath: DocumentPath::root(), + new_subpath: DocumentPath::root(), + }, + ]), + tree, + blessed_value: None, + generated_value: None, + }; + + let mut colorized = Styles::default(); + colorized.colorize(); + // 7 (rather than 1) to make it visually clear the marker is whatever + // anchor the caller passes, and is not tied to insert order. + let status = CompatRenderStatus::Duplicate { anchor: 7 }; + expectorate::assert_contents( + output_path("abbreviated_multi_change.txt"), + &format!("{}\n", issue.display(&Styles::default(), status)), + ); + expectorate::assert_contents( + output_path("abbreviated_multi_change.ansi"), + &format!("{}\n", issue.display(&colorized, status)), + ); + } + + /// Snapshot the abbreviated rendering used for duplicate issues. The + /// header and the per-API tree should both appear; the back-reference + /// `(see #N)` should be on the header line. + #[test] + fn test_render_abbreviated() { + let ops = op_ids(&[ + ("#/paths/~1bar1/get", "get_bar1"), + ("#/paths/~1bar2/get", "get_bar2"), + ]); + let make_chain = |route: &str| { + vec![ + PathTreeKey::parse( + "#/components/schemas/Wrapper/properties/value/$ref", + &ops, + ), + PathTreeKey::parse( + &format!( + "#/paths/~1{route}/get/responses/200/content/application~1json/schema/$ref" + ), + &ops, + ), + ] + }; + let mut tree = PathTree::default(); + tree.insert(make_chain("bar1")); + tree.insert(make_chain("bar2")); + + let issue = ApiCompatIssue { + blessed_base: component("#/components/schemas/SubType"), + generated_base: component("#/components/schemas/SubType"), + changes: BTreeSet::from([SubpathChange { + class: ChangeClass::Incompatible, + message: "schema types changed".into(), + old_subpath: DocumentPath::parse("properties/value"), + new_subpath: DocumentPath::parse("properties/value"), + }]), + tree, + blessed_value: None, + generated_value: None, + }; + + let mut colorized = Styles::default(); + colorized.colorize(); + let status = CompatRenderStatus::Duplicate { anchor: 3 }; + expectorate::assert_contents( + output_path("abbreviated.txt"), + &format!("{}\n", issue.display(&Styles::default(), status)), + ); + expectorate::assert_contents( + output_path("abbreviated.ansi"), + &format!("{}\n", issue.display(&colorized, status)), + ); + } + /// Snapshot the issue in its surrounding render context: a cargo-style /// `Failure` header, the second-tier `error:` keyword, and the issue /// body indented so its leftmost label aligns with where `error` @@ -1150,7 +1306,10 @@ mod tests { &mut indented, ), "{}", - issue.display(styles), + issue.display( + styles, + CompatRenderStatus::FirstOccurrence { anchor: None }, + ), ) .unwrap(); out.push_str(&indented); diff --git a/crates/dropshot-api-manager/src/compatibility/mod.rs b/crates/dropshot-api-manager/src/compatibility/mod.rs index 167410f..842d808 100644 --- a/crates/dropshot-api-manager/src/compatibility/mod.rs +++ b/crates/dropshot-api-manager/src/compatibility/mod.rs @@ -6,5 +6,9 @@ mod detect; mod display; mod types; -pub use detect::api_compatible; -pub use types::ApiCompatIssue; +pub(crate) use detect::{ + CompatDedupMap, FinalizedCompatDedupMap, api_compatible, +}; +pub(crate) use types::{ + ApiCompatIssue, CompatIssueLocation, CompatRenderStatus, +}; diff --git a/crates/dropshot-api-manager/src/compatibility/types.rs b/crates/dropshot-api-manager/src/compatibility/types.rs index 61e4930..9d991b6 100644 --- a/crates/dropshot-api-manager/src/compatibility/types.rs +++ b/crates/dropshot-api-manager/src/compatibility/types.rs @@ -10,6 +10,7 @@ use super::display::ApiCompatIssueDisplay; use crate::output::Styles; use drift::ChangeClass; +use dropshot_api_manager_types::ApiIdent; use std::collections::{BTreeMap, BTreeSet, HashMap}; /// A compatibility error between two OpenAPI documents. @@ -19,7 +20,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap}; /// inverted reference tree records every such chain so we can show all affected /// endpoints. #[derive(Debug)] -pub struct ApiCompatIssue { +pub(crate) struct ApiCompatIssue { /// Base location in the blessed (old) document, e.g. a schema component /// or an endpoint. pub(super) blessed_base: DocumentBasePath, @@ -30,8 +31,8 @@ pub struct ApiCompatIssue { pub(super) generated_base: DocumentBasePath, /// Non-trivial changes detected within this base location. /// - /// A `BTreeSet` rather than a `Vec` so iteration (and therefore display) - /// order is deterministic regardless of the order drift emits changes in. + /// This is a `BTreeSet` rather than a `Vec` so display order and dedup + /// comparisons are both independent of whatever drift returns. pub(super) changes: BTreeSet, /// Inverted reference tree. /// @@ -53,12 +54,59 @@ impl ApiCompatIssue { to_json_pretty(self.generated_value.as_ref()) } + /// Returns a `Display` adapter that renders this issue per `status`. pub(crate) fn display<'a>( &'a self, styles: &'a Styles, + status: CompatRenderStatus, ) -> ApiCompatIssueDisplay<'a> { - ApiCompatIssueDisplay { issue: self, styles } + ApiCompatIssueDisplay { issue: self, styles, status } } + + /// Returns true if `self` and `other` represent the same underlying + /// compatibility change for dedup purposes. + /// + /// `tree` is deliberately excluded: two sites reaching the same component + /// via different `$ref` chains are still the same change. (The + /// abbreviated display renders each site's tree, so no info is lost.) + pub(super) fn is_same_change_as(&self, other: &Self) -> bool { + let Self { + blessed_base, + generated_base, + changes, + tree: _, + blessed_value, + generated_value, + } = self; + *blessed_base == other.blessed_base + && *generated_base == other.generated_base + && *changes == other.changes + && *blessed_value == other.blessed_value + && *generated_value == other.generated_value + } +} + +/// A label identifying which API and version reported a given compatibility +/// issue. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct CompatIssueLocation<'a> { + pub(crate) api: &'a ApiIdent, + pub(crate) version: &'a semver::Version, +} + +/// How the issue renderer should render one compatibility issue at one `(api, +/// version)` site. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum CompatRenderStatus { + /// First (canonical) occurrence of the issue. + /// + /// `anchor` is `Some(n)` if the issue is also reported at another site. + FirstOccurrence { anchor: Option }, + /// Issue already rendered in full at an earlier site. + /// + /// The renderer emits the abbreviated form with a `(see #anchor)` + /// back-reference. + Duplicate { anchor: usize }, } /// A non-trivial change at a particular subpath within a base location. diff --git a/crates/dropshot-api-manager/src/output.rs b/crates/dropshot-api-manager/src/output.rs index 0a70cdd..e119c33 100644 --- a/crates/dropshot-api-manager/src/output.rs +++ b/crates/dropshot-api-manager/src/output.rs @@ -3,7 +3,10 @@ use crate::{ FAILURE_EXIT_CODE, NEEDS_UPDATE_EXIT_CODE, apis::{ManagedApi, ManagedApis}, - compatibility::ApiCompatIssue, + compatibility::{ + ApiCompatIssue, CompatIssueLocation, CompatRenderStatus, + FinalizedCompatDedupMap, + }, environment::{ErrorAccumulator, ResolvedEnv}, resolved::{ Fix, NonVersionProblem, Resolution, ResolutionKind, Resolved, @@ -306,6 +309,8 @@ pub fn display_resolution( let mut num_failed = 0; let mut num_non_version_problems = 0; + let dedup = resolved.build_compat_dedup_map(); + // Print problems associated with a supported API version // (i.e., one of the expected OpenAPI documents). for api in apis.iter_apis() { @@ -322,7 +327,9 @@ pub fn display_resolution( } else { num_fresh += 1; } - summarize_one(writer, env, api, version, resolution, styles)?; + summarize_one( + writer, env, api, version, resolution, styles, &dedup, + )?; } if !api.is_versioned() { @@ -483,6 +490,7 @@ fn summarize_one( version: &semver::Version, resolution: &Resolution<'_>, styles: &Styles, + dedup: &FinalizedCompatDedupMap<'_>, ) -> io::Result<()> { let problems: Vec<_> = resolution.problems().collect(); if problems.is_empty() { @@ -507,18 +515,31 @@ fn summarize_one( display_api_spec_version(api, version, styles, resolution), )?; - display_version_problems(writer, env, problems, styles)?; + let compat_ctx = CompatDisplayContext { + dedup, + current: CompatIssueLocation { api: api.ident(), version }, + }; + display_version_problems(writer, env, problems, styles, compat_ctx)?; } Ok(()) } +pub(crate) struct CompatDisplayContext<'a> { + pub(crate) dedup: &'a FinalizedCompatDedupMap<'a>, + pub(crate) current: CompatIssueLocation<'a>, +} + /// Print a formatted list of per-(api, version) [`VersionProblem`]s to /// `writer`, including any compatibility-issue diffs and fix descriptions. -pub fn display_version_problems<'a, T>( +/// +/// `compat_ctx` enables [`VersionProblem::BlessedVersionBroken`] rendering to +/// abbreviate compatibility issues that have already been shown elsewhere. +pub(crate) fn display_version_problems<'a, T>( writer: &mut dyn io::Write, env: &ResolvedEnv, problems: T, styles: &Styles, + compat_ctx: CompatDisplayContext<'_>, ) -> io::Result<()> where T: IntoIterator>, @@ -533,27 +554,24 @@ where // cargo headers. let issue_indent = " ".repeat(HEADER_WIDTH - "error".len()); - // For BlessedVersionBroken, print each item separately, along with a - // diff between blessed and generated versions. - // - // Each issue is surrounded by blank lines: the leading blank in - // `display_compat_issue` separates it from the problem header (or - // the previous issue's diff), and a single trailing blank here - // separates the last issue from the next problem. - if let VersionProblem::BlessedVersionBroken { compatibility_issues } = - &p - { - for issue in compatibility_issues { - display_compat_issue( - &mut *writer, - issue, - &issue_indent, - styles, - )?; - } - if !compatibility_issues.is_empty() { - writeln!(writer)?; - } + // Each issue gets a leading blank (emitted by `display_compat_issue`, + // separating it from the problem header or the previous diff) and a + // single trailing blank here (separating the last issue from the next + // problem). An issue already reported elsewhere is rendered in + // abbreviated form, pointing at the canonical occurrence. + let issues = p.compatibility_issues(); + for issue in issues { + let status = compat_ctx.dedup.status_for(issue, compat_ctx.current); + display_compat_issue( + &mut *writer, + issue, + &issue_indent, + styles, + status, + )?; + } + if !issues.is_empty() { + writeln!(writer)?; } // For BlessedLatestVersionBytewiseMismatch, show a diff between blessed @@ -742,6 +760,7 @@ fn display_compat_issue( issue: &ApiCompatIssue, body_indent: &str, styles: &Styles, + status: CompatRenderStatus, ) -> io::Result<()> { // A blank line separates this issue from the previous problem header // (or, in the full form, from the previous issue's JSON diff which @@ -754,29 +773,38 @@ fn display_compat_issue( write!( IndentWriter::new(body_indent, &mut buf), "{}", - issue.display(styles) + issue.display(styles, status), ) .expect("writing to a String never fails"); writeln!(writer, "{buf}")?; - // Print the textual diff between the blessed and generated values for - // this base. - let blessed_json = issue.blessed_json(); - let generated_json = issue.generated_json(); - - let diff = TextDiff::from_lines(&blessed_json, &generated_json); - write_diff( - &diff, - "blessed".as_ref(), - "generated".as_ref(), - styles, - // context_radius: use a large radius to ensure that most of the - // schema is printed out. - 8, - /* missing_newline_hint */ false, - // Align diff with the issue body. - &mut indent_write::io::IndentWriter::new(body_indent, writer), - ) + match status { + CompatRenderStatus::FirstOccurrence { .. } => { + // Full form: print the textual diff between the blessed and + // generated values for this base. + let blessed_json = issue.blessed_json(); + let generated_json = issue.generated_json(); + + let diff = TextDiff::from_lines(&blessed_json, &generated_json); + write_diff( + &diff, + "blessed".as_ref(), + "generated".as_ref(), + styles, + // context_radius: use a large radius to ensure that most + // of the schema is printed out. + 8, + /* missing_newline_hint */ false, + // Align diff with the issue body. + &mut indent_write::io::IndentWriter::new(body_indent, writer), + ) + } + CompatRenderStatus::Duplicate { .. } => { + // Abbreviated form: no JSON diff, since it was already shown + // at the first occurrence. + Ok(()) + } + } } /// Adapter for [`Error`]s that provides a [`std::fmt::Display`] implementation /// that print the full chain of error sources, separated by `: `. diff --git a/crates/dropshot-api-manager/src/resolved.rs b/crates/dropshot-api-manager/src/resolved.rs index 6039f5d..11f1dfc 100644 --- a/crates/dropshot-api-manager/src/resolved.rs +++ b/crates/dropshot-api-manager/src/resolved.rs @@ -4,7 +4,10 @@ use crate::{ apis::{ManagedApi, ManagedApis}, - compatibility::{ApiCompatIssue, api_compatible}, + compatibility::{ + ApiCompatIssue, CompatDedupMap, CompatIssueLocation, + FinalizedCompatDedupMap, api_compatible, + }, environment::ResolvedEnv, iter_only::iter_only, output::{InlineErrorChain, plural}, @@ -566,6 +569,33 @@ impl<'a> VersionProblem<'a> { self.fix().is_some() } + pub(crate) fn compatibility_issues(&self) -> &[ApiCompatIssue] { + // Exhaustive match to ensure new variants cause a compile error. + match self { + VersionProblem::BlessedVersionBroken { compatibility_issues } => { + compatibility_issues + } + VersionProblem::BlessedVersionMissingLocal { .. } + | VersionProblem::BlessedVersionExtraLocalSpec { .. } + | VersionProblem::BlessedVersionCompareError { .. } + | VersionProblem::BlessedLatestVersionBytewiseMismatch { .. } + | VersionProblem::LockstepMissingLocal { .. } + | VersionProblem::LockstepStale { .. } + | VersionProblem::LocalVersionMissingLocal { .. } + | VersionProblem::LocalVersionExtra { .. } + | VersionProblem::LocalVersionStale { .. } + | VersionProblem::GeneratedSourceMissing { .. } + | VersionProblem::GeneratedValidationError { .. } + | VersionProblem::ExtraFileStale { .. } + | VersionProblem::BlessedVersionShouldBeGitStub { .. } + | VersionProblem::GitStubShouldBeJson { .. } + | VersionProblem::BlessedVersionCorruptedLocal { .. } + | VersionProblem::DuplicateLocalFile { .. } + | VersionProblem::GitStubCommitStale { .. } + | VersionProblem::GitStubFirstCommitUnknown { .. } => &[], + } + } + pub fn fix(&'a self) -> Option> { match self { VersionProblem::BlessedVersionMissingLocal { @@ -1295,6 +1325,27 @@ impl<'a> Resolved<'a> { self.api_results.get(ident).and_then(|v| v.by_version.get(version)) } + /// Records every compatibility issue across every API and version in a + /// dedup map, then finalizes it for lookups. + /// + /// Iteration is sorted by API ident, then by semver, so the first + /// occurrence of an issue (which the renderer shows in full) is + /// deterministic. Subsequent occurrences are abbreviated. + pub(crate) fn build_compat_dedup_map(&self) -> FinalizedCompatDedupMap<'_> { + let mut dedup = CompatDedupMap::default(); + for (api, api_resolved) in &self.api_results { + for (version, resolution) in &api_resolved.by_version { + let location = CompatIssueLocation { api, version }; + for problem in resolution.problems() { + for issue in problem.compatibility_issues() { + dedup.insert(location, issue); + } + } + } + } + dedup.finalize() + } + /// Returns the "latest" symlink problem for an API, if any. /// /// Symlink problems are [`NonVersionProblem`]s but live separately from diff --git a/crates/dropshot-api-manager/tests/output/compatibility/abbreviated.ansi b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated.ansi new file mode 100644 index 0000000..7a2a7bc --- /dev/null +++ b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated.ansi @@ -0,0 +1,6 @@ +incompatible: schema types changed (see #3) + at: SubType.value + used by: GET /bar1 (get_bar1) + └─ Wrapper.value + used by: GET /bar2 (get_bar2) + └─ Wrapper.value (*) diff --git a/crates/dropshot-api-manager/tests/output/compatibility/abbreviated.txt b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated.txt new file mode 100644 index 0000000..935b06b --- /dev/null +++ b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated.txt @@ -0,0 +1,6 @@ +incompatible: schema types changed (see #3) + at: SubType.value + used by: GET /bar1 (get_bar1) + └─ Wrapper.value + used by: GET /bar2 (get_bar2) + └─ Wrapper.value (*) diff --git a/crates/dropshot-api-manager/tests/output/compatibility/abbreviated_multi_change.ansi b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated_multi_change.ansi new file mode 100644 index 0000000..8f6346a --- /dev/null +++ b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated_multi_change.ansi @@ -0,0 +1,6 @@ +incompatible: schema kind changed (see #7) + at: SubType.value + change: object properties changed + at: SubType + used by: GET /bar1 (get_bar1) + └─ Wrapper.value diff --git a/crates/dropshot-api-manager/tests/output/compatibility/abbreviated_multi_change.txt b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated_multi_change.txt new file mode 100644 index 0000000..4d10abd --- /dev/null +++ b/crates/dropshot-api-manager/tests/output/compatibility/abbreviated_multi_change.txt @@ -0,0 +1,6 @@ +incompatible: schema kind changed (see #7) + at: SubType.value + change: object properties changed + at: SubType + used by: GET /bar1 (get_bar1) + └─ Wrapper.value diff --git a/crates/integration-tests/tests/output/integration/blessed_version_broken.txt b/crates/integration-tests/tests/output/integration/blessed_version_broken.txt index dfe8a88..0b4f3f0 100644 --- a/crates/integration-tests/tests/output/integration/blessed_version_broken.txt +++ b/crates/integration-tests/tests/output/integration/blessed_version_broken.txt @@ -9,7 +9,7 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: A new, required parameter 'verbose' was added + backward-incompatible: A new, required parameter 'verbose' was added [#1] at: GET /health/detailed (detailed_health_check) (.parameters.0) --- a/blessed +++ b/generated @@ -41,6 +41,9 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) + backward-incompatible: A new, required parameter 'verbose' was added (see #1) + at: GET /health/detailed (detailed_health_check) (.parameters.0) + forward-incompatible: The operation get_system_info was added at: GET /system/info (get_system_info) --- a/blessed @@ -72,34 +75,6 @@ + } +} - backward-incompatible: A new, required parameter 'verbose' was added - at: GET /health/detailed (detailed_health_check) (.parameters.0) - --- a/blessed - +++ b/generated - @@ -1,11 +1,22 @@ - { - "get": { - + "description": "Adds a required query parameter `verbose` relative to the blessed shape. See `DetailedFilter` below.", - "operationId": "detailed_health_check", - + "parameters": [ - + { - + "in": "query", - + "name": "verbose", - + "required": true, - + "schema": { - + "type": "boolean" - + } - + } - + ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/DetailedHealthStatus" - } - } - Fresh versioned-health "latest" symlink ------- Failure 3 documents checked: 2 fresh, 0 stale, 2 failed, 0 other problems diff --git a/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt b/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt index c9279c2..e9242d0 100644 --- a/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt +++ b/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt @@ -9,7 +9,7 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: The parameter 'verbose' was removed + backward-incompatible: The parameter 'verbose' was removed [#1] at: GET /health/detailed (detailed_health_check) (.parameters.0) --- a/blessed +++ b/generated @@ -41,33 +41,8 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: The parameter 'verbose' was removed + backward-incompatible: The parameter 'verbose' was removed (see #1) at: GET /health/detailed (detailed_health_check) (.parameters.0) - --- a/blessed - +++ b/generated - @@ -1,22 +1,11 @@ - { - "get": { - - "description": "Adds a required query parameter `verbose` relative to the blessed shape. See `DetailedFilter` below.", - "operationId": "detailed_health_check", - - "parameters": [ - - { - - "in": "query", - - "name": "verbose", - - "required": true, - - "schema": { - - "type": "boolean" - - } - - } - - ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/DetailedHealthStatus" - } - } backward-incompatible: The operation get_system_info was removed at: GET /system/info (get_system_info) diff --git a/crates/integration-tests/tests/output/integration/cross_api_dedup.txt b/crates/integration-tests/tests/output/integration/cross_api_dedup.txt index 54244ac..772aaed 100644 --- a/crates/integration-tests/tests/output/integration/cross_api_dedup.txt +++ b/crates/integration-tests/tests/output/integration/cross_api_dedup.txt @@ -9,7 +9,7 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: A new, required parameter 'verbose' was added + backward-incompatible: A new, required parameter 'verbose' was added [#1] at: GET /health/detailed (detailed_health_check) (.parameters.0) --- a/blessed +++ b/generated @@ -41,7 +41,10 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - forward-incompatible: The operation get_system_info was added + backward-incompatible: A new, required parameter 'verbose' was added (see #1) + at: GET /health/detailed (detailed_health_check) (.parameters.0) + + forward-incompatible: The operation get_system_info was added [#2] at: GET /system/info (get_system_info) --- a/blessed +++ b/generated @@ -72,130 +75,24 @@ + } +} - backward-incompatible: A new, required parameter 'verbose' was added - at: GET /health/detailed (detailed_health_check) (.parameters.0) - --- a/blessed - +++ b/generated - @@ -1,11 +1,22 @@ - { - "get": { - + "description": "Adds a required query parameter `verbose` relative to the blessed shape. See `DetailedFilter` below.", - "operationId": "detailed_health_check", - + "parameters": [ - + { - + "in": "query", - + "name": "verbose", - + "required": true, - + "schema": { - + "type": "boolean" - + } - + } - + ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/DetailedHealthStatus" - } - } - Fresh versioned-health "latest" symlink Fresh versioned-monitor (versioned v1.0.0 (blessed)): Versioned Monitor API Failure versioned-monitor (versioned v2.0.0 (blessed)): Versioned Monitor API error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: A new, required parameter 'verbose' was added + backward-incompatible: A new, required parameter 'verbose' was added (see #1) at: GET /health/detailed (detailed_health_check) (.parameters.0) - --- a/blessed - +++ b/generated - @@ -1,11 +1,22 @@ - { - "get": { - + "description": "Adds a required query parameter `verbose` relative to the blessed shape. See `DetailedFilter` below.", - "operationId": "detailed_health_check", - + "parameters": [ - + { - + "in": "query", - + "name": "verbose", - + "required": true, - + "schema": { - + "type": "boolean" - + } - + } - + ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/DetailedHealthStatus" - } - } Failure versioned-monitor (versioned v3.0.0 (blessed)): Versioned Monitor API error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - forward-incompatible: The operation get_system_info was added - at: GET /system/info (get_system_info) - --- a/blessed - +++ b/generated - @@ -0,0 +1,25 @@ - +{ - + "get": { - + "description": "This breaks backward compatibility by adding a new endpoint to v3.0.0.", - + "operationId": "get_system_info", - + "responses": { - + "200": { - + "content": { - + "application/json": { - + "schema": { - + "$ref": "#/components/schemas/SystemInfo" - + } - + } - + }, - + "description": "successful operation" - + }, - + "4XX": { - + "$ref": "#/components/responses/Error" - + }, - + "5XX": { - + "$ref": "#/components/responses/Error" - + } - + }, - + "summary": "Get system info (v3+): new endpoint added to existing version." - + } - +} - - backward-incompatible: A new, required parameter 'verbose' was added + backward-incompatible: A new, required parameter 'verbose' was added (see #1) at: GET /health/detailed (detailed_health_check) (.parameters.0) - --- a/blessed - +++ b/generated - @@ -1,11 +1,22 @@ - { - "get": { - + "description": "Adds a required query parameter `verbose` relative to the blessed shape. See `DetailedFilter` below.", - "operationId": "detailed_health_check", - + "parameters": [ - + { - + "in": "query", - + "name": "verbose", - + "required": true, - + "schema": { - + "type": "boolean" - + } - + } - + ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/DetailedHealthStatus" - } - } + + forward-incompatible: The operation get_system_info was added (see #2) + at: GET /system/info (get_system_info) Fresh versioned-monitor "latest" symlink -------