Skip to content

Commit 658c2d0

Browse files
committed
Refactor finding logic
1 parent 05e2797 commit 658c2d0

13 files changed

Lines changed: 240 additions & 67 deletions

File tree

crates/vespera_macro/src/lib.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,13 @@ pub fn export_app(input: TokenStream) -> TokenStream {
294294
.lock()
295295
.unwrap_or_else(std::sync::PoisonError::into_inner);
296296

297-
match process_export_app(&name, &folder_name, &schema_storage, &manifest_dir, &route_storage) {
297+
match process_export_app(
298+
&name,
299+
&folder_name,
300+
&schema_storage,
301+
&manifest_dir,
302+
&route_storage,
303+
) {
298304
Ok(tokens) => tokens.into(),
299305
Err(e) => e.to_compile_error().into(),
300306
}

crates/vespera_macro/src/metadata.rs

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Metadata collection and storage for routes and schemas
22
3-
use std::collections::BTreeMap;
3+
use std::collections::{BTreeMap, HashMap};
44

55
use serde::{Deserialize, Serialize};
66

@@ -102,6 +102,29 @@ impl CollectedMetadata {
102102
structs: Vec::new(),
103103
}
104104
}
105+
106+
/// Check for duplicate schema names among `include_in_openapi` structs.
107+
/// Returns `Err` with a descriptive message if duplicates are found.
108+
pub fn check_duplicate_schema_names(&self) -> Result<(), String> {
109+
let mut seen: HashMap<&str, usize> = HashMap::new();
110+
for (i, s) in self.structs.iter().enumerate() {
111+
if !s.include_in_openapi {
112+
continue;
113+
}
114+
if let Some(&prev_idx) = seen.get(s.name.as_str()) {
115+
// Only report if definitions actually differ (identical re-registration is OK)
116+
if self.structs[prev_idx].definition != s.definition {
117+
return Err(format!(
118+
"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 = \"...\")].",
119+
s.name
120+
));
121+
}
122+
} else {
123+
seen.insert(&s.name, i);
124+
}
125+
}
126+
Ok(())
127+
}
105128
}
106129

107130
#[cfg(test)]
@@ -167,4 +190,63 @@ mod tests {
167190
assert!(meta.routes.is_empty());
168191
assert!(meta.structs.is_empty());
169192
}
193+
194+
#[test]
195+
fn test_check_duplicate_schema_names_no_duplicates() {
196+
let mut meta = CollectedMetadata::new();
197+
meta.structs
198+
.push(StructMetadata::new("User".into(), "struct User {}".into()));
199+
meta.structs
200+
.push(StructMetadata::new("Post".into(), "struct Post {}".into()));
201+
assert!(meta.check_duplicate_schema_names().is_ok());
202+
}
203+
204+
#[test]
205+
fn test_check_duplicate_schema_names_different_definitions() {
206+
let mut meta = CollectedMetadata::new();
207+
meta.structs.push(StructMetadata::new(
208+
"User".into(),
209+
"struct User { id: i32 }".into(),
210+
));
211+
meta.structs.push(StructMetadata::new(
212+
"User".into(),
213+
"struct User { name: String }".into(),
214+
));
215+
let err = meta.check_duplicate_schema_names().unwrap_err();
216+
assert!(
217+
err.contains("Duplicate OpenAPI schema name 'User'"),
218+
"got: {err}"
219+
);
220+
}
221+
222+
#[test]
223+
fn test_check_duplicate_schema_names_identical_definition_ok() {
224+
let mut meta = CollectedMetadata::new();
225+
let def = "struct User { id: i32 }".to_string();
226+
meta.structs
227+
.push(StructMetadata::new("User".into(), def.clone()));
228+
meta.structs.push(StructMetadata::new("User".into(), def));
229+
assert!(meta.check_duplicate_schema_names().is_ok());
230+
}
231+
232+
#[test]
233+
fn test_check_duplicate_schema_names_ignores_models() {
234+
let mut meta = CollectedMetadata::new();
235+
meta.structs.push(StructMetadata::new_model(
236+
"Model".into(),
237+
"struct Model { id: i32 }".into(),
238+
));
239+
meta.structs.push(StructMetadata::new_model(
240+
"Model".into(),
241+
"struct Model { name: String }".into(),
242+
));
243+
// Models (include_in_openapi=false) are not checked
244+
assert!(meta.check_duplicate_schema_names().is_ok());
245+
}
246+
247+
#[test]
248+
fn test_check_duplicate_schema_names_empty() {
249+
let meta = CollectedMetadata::new();
250+
assert!(meta.check_duplicate_schema_names().is_ok());
251+
}
170252
}

