Skip to content

Commit 17d43b8

Browse files
committed
Optimize java
1 parent f67b4df commit 17d43b8

17 files changed

Lines changed: 389 additions & 120 deletions

File tree

crates/vespera_core/src/openapi.rs

Lines changed: 146 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,63 @@ fn has_any_component_map(components: &Components) -> bool {
168168
|| components.security_schemes.is_some()
169169
}
170170

171+
/// Merge `other`'s per-method operations into `into` with **self-wins**
172+
/// semantics: an operation (or path-level field) already present on `into`
173+
/// is kept; a slot empty on `into` is filled from `other`.
174+
///
175+
/// Applied on a path-key conflict so two apps that define the same path
176+
/// under different methods both keep their operations, instead of the
177+
/// incoming [`PathItem`] being dropped whole. Destructuring `other` keeps
178+
/// this exhaustive — adding a `PathItem` field forces this to be updated.
179+
fn merge_path_item(into: &mut PathItem, other: PathItem) {
180+
let PathItem {
181+
get,
182+
post,
183+
put,
184+
patch,
185+
delete,
186+
head,
187+
options,
188+
trace,
189+
parameters,
190+
summary,
191+
description,
192+
} = other;
193+
if into.get.is_none() {
194+
into.get = get;
195+
}
196+
if into.post.is_none() {
197+
into.post = post;
198+
}
199+
if into.put.is_none() {
200+
into.put = put;
201+
}
202+
if into.patch.is_none() {
203+
into.patch = patch;
204+
}
205+
if into.delete.is_none() {
206+
into.delete = delete;
207+
}
208+
if into.head.is_none() {
209+
into.head = head;
210+
}
211+
if into.options.is_none() {
212+
into.options = options;
213+
}
214+
if into.trace.is_none() {
215+
into.trace = trace;
216+
}
217+
if into.parameters.is_none() {
218+
into.parameters = parameters;
219+
}
220+
if into.summary.is_none() {
221+
into.summary = summary;
222+
}
223+
if into.description.is_none() {
224+
into.description = description;
225+
}
226+
}
227+
171228
impl OpenApi {
172229
/// Merge another `OpenAPI` document into this one.
173230
///
@@ -177,9 +234,21 @@ impl OpenApi {
177234
/// and `external_docs` are adopted from `other` only when `self` has
178235
/// not set its own. On any key/field conflict, `self` takes precedence.
179236
pub fn merge(&mut self, other: Self) {
180-
// Merge paths (self takes precedence on conflict)
237+
// Merge paths. On a path-key conflict, merge per HTTP method
238+
// (self-wins per operation) instead of dropping the incoming
239+
// `PathItem` wholesale: two merged apps that both define the same
240+
// path under DIFFERENT methods (parent `GET /users`, child
241+
// `POST /users`) must keep BOTH operations in the generated
242+
// document — otherwise the spec under-documents what the merged
243+
// router actually serves at runtime.
181244
for (path, item) in other.paths {
182-
self.paths.entry(path).or_insert(item);
245+
use std::collections::btree_map::Entry;
246+
match self.paths.entry(path) {
247+
Entry::Vacant(slot) => {
248+
slot.insert(item);
249+
}
250+
Entry::Occupied(mut slot) => merge_path_item(slot.get_mut(), item),
251+
}
183252
}
184253

185254
// Merge components (every reusable component kind, self-wins on
@@ -314,6 +383,81 @@ mod tests {
314383
);
315384
}
316385

386+
fn create_post_path_item(summary: &str) -> PathItem {
387+
PathItem {
388+
post: Some(Operation {
389+
summary: Some(summary.to_string()),
390+
description: None,
391+
operation_id: None,
392+
tags: None,
393+
parameters: None,
394+
request_body: None,
395+
responses: BTreeMap::new(),
396+
security: None,
397+
deprecated: None,
398+
}),
399+
..Default::default()
400+
}
401+
}
402+
403+
#[test]
404+
fn test_merge_same_path_different_methods_are_combined() {
405+
// Regression: a path-key conflict must merge per HTTP method, not
406+
// drop the incoming PathItem wholesale. Parent defines GET /users,
407+
// child defines POST /users — the merged document must expose BOTH
408+
// operations (otherwise the spec under-documents the merged router).
409+
let mut base = create_base_openapi();
410+
base.paths
411+
.insert("/users".to_string(), create_path_item("List users")); // GET
412+
413+
let mut other = create_base_openapi();
414+
other
415+
.paths
416+
.insert("/users".to_string(), create_post_path_item("Create user")); // POST
417+
418+
base.merge(other);
419+
420+
let users = base.paths.get("/users").expect("/users present");
421+
// self-wins GET is preserved
422+
assert_eq!(
423+
users.get.as_ref().unwrap().summary,
424+
Some("List users".to_string())
425+
);
426+
// incoming POST is merged in (previously dropped on the whole-item
427+
// `or_insert`)
428+
assert_eq!(
429+
users.post.as_ref().unwrap().summary,
430+
Some("Create user".to_string())
431+
);
432+
}
433+
434+
#[test]
435+
fn test_merge_same_path_same_method_self_wins() {
436+
// Same path AND same method on both sides: self's operation is kept,
437+
// the incoming one is discarded.
438+
let mut base = create_base_openapi();
439+
base.paths
440+
.insert("/users".to_string(), create_path_item("Base get"));
441+
442+
let mut other = create_base_openapi();
443+
other
444+
.paths
445+
.insert("/users".to_string(), create_path_item("Other get"));
446+
447+
base.merge(other);
448+
449+
assert_eq!(
450+
base.paths
451+
.get("/users")
452+
.unwrap()
453+
.get
454+
.as_ref()
455+
.unwrap()
456+
.summary,
457+
Some("Base get".to_string())
458+
);
459+
}
460+
317461
#[test]
318462
fn test_merge_schemas() {
319463
let mut base = create_base_openapi();

crates/vespera_jni/src/streaming_closures.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,10 +451,16 @@ pub fn close_input_stream(
451451
env: &mut jni::Env<'_>,
452452
stream: &Global<JObject<'static>>,
453453
) -> jni::errors::Result<()> {
454-
env.call_method(stream, jni_str!("close"), jni_sig!("()V"), &[])?;
454+
let result = env.call_method(stream, jni_str!("close"), jni_sig!("()V"), &[]);
455+
// Scrub a pending exception (e.g. an `IOException` from closing an
456+
// already-broken stream) on BOTH success and failure — capturing the
457+
// result and clearing BEFORE `?` so a throwing `close()` still leaves the
458+
// thread clean, matching `complete_future{,_local}`'s self-contained
459+
// contract (the prior `?`-before-clear returned early on a throw).
455460
if env.exception_check() {
456461
env.exception_clear();
457462
}
463+
result?;
458464
Ok(())
459465
}
460466

crates/vespera_macro/src/openapi_generator/paths.rs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::{
2626
route_impl::StoredRouteInfo,
2727
};
2828

29-
type FnIndex<'a> = HashMap<&'a str, HashMap<String, &'a syn::ItemFn>>;
29+
type FnIndex<'a> = HashMap<String, HashMap<String, &'a syn::ItemFn>>;
3030
type StorageFnSigs<'a> = HashMap<(Option<String>, &'a str), Option<&'a str>>;
3131

3232
/// Build path items and collect tags from route metadata.
@@ -44,12 +44,23 @@ pub(super) fn build_path_items(
4444
let mut paths = BTreeMap::new();
4545
let mut all_tags = BTreeSet::new();
4646

47+
// Compute once: `cwd` anchors every path normalization below so the
48+
// three path sources — `file_cache` keys (collector), route metadata
49+
// spans, and ROUTE_STORAGE `#[route]` spans — compare in one canonical
50+
// space (separator/relativity/case can differ, especially on Windows).
51+
let cwd = std::env::current_dir().unwrap_or_default();
52+
4753
// Build the file-AST function index FIRST so the storage path
4854
// below can skip any function whose AST is already reachable through
4955
// `file_cache`. `collector::collect_metadata` has already walked
5056
// these files via `syn::parse_file`, so re-parsing `fn_sig_str`
5157
// from ROUTE_STORAGE for the same function is pure duplicated work.
52-
let fn_index: HashMap<&str, HashMap<String, &syn::ItemFn>> = file_cache
58+
//
59+
// Keyed by the NORMALIZED path so the `already_in_ast` storage check
60+
// and the main-loop AST lookup match regardless of path format — a raw
61+
// key misses when the `#[route]` span path differs from the collector's
62+
// `file_cache` key, needlessly re-parsing the signature on a worker.
63+
let fn_index: FnIndex<'_> = file_cache
5364
.iter()
5465
.map(|(path, ast)| {
5566
let fns: HashMap<String, &syn::ItemFn> = ast
@@ -63,7 +74,7 @@ pub(super) fn build_path_items(
6374
}
6475
})
6576
.collect();
66-
(path.as_str(), fns)
77+
(normalize_path_key(path, &cwd), fns)
6778
})
6879
.collect();
6980

@@ -73,7 +84,6 @@ pub(super) fn build_path_items(
7384
// `syn::parse_str` + operation build runs on worker threads below;
7485
// `syn` ASTs are not `Send`, which is also why fn_index-backed
7586
// routes stay on this thread.
76-
let cwd = std::env::current_dir().unwrap_or_default();
7787
let storage_fn_sigs = build_storage_fn_sigs(route_storage, &fn_index, &cwd);
7888

7989
// Split routes by signature source. `idx` preserves the original
@@ -96,7 +106,7 @@ pub(super) fn build_path_items(
96106
.or_else(|| storage_fn_sigs.get(&legacy_storage_key).copied().flatten())
97107
{
98108
parallel_jobs.push((idx, route_meta, fn_sig_str));
99-
} else if let Some(fns) = fn_index.get(route_meta.file_path.as_str())
109+
} else if let Some(fns) = fn_index.get(&normalize_path_key(&route_meta.file_path, &cwd))
100110
&& let Some(fn_item) = fns.get(&route_meta.function_name)
101111
{
102112
ast_jobs.push((idx, route_meta, &fn_item.sig));
@@ -179,7 +189,8 @@ fn build_storage_fn_sigs<'a>(
179189
let already_in_ast = s
180190
.file_path
181191
.as_deref()
182-
.and_then(|fp| fn_index.get(fp))
192+
.map(|fp| normalize_path_key(fp, cwd))
193+
.and_then(|fp| fn_index.get(&fp))
183194
.is_some_and(|fns| fns.contains_key(&s.fn_name));
184195
if already_in_ast {
185196
continue;

crates/vespera_macro/src/schema_macro/file_lookup/lookup.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,19 @@ pub fn collect_rs_files_recursive(dir: &Path, files: &mut Vec<std::path::PathBuf
260260
};
261261

262262
for entry in entries.flatten() {
263+
// `entry.file_type()` reads the kind from the directory-entry data the
264+
// OS already returned for this `read_dir` walk — no extra `metadata`
265+
// stat per entry, unlike `path.is_dir()`. Mirrors the established
266+
// `file_utils::collect_with_mtimes_into` pattern (symlinks, which are
267+
// neither file nor dir here, are skipped — never present in a `src/`
268+
// tree this indexes).
269+
let Ok(file_type) = entry.file_type() else {
270+
continue;
271+
};
263272
let path = entry.path();
264-
if path.is_dir() {
273+
if file_type.is_dir() {
265274
collect_rs_files_recursive(&path, files);
266-
} else if path.extension().is_some_and(|ext| ext == "rs") {
275+
} else if file_type.is_file() && path.extension().is_some_and(|ext| ext == "rs") {
267276
files.push(path);
268277
}
269278
}

0 commit comments

Comments
 (0)