Skip to content

Commit e7d8c78

Browse files
authored
Merge pull request #139 from dev-five-git/owjs3901/vespera-schema-collision
Fail merged apps with conflicting schema names
2 parents 28a25b0 + 0ad81ba commit e7d8c78

5 files changed

Lines changed: 234 additions & 3 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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"}

crates/vespera_macro/src/metadata.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,18 @@ impl CollectedMetadata {
184184
if let Some(&prev_idx) = seen.get(s.name.as_str()) {
185185
// Only report if definitions actually differ (identical re-registration is OK)
186186
if self.structs[prev_idx].definition != s.definition {
187+
let origins = match (
188+
self.structs[prev_idx].source_identity.as_deref(),
189+
s.source_identity.as_deref(),
190+
) {
191+
(Some(first), Some(second)) => {
192+
format!(" Conflicting definitions came from {first} and {second}.")
193+
}
194+
_ => String::new(),
195+
};
187196
return Err(format!(
188-
"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 = \"...\")].",
189-
s.name
197+
"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 = \"...\")].",
198+
s.name,
190199
));
191200
}
192201
} else {

crates/vespera_macro/src/vespera_impl.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ mod openapi_io;
99
mod orchestrator;
1010
mod path_utils;
1111
mod route_merge;
12+
mod schema_merge;
1213

1314
pub use orchestrator::{process_export_app, process_vespera_macro};

crates/vespera_macro/src/vespera_impl/openapi_io.rs

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,22 @@ use crate::{
88
router_codegen::ProcessedVesperaInput,
99
};
1010
use proc_macro2::Span;
11+
use syn::spanned::Spanned;
1112

1213
use super::{
1314
cache::{MergeSpecCache, MergeSpecRead, path_fingerprint},
1415
path_utils::{current_crate_tag, find_target_dir},
16+
schema_merge::SchemaMergeGuard,
1517
};
1618

19+
fn display_merge_path(path: &syn::Path) -> String {
20+
path.segments
21+
.iter()
22+
.map(|segment| segment.ident.to_string())
23+
.collect::<Vec<_>>()
24+
.join("::")
25+
}
26+
1727
/// OpenAPI write result consumed by router/doc codegen and incremental cache sidecars.
1828
///
1929
/// The docs/redoc URLs are intentionally **not** carried here: the sole
@@ -80,8 +90,12 @@ pub fn generate_and_write_openapi(
8090
route_storage,
8191
)?;
8292

83-
// Merge specs from child apps at compile time
93+
// Merge specs from child apps at compile time. This is the one point where
94+
// the parent and every exported child definition are all available, so
95+
// schema-name conflicts are checked here before OpenApi's first-wins merge
96+
// can discard a later definition.
8497
if !input.merge.is_empty() {
98+
let mut schema_guard = SchemaMergeGuard::new(&openapi_doc)?;
8599
for merge_path in &input.merge {
86100
// Extract the struct name (last segment, e.g., "ThirdApp" from "third::ThirdApp")
87101
if let Some((struct_name, spec_file)) = merge_specs.spec_file_for(merge_path) {
@@ -95,6 +109,8 @@ pub fn generate_and_write_openapi(
95109
}
96110
};
97111
let child_spec = serde_json::from_str::<vespera_core::openapi::OpenApi>(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())))?;
112+
let child_origin = format!("merged app `{}`", display_merge_path(merge_path));
113+
schema_guard.check_child(&child_spec, &child_origin, merge_path.span())?;
98114
openapi_doc.merge(child_spec);
99115
}
100116
}
@@ -594,6 +610,58 @@ mod tests {
594610
assert!(result.is_ok());
595611
}
596612

613+
#[serial_test::serial]
614+
#[test]
615+
fn merged_child_schema_conflict_is_a_spanned_compile_error() {
616+
let temp_dir = TempDir::new().unwrap();
617+
let target_dir = temp_dir.path().join("target/vespera");
618+
fs::create_dir_all(&target_dir).unwrap();
619+
let spec = |property: &str, schema_type: &str| {
620+
serde_json::json!({
621+
"openapi": "3.1.0",
622+
"info": { "title": "child", "version": "1.0.0" },
623+
"paths": {},
624+
"components": {
625+
"schemas": {
626+
"ExampleItem": {
627+
"type": "object",
628+
"properties": { (property): { "type": schema_type } }
629+
}
630+
}
631+
}
632+
})
633+
.to_string()
634+
};
635+
fs::write(
636+
target_dir.join("PluginA.openapi.json"),
637+
spec("id", "string"),
638+
)
639+
.unwrap();
640+
fs::write(
641+
target_dir.join("PluginB.openapi.json"),
642+
spec("collisionMarker", "boolean"),
643+
)
644+
.unwrap();
645+
646+
let _restore = RestoreManifest(std::env::var("CARGO_MANIFEST_DIR").ok());
647+
// SAFETY: this serialized test restores the process environment through RAII.
648+
unsafe { std::env::set_var("CARGO_MANIFEST_DIR", temp_dir.path()) };
649+
let mut processed = merge_input(syn::parse_quote!(plugin_a::PluginA));
650+
processed.merge.push(syn::parse_quote!(plugin_b::PluginB));
651+
let error = generate_and_write_openapi(
652+
&processed,
653+
&CollectedMetadata::new(),
654+
HashMap::new(),
655+
&[],
656+
&mut MergeSpecCache::new(),
657+
)
658+
.expect_err("different same-named child schemas must fail the build");
659+
let message = error.to_string();
660+
assert!(message.contains("Duplicate OpenAPI schema name 'ExampleItem'"));
661+
assert!(message.contains("plugin_a::PluginA"));
662+
assert!(message.contains("plugin_b::PluginB"));
663+
}
664+
597665
#[test]
598666
fn test_generate_and_write_openapi_file_write_error() {
599667
// Line 95: fs::write failure when output path is a directory
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
use proc_macro2::Span;
2+
use vespera_core::openapi::OpenApi;
3+
4+
use crate::{
5+
error::MacroResult,
6+
metadata::{CollectedMetadata, StructMetadata},
7+
};
8+
9+
/// Tracks component-schema definitions and their source while exported apps
10+
/// are folded into a parent document.
11+
pub(super) struct SchemaMergeGuard {
12+
metadata: CollectedMetadata,
13+
}
14+
15+
impl SchemaMergeGuard {
16+
pub(super) fn new(parent: &OpenApi) -> MacroResult<Self> {
17+
let mut guard = Self {
18+
metadata: CollectedMetadata::new(),
19+
};
20+
guard.record(parent, "the parent app", Span::call_site())?;
21+
Ok(guard)
22+
}
23+
24+
/// Reject a child whose component name is already attached to a different
25+
/// JSON Schema. Equal definitions are intentionally retained as normal
26+
/// deduplication.
27+
pub(super) fn check_child(
28+
&mut self,
29+
child: &OpenApi,
30+
child_origin: &str,
31+
span: Span,
32+
) -> MacroResult<()> {
33+
self.record(child, child_origin, span)
34+
}
35+
36+
fn record(&mut self, document: &OpenApi, origin: &str, span: Span) -> MacroResult<()> {
37+
let Some(schemas) = document
38+
.components
39+
.as_ref()
40+
.and_then(|components| components.schemas.as_ref())
41+
else {
42+
return Ok(());
43+
};
44+
45+
for (name, schema) in schemas {
46+
let definition = serde_json::to_string(schema).map_err(|error| {
47+
syn::Error::new(
48+
span,
49+
format!(
50+
"OpenAPI merge: failed to compare schema `{name}` from {origin}. Error: {error}."
51+
),
52+
)
53+
})?;
54+
55+
self.metadata.structs.push(
56+
StructMetadata::new(name.clone(), definition)
57+
.with_source_identity(origin.to_string()),
58+
);
59+
}
60+
61+
self.metadata
62+
.check_duplicate_schema_names()
63+
.map_err(|message| syn::Error::new(span, format!("OpenAPI merge: {message}")))
64+
}
65+
}
66+
67+
#[cfg(test)]
68+
mod tests {
69+
use super::*;
70+
71+
fn document(schema: &serde_json::Value) -> OpenApi {
72+
serde_json::from_value(serde_json::json!({
73+
"openapi": "3.1.0",
74+
"info": { "title": "test", "version": "1.0.0" },
75+
"paths": {},
76+
"components": { "schemas": { "ExampleItem": schema } }
77+
}))
78+
.unwrap()
79+
}
80+
81+
#[test]
82+
fn different_same_named_schemas_are_rejected_with_both_origins() {
83+
let parent = document(&serde_json::json!({
84+
"type": "object",
85+
"properties": { "id": { "type": "string" } }
86+
}));
87+
let child = document(&serde_json::json!({
88+
"type": "object",
89+
"properties": { "collisionMarker": { "type": "boolean" } }
90+
}));
91+
let mut guard = SchemaMergeGuard::new(&parent).unwrap();
92+
93+
let error = guard
94+
.check_child(&child, "merged app `plugin_b::PluginB`", Span::call_site())
95+
.expect_err("different definitions must fail");
96+
let message = error.to_string();
97+
98+
assert!(message.contains("Duplicate OpenAPI schema name 'ExampleItem'"));
99+
assert!(message.contains("plugin_b::PluginB"));
100+
assert!(message.contains("parent app"));
101+
}
102+
103+
#[test]
104+
fn identical_same_named_schemas_are_accepted() {
105+
let schema = serde_json::json!({
106+
"type": "object",
107+
"properties": {
108+
"error": { "type": "string" },
109+
"code": { "type": "integer" }
110+
},
111+
"required": ["error", "code"]
112+
});
113+
let parent = document(&schema);
114+
let child = document(&schema);
115+
let mut guard = SchemaMergeGuard::new(&parent).unwrap();
116+
117+
guard
118+
.check_child(&child, "merged app `plugin::Plugin`", Span::call_site())
119+
.expect("identical definitions should deduplicate");
120+
}
121+
122+
#[test]
123+
fn a_schema_defined_once_is_unaffected() {
124+
let parent = document(&serde_json::json!({ "type": "string" }));
125+
126+
SchemaMergeGuard::new(&parent).expect("one definition should be accepted");
127+
}
128+
129+
#[test]
130+
fn child_to_child_conflict_reports_the_first_child() {
131+
let empty: OpenApi = serde_json::from_value(serde_json::json!({
132+
"openapi": "3.1.0",
133+
"info": { "title": "test", "version": "1.0.0" },
134+
"paths": {}
135+
}))
136+
.unwrap();
137+
let first = document(&serde_json::json!({ "type": "string" }));
138+
let second = document(&serde_json::json!({ "type": "integer" }));
139+
let mut guard = SchemaMergeGuard::new(&empty).unwrap();
140+
guard
141+
.check_child(&first, "merged app `plugin_a::PluginA`", Span::call_site())
142+
.unwrap();
143+
144+
let error = guard
145+
.check_child(&second, "merged app `plugin_b::PluginB`", Span::call_site())
146+
.expect_err("the later child must conflict with the first");
147+
let message = error.to_string();
148+
149+
assert!(message.contains("plugin_a::PluginA"));
150+
assert!(message.contains("plugin_b::PluginB"));
151+
}
152+
}

0 commit comments

Comments
 (0)