crates/vespera_macro/src/openapi_generator.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,12 @@ fn parse_component_schemas(
211211
});
212212

213213
if let Some(ast) = file_ast {
214-
process_default_functions(struct_item, ast, &mut schema, &struct_meta.field_defaults);
214+
process_default_functions(
215+
struct_item,
216+
ast,
217+
&mut schema,
218+
&struct_meta.field_defaults,
219+
);
215220
}
216221
}
217222

@@ -267,7 +272,8 @@ fn build_path_items(
267272

268273
for route_meta in &metadata.routes {
269274
// Try ROUTE_STORAGE first (avoids file_cache dependency for known routes)
270-
let fn_sig = if let Some(cached_fn) = route_fn_cache.get(route_meta.function_name.as_str()) {
275+
let fn_sig = if let Some(cached_fn) = route_fn_cache.get(route_meta.function_name.as_str())
276+
{
271277
&cached_fn.sig
272278
} else if let Some(fns) = fn_index.get(route_meta.file_path.as_str())
273279
&& let Some(fn_item) = fns.get(&route_meta.function_name)
@@ -756,7 +762,14 @@ pub fn get_user() -> User {
756762
description: None,
757763
});
758764

759-
let doc = generate_openapi_doc_with_metadata(Some("Test API".to_string()), Some("1.0.0".to_string()), None, &metadata, None, &[]);
765+
let doc = generate_openapi_doc_with_metadata(
766+
Some("Test API".to_string()),
767+
Some("1.0.0".to_string()),
768+
None,
769+
&metadata,
770+
None,
771+
&[],
772+
);
760773

761774
// Check struct schema
762775
assert!(doc.components.as_ref().unwrap().schemas.is_some());
@@ -955,7 +968,8 @@ pub fn get_users() -> String {
955968
},
956969
];
957970

958-
let doc = generate_openapi_doc_with_metadata(None, None, Some(servers), &metadata, None, &[]);
971+
let doc =
972+
generate_openapi_doc_with_metadata(None, None, Some(servers), &metadata, None, &[]);
959973

960974
assert!(doc.servers.is_some());
961975
let doc_servers = doc.servers.unwrap();

crates/vespera_macro/src/parser/schema/type_schema.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,10 +264,13 @@ fn parse_type_impl(
264264
"Uuid" => string_with_format("uuid"),
265265
"String" | "str" => SchemaRef::Inline(Box::new(Schema::string())),
266266
// Date-time types from chrono and time crates
267-
"DateTime" | "NaiveDateTime" | "DateTimeWithTimeZone" | "DateTimeUtc"
268-
| "DateTimeLocal" | "OffsetDateTime" | "PrimitiveDateTime" => {
269-
string_with_format("date-time")
270-
}
267+
"DateTime"
268+
| "NaiveDateTime"
269+
| "DateTimeWithTimeZone"
270+
| "DateTimeUtc"
271+
| "DateTimeLocal"
272+
| "OffsetDateTime"
273+
| "PrimitiveDateTime" => string_with_format("date-time"),
271274
"NaiveDate" | "Date" => string_with_format("date"),
272275
"NaiveTime" | "Time" => string_with_format("time"),
273276
// Duration types

crates/vespera_macro/src/route_impl.rs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,10 @@ pub fn process_route_attribute(
139139
fn_name: item_fn.sig.ident.to_string(),
140140
method: route_args.method.as_ref().map(syn::Ident::to_string),
141141
custom_path: route_args.path.as_ref().map(syn::LitStr::value),
142-
error_status: route_args.error_status.as_ref().and_then(extract_error_status_codes),
142+
error_status: route_args
143+
.error_status
144+
.as_ref()
145+
.and_then(extract_error_status_codes),
143146
tags: route_args.tags.as_ref().and_then(extract_tag_strings),
144147
description: route_args.description.as_ref().map(syn::LitStr::value),
145148
fn_item_str: item.to_string(),
@@ -319,7 +322,13 @@ mod tests {
319322

320323
#[test]
321324
fn test_route_storage_populated_by_process_route_attribute() {
322-
let attr = quote!(get, path = "/{id}", tags = ["users"], description = "Get user by ID", error_status = [404]);
325+
let attr = quote!(
326+
get,
327+
path = "/{id}",
328+
tags = ["users"],
329+
description = "Get user by ID",
330+
error_status = [404]
331+
);
323332
let item = quote!(
324333
pub async fn get_user_test_storage() -> String {
325334
"test".to_string()
@@ -334,8 +343,13 @@ mod tests {
334343
.unwrap_or_else(std::sync::PoisonError::into_inner);
335344

336345
// Find our entry and verify fields
337-
let stored = storage.iter().find(|s| s.fn_name == "get_user_test_storage");
338-
assert!(stored.is_some(), "StoredRouteInfo should be in ROUTE_STORAGE");
346+
let stored = storage
347+
.iter()
348+
.find(|s| s.fn_name == "get_user_test_storage");
349+
assert!(
350+
stored.is_some(),
351+
"StoredRouteInfo should be in ROUTE_STORAGE"
352+
);
339353
let stored = stored.unwrap();
340354
assert_eq!(stored.method, Some("get".to_string()));
341355
assert_eq!(stored.custom_path, Some("/{id}".to_string()));
@@ -391,6 +405,13 @@ mod tests {
391405
#[test]
392406
fn test_extract_tag_strings_values() {
393407
let arr: syn::ExprArray = syn::parse_quote!(["users", "admin", "api"]);
394-
assert_eq!(extract_tag_strings(&arr), Some(vec!["users".to_string(), "admin".to_string(), "api".to_string()]));
408+
assert_eq!(
409+
extract_tag_strings(&arr),
410+
Some(vec![
411+
"users".to_string(),
412+
"admin".to_string(),
413+
"api".to_string()
414+
])
415+
);
395416
}
396417
}

crates/vespera_macro/src/router_codegen.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,9 @@ mod tests {
621621
let folder_name = "routes";
622622

623623
let result = generate_router_code(
624-
&collect_metadata(temp_dir.path(), folder_name, &[]).unwrap().0,
624+
&collect_metadata(temp_dir.path(), folder_name, &[])
625+
.unwrap()
626+
.0,
625627
None,
626628
None,
627629
None,
@@ -778,7 +780,9 @@ pub fn get_users() -> String {
778780
}
779781

780782
let result = generate_router_code(
781-
&collect_metadata(temp_dir.path(), folder_name, &[]).unwrap().0,
783+
&collect_metadata(temp_dir.path(), folder_name, &[])
784+
.unwrap()
785+
.0,
782786
None,
783787
None,
784788
None,
@@ -858,7 +862,9 @@ pub fn update_user() -> String {
858862
);
859863

860864
let result = generate_router_code(
861-
&collect_metadata(temp_dir.path(), folder_name, &[]).unwrap().0,
865+
&collect_metadata(temp_dir.path(), folder_name, &[])
866+
.unwrap()
867+
.0,
862868
None,
863869
None,
864870
None,
@@ -913,7 +919,9 @@ pub fn create_users() -> String {
913919
);
914920

915921
let result = generate_router_code(
916-
&collect_metadata(temp_dir.path(), folder_name, &[]).unwrap().0,
922+
&collect_metadata(temp_dir.path(), folder_name, &[])
923+
.unwrap()
924+
.0,
917925
None,
918926
None,
919927
None,
@@ -960,7 +968,9 @@ pub fn index() -> String {
960968
);
961969

962970
let result = generate_router_code(
963-
&collect_metadata(temp_dir.path(), folder_name, &[]).unwrap().0,
971+
&collect_metadata(temp_dir.path(), folder_name, &[])
972+
.unwrap()
973+
.0,
964974
None,
965975
None,
966976
None,
@@ -998,7 +1008,9 @@ pub fn get_users() -> String {
9981008
);
9991009

10001010
let result = generate_router_code(
1001-
&collect_metadata(temp_dir.path(), folder_name, &[]).unwrap().0,
1011+
&collect_metadata(temp_dir.path(), folder_name, &[])
1012+
.unwrap()
1013+
.0,
10021014
None,
10031015
None,
10041016
None,
@@ -1354,7 +1366,8 @@ pub fn get_users() -> String {
13541366
"#,
13551367
);
13561368

1357-
let (mut metadata, _file_asts) = collect_metadata(temp_dir.path(), folder_name, &[]).unwrap();
1369+
let (mut metadata, _file_asts) =
1370+
collect_metadata(temp_dir.path(), folder_name, &[]).unwrap();
13581371
// Inject an additional route with invalid method
13591372
metadata.routes.push(crate::metadata::RouteMetadata {
13601373
method: "CONNECT".to_string(),

crates/vespera_macro/src/schema_impl.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,17 +122,17 @@ fn extract_field_defaults(input: &syn::DeriveInput) -> BTreeMap<String, serde_js
122122
};
123123

124124
// Read and parse the file
125-
let Some(file_ast) =
126-
crate::file_utils::read_and_parse_file_warn(&file_path, "derive(Schema) default extraction")
127-
else {
125+
let Some(file_ast) = crate::file_utils::read_and_parse_file_warn(
126+
&file_path,
127+
"derive(Schema) default extraction",
128+
) else {
128129
return defaults;
129130
};
130131

131132
// Extract default values from functions
132133
for (field_name, fn_name) in fn_defaults {
133134
if let Some(func) = crate::openapi_generator::find_function_in_file(&file_ast, &fn_name)
134-
&& let Some(value) =
135-
crate::openapi_generator::extract_default_value_from_function(func)
135+
&& let Some(value) = crate::openapi_generator::extract_default_value_from_function(func)
136136
{
137137
defaults.insert(field_name, value);
138138
}

crates/vespera_macro/src/schema_macro/codegen.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ fn schema_type_to_tokens(st: &SchemaType) -> TokenStream {
130130
quote! { vespera::schema::SchemaType::#ident }
131131
}
132132

133-
134133
/// Convert `SchemaRef` to `TokenStream` for code generation
135134
pub fn schema_ref_to_tokens(schema_ref: &SchemaRef) -> TokenStream {
136135
match schema_ref {

crates/vespera_macro/src/schema_macro/file_cache.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,9 @@ fn ensure_struct_definitions(cache: &mut FileCache, path: &Path) -> bool {
184184
}
185185

186186
if let Some(mtime) = current_mtime {
187-
cache.struct_definitions.insert(path.to_path_buf(), (mtime, defs));
187+
cache
188+
.struct_definitions
189+
.insert(path.to_path_buf(), (mtime, defs));
188190
}
189191

190192
true
@@ -204,7 +206,12 @@ pub fn get_struct_definition(path: &Path, struct_name: &str) -> Option<String> {
204206
if !ensure_struct_definitions(&mut cache, path) {
205207
return None;
206208
}
207-
cache.struct_definitions.get(path)?.1.get(struct_name).cloned()
209+
cache
210+
.struct_definitions
211+
.get(path)?
212+
.1
213+
.get(struct_name)
214+
.cloned()
208215
})
209216
}
210217

@@ -454,7 +461,6 @@ mod tests {
454461
assert!(candidates[0].ends_with("has_model.rs"));
455462
}
456463

457-
458464
#[test]
459465
fn test_get_struct_candidates_caches_result() {
460466
let temp_dir = TempDir::new().unwrap();

0 commit comments

Comments
 (0)