From 0ad81ba0699082e0585d0b7001bfbf08f669bb05 Mon Sep 17 00:00:00 2001 From: devfive Date: Sun, 30 Aug 2026 02:18:18 +0900 Subject: [PATCH] Detect schema conflicts across merged apps --- ...changepack_log_schema-merge-collision.json | 1 + crates/vespera_macro/src/metadata.rs | 13 +- crates/vespera_macro/src/vespera_impl.rs | 1 + .../src/vespera_impl/openapi_io.rs | 70 +++++++- .../src/vespera_impl/schema_merge.rs | 152 ++++++++++++++++++ 5 files changed, 234 insertions(+), 3 deletions(-) create mode 100644 .changepacks/changepack_log_schema-merge-collision.json create mode 100644 crates/vespera_macro/src/vespera_impl/schema_merge.rs diff --git a/.changepacks/changepack_log_schema-merge-collision.json b/.changepacks/changepack_log_schema-merge-collision.json new file mode 100644 index 00000000..0a57cd02 --- /dev/null +++ b/.changepacks/changepack_log_schema-merge-collision.json @@ -0,0 +1 @@ +{"changes":{"crates/vespera_macro/Cargo.toml":"Minor"},"note":"Fail the build when merged apps define conflicting same-named OpenAPI schemas, turning a previously silent first-wins condition into an actionable compile error while preserving identical-schema deduplication.","date":"2026-08-29T17:10:09.921Z"} diff --git a/crates/vespera_macro/src/metadata.rs b/crates/vespera_macro/src/metadata.rs index ccdbae59..27ba6ea9 100644 --- a/crates/vespera_macro/src/metadata.rs +++ b/crates/vespera_macro/src/metadata.rs @@ -184,9 +184,18 @@ impl CollectedMetadata { if let Some(&prev_idx) = seen.get(s.name.as_str()) { // Only report if definitions actually differ (identical re-registration is OK) if self.structs[prev_idx].definition != s.definition { + let origins = match ( + self.structs[prev_idx].source_identity.as_deref(), + s.source_identity.as_deref(), + ) { + (Some(first), Some(second)) => { + format!(" Conflicting definitions came from {first} and {second}.") + } + _ => String::new(), + }; return Err(format!( - "Duplicate OpenAPI schema name '{}'. Two different structs produce the same schema name, which would corrupt the OpenAPI spec. Rename one of them or use #[schema(name = \"...\")].", - s.name + "Duplicate OpenAPI schema name '{}'. Two different structs produce the same schema name, which would corrupt the OpenAPI spec.{origins} Rename one of them or use #[schema(name = \"...\")].", + s.name, )); } } else { diff --git a/crates/vespera_macro/src/vespera_impl.rs b/crates/vespera_macro/src/vespera_impl.rs index 1adc45cc..ee803144 100644 --- a/crates/vespera_macro/src/vespera_impl.rs +++ b/crates/vespera_macro/src/vespera_impl.rs @@ -9,5 +9,6 @@ mod openapi_io; mod orchestrator; mod path_utils; mod route_merge; +mod schema_merge; pub use orchestrator::{process_export_app, process_vespera_macro}; diff --git a/crates/vespera_macro/src/vespera_impl/openapi_io.rs b/crates/vespera_macro/src/vespera_impl/openapi_io.rs index ac45cf2f..4ce3662a 100644 --- a/crates/vespera_macro/src/vespera_impl/openapi_io.rs +++ b/crates/vespera_macro/src/vespera_impl/openapi_io.rs @@ -8,12 +8,22 @@ use crate::{ router_codegen::ProcessedVesperaInput, }; use proc_macro2::Span; +use syn::spanned::Spanned; use super::{ cache::{MergeSpecCache, MergeSpecRead, path_fingerprint}, path_utils::{current_crate_tag, find_target_dir}, + schema_merge::SchemaMergeGuard, }; +fn display_merge_path(path: &syn::Path) -> String { + path.segments + .iter() + .map(|segment| segment.ident.to_string()) + .collect::>() + .join("::") +} + /// OpenAPI write result consumed by router/doc codegen and incremental cache sidecars. /// /// The docs/redoc URLs are intentionally **not** carried here: the sole @@ -80,8 +90,12 @@ pub fn generate_and_write_openapi( route_storage, )?; - // Merge specs from child apps at compile time + // Merge specs from child apps at compile time. This is the one point where + // the parent and every exported child definition are all available, so + // schema-name conflicts are checked here before OpenApi's first-wins merge + // can discard a later definition. if !input.merge.is_empty() { + let mut schema_guard = SchemaMergeGuard::new(&openapi_doc)?; for merge_path in &input.merge { // Extract the struct name (last segment, e.g., "ThirdApp" from "third::ThirdApp") if let Some((struct_name, spec_file)) = merge_specs.spec_file_for(merge_path) { @@ -95,6 +109,8 @@ pub fn generate_and_write_openapi( } }; let child_spec = serde_json::from_str::(spec_content).map_err(|e| err_call_site(format!("OpenAPI merge: failed to parse child spec for `{struct_name}` at '{}'. Error: {e}.", spec_file.display())))?; + let child_origin = format!("merged app `{}`", display_merge_path(merge_path)); + schema_guard.check_child(&child_spec, &child_origin, merge_path.span())?; openapi_doc.merge(child_spec); } } @@ -594,6 +610,58 @@ mod tests { assert!(result.is_ok()); } + #[serial_test::serial] + #[test] + fn merged_child_schema_conflict_is_a_spanned_compile_error() { + let temp_dir = TempDir::new().unwrap(); + let target_dir = temp_dir.path().join("target/vespera"); + fs::create_dir_all(&target_dir).unwrap(); + let spec = |property: &str, schema_type: &str| { + serde_json::json!({ + "openapi": "3.1.0", + "info": { "title": "child", "version": "1.0.0" }, + "paths": {}, + "components": { + "schemas": { + "ExampleItem": { + "type": "object", + "properties": { (property): { "type": schema_type } } + } + } + } + }) + .to_string() + }; + fs::write( + target_dir.join("PluginA.openapi.json"), + spec("id", "string"), + ) + .unwrap(); + fs::write( + target_dir.join("PluginB.openapi.json"), + spec("collisionMarker", "boolean"), + ) + .unwrap(); + + let _restore = RestoreManifest(std::env::var("CARGO_MANIFEST_DIR").ok()); + // SAFETY: this serialized test restores the process environment through RAII. + unsafe { std::env::set_var("CARGO_MANIFEST_DIR", temp_dir.path()) }; + let mut processed = merge_input(syn::parse_quote!(plugin_a::PluginA)); + processed.merge.push(syn::parse_quote!(plugin_b::PluginB)); + let error = generate_and_write_openapi( + &processed, + &CollectedMetadata::new(), + HashMap::new(), + &[], + &mut MergeSpecCache::new(), + ) + .expect_err("different same-named child schemas must fail the build"); + let message = error.to_string(); + assert!(message.contains("Duplicate OpenAPI schema name 'ExampleItem'")); + assert!(message.contains("plugin_a::PluginA")); + assert!(message.contains("plugin_b::PluginB")); + } + #[test] fn test_generate_and_write_openapi_file_write_error() { // Line 95: fs::write failure when output path is a directory diff --git a/crates/vespera_macro/src/vespera_impl/schema_merge.rs b/crates/vespera_macro/src/vespera_impl/schema_merge.rs new file mode 100644 index 00000000..0e2c067a --- /dev/null +++ b/crates/vespera_macro/src/vespera_impl/schema_merge.rs @@ -0,0 +1,152 @@ +use proc_macro2::Span; +use vespera_core::openapi::OpenApi; + +use crate::{ + error::MacroResult, + metadata::{CollectedMetadata, StructMetadata}, +}; + +/// Tracks component-schema definitions and their source while exported apps +/// are folded into a parent document. +pub(super) struct SchemaMergeGuard { + metadata: CollectedMetadata, +} + +impl SchemaMergeGuard { + pub(super) fn new(parent: &OpenApi) -> MacroResult { + let mut guard = Self { + metadata: CollectedMetadata::new(), + }; + guard.record(parent, "the parent app", Span::call_site())?; + Ok(guard) + } + + /// Reject a child whose component name is already attached to a different + /// JSON Schema. Equal definitions are intentionally retained as normal + /// deduplication. + pub(super) fn check_child( + &mut self, + child: &OpenApi, + child_origin: &str, + span: Span, + ) -> MacroResult<()> { + self.record(child, child_origin, span) + } + + fn record(&mut self, document: &OpenApi, origin: &str, span: Span) -> MacroResult<()> { + let Some(schemas) = document + .components + .as_ref() + .and_then(|components| components.schemas.as_ref()) + else { + return Ok(()); + }; + + for (name, schema) in schemas { + let definition = serde_json::to_string(schema).map_err(|error| { + syn::Error::new( + span, + format!( + "OpenAPI merge: failed to compare schema `{name}` from {origin}. Error: {error}." + ), + ) + })?; + + self.metadata.structs.push( + StructMetadata::new(name.clone(), definition) + .with_source_identity(origin.to_string()), + ); + } + + self.metadata + .check_duplicate_schema_names() + .map_err(|message| syn::Error::new(span, format!("OpenAPI merge: {message}"))) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn document(schema: &serde_json::Value) -> OpenApi { + serde_json::from_value(serde_json::json!({ + "openapi": "3.1.0", + "info": { "title": "test", "version": "1.0.0" }, + "paths": {}, + "components": { "schemas": { "ExampleItem": schema } } + })) + .unwrap() + } + + #[test] + fn different_same_named_schemas_are_rejected_with_both_origins() { + let parent = document(&serde_json::json!({ + "type": "object", + "properties": { "id": { "type": "string" } } + })); + let child = document(&serde_json::json!({ + "type": "object", + "properties": { "collisionMarker": { "type": "boolean" } } + })); + let mut guard = SchemaMergeGuard::new(&parent).unwrap(); + + let error = guard + .check_child(&child, "merged app `plugin_b::PluginB`", Span::call_site()) + .expect_err("different definitions must fail"); + let message = error.to_string(); + + assert!(message.contains("Duplicate OpenAPI schema name 'ExampleItem'")); + assert!(message.contains("plugin_b::PluginB")); + assert!(message.contains("parent app")); + } + + #[test] + fn identical_same_named_schemas_are_accepted() { + let schema = serde_json::json!({ + "type": "object", + "properties": { + "error": { "type": "string" }, + "code": { "type": "integer" } + }, + "required": ["error", "code"] + }); + let parent = document(&schema); + let child = document(&schema); + let mut guard = SchemaMergeGuard::new(&parent).unwrap(); + + guard + .check_child(&child, "merged app `plugin::Plugin`", Span::call_site()) + .expect("identical definitions should deduplicate"); + } + + #[test] + fn a_schema_defined_once_is_unaffected() { + let parent = document(&serde_json::json!({ "type": "string" })); + + SchemaMergeGuard::new(&parent).expect("one definition should be accepted"); + } + + #[test] + fn child_to_child_conflict_reports_the_first_child() { + let empty: OpenApi = serde_json::from_value(serde_json::json!({ + "openapi": "3.1.0", + "info": { "title": "test", "version": "1.0.0" }, + "paths": {} + })) + .unwrap(); + let first = document(&serde_json::json!({ "type": "string" })); + let second = document(&serde_json::json!({ "type": "integer" })); + let mut guard = SchemaMergeGuard::new(&empty).unwrap(); + guard + .check_child(&first, "merged app `plugin_a::PluginA`", Span::call_site()) + .unwrap(); + + let error = guard + .check_child(&second, "merged app `plugin_b::PluginB`", Span::call_site()) + .expect_err("the later child must conflict with the first"); + let message = error.to_string(); + + assert!(message.contains("plugin_a::PluginA")); + assert!(message.contains("plugin_b::PluginB")); + } +}