[WIP] refactor/java provider and add java catalog cache#110
Open
BegoniaHe wants to merge 13 commits intoHydroRoll-Team:mainfrom
Open
[WIP] refactor/java provider and add java catalog cache#110BegoniaHe wants to merge 13 commits intoHydroRoll-Team:mainfrom
BegoniaHe wants to merge 13 commits intoHydroRoll-Team:mainfrom
Conversation
|
@BegoniaHe is attempting to deploy a commit to the retrofor Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Reviewer's GuideRefactors the Rust Java core module into smaller responsibilities, adds a durable cached catalog layer and atomic persistence, hardens the download queue and ZIP extraction, and improves the Adoptium provider with bounded concurrency, retries, and richer error reporting/typing exported to the React frontend. Class diagram for refactored Java core types and download resume resultclassDiagram
class JavaProvider {
<<trait>>
+fetch_catalog(app_handle: AppHandle, force_refresh: bool) Result~JavaCatalog, JavaError~
+fetch_release(major_version: u32, image_type: ImageType) Result~JavaDownloadInfo, JavaError~
+available_versions() Result~Vec~u32~, JavaError~
+install_prefix() &str
+id() &str
}
class AdoptiumProvider {
+new() AdoptiumProvider
+fetch_catalog(app_handle: AppHandle, force_refresh: bool) Result~JavaCatalog, JavaError~
+fetch_release(major_version: u32, image_type: ImageType) Result~JavaDownloadInfo, JavaError~
+available_versions() Result~Vec~u32~, JavaError~
+install_prefix() &str
+id() &str
}
class ImageType {
<<enum>>
Jre
Jdk
+parse(value: &str) Option~ImageType~
+to_string() String
}
class JavaInstallation {
+path: String
+version: String
+architecture: String
+is_64bit: bool
}
class JavaReleaseInfo {
+major_version: u32
+image_type: ImageType
+version: String
+release_name: String
+release_date: Option~String~
+file_size: u64
+checksum: Option~String~
+download_url: String
+is_lts: bool
+is_available: bool
+architecture: String
}
class JavaDownloadInfo {
+major_version: u32
+version: String
+download_url: String
+file_name: String
+file_size: u64
+checksum: Option~String~
+image_type: ImageType
}
class ResumeJavaDownloadFailureReason {
<<enum>>
Network
Cancelled
ChecksumMismatch
ExtractionFailed
VerificationFailed
Filesystem
InvalidArchive
Unknown
+from_error_message(error: &str) ResumeJavaDownloadFailureReason
}
class ResumeJavaDownloadFailure {
+major_version: u32
+image_type: String
+install_path: String
+reason: ResumeJavaDownloadFailureReason
+error: String
}
class ResumeJavaDownloadsResult {
+successful_installations: Vec~JavaInstallation~
+failed_downloads: Vec~ResumeJavaDownloadFailure~
+total_pending: usize
}
class PendingJavaDownload {
+major_version: u32
+image_type: String
+download_url: String
+file_name: String
+file_size: u64
+checksum: Option~String~
+install_path: String
+created_at: u64
}
class DownloadQueue {
+pending_downloads: Vec~PendingJavaDownload~
+add(download: PendingJavaDownload) void
+remove(major_version: u32, image_type: &str) void
+update(app_handle: AppHandle, mutator: F) Result~T, String~
+list_pending(app_handle: AppHandle) Vec~PendingJavaDownload~
+add_pending(app_handle: AppHandle, download: PendingJavaDownload) Result~(), String~
+remove_pending(app_handle: AppHandle, major_version: u32, image_type: &str) Result~(), String~
+load(app_handle: AppHandle) DownloadQueue
+save(app_handle: AppHandle) Result~(), String~
}
class JavaCatalog {
+cached_at: u64
+releases: Vec~JavaReleaseInfo~
}
class JavaError {
<<enum>>
NetworkError(String)
SerializationError(String)
InvalidConfig(String)
Other(String)
}
class InstallModule {
<<module>>
+download_and_install_java_with_provider(provider: JavaProvider, app_handle: AppHandle, major_version: u32, image_type: ImageType, custom_path: Option~PathBuf~) Result~JavaInstallation, String~
}
class CacheModule {
<<module>>
+load_cached_catalog_result(app_handle: AppHandle) Result~Option~JavaCatalog~, JavaError~
+load_cached_catalog(app_handle: AppHandle) Option~JavaCatalog~
+save_catalog_cache(app_handle: AppHandle, catalog: &JavaCatalog) Result~(), JavaError~
+clear_catalog_cache(app_handle: &AppHandle) Result~(), JavaError~
}
JavaProvider <|.. AdoptiumProvider
AdoptiumProvider --> JavaCatalog
AdoptiumProvider --> JavaReleaseInfo
AdoptiumProvider --> JavaDownloadInfo
JavaReleaseInfo --> ImageType
JavaDownloadInfo --> ImageType
ResumeJavaDownloadsResult --> JavaInstallation
ResumeJavaDownloadsResult --> ResumeJavaDownloadFailure
ResumeJavaDownloadFailure --> ResumeJavaDownloadFailureReason
DownloadQueue --> PendingJavaDownload
JavaCatalog --> JavaReleaseInfo
JavaError <.. CacheModule
JavaError <.. AdoptiumProvider
JavaProvider <.. InstallModule
JavaInstallation <.. InstallModule
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
downloader.rs, theDOWNLOAD_QUEUE_FILE_LOCKstatic is declared asstatic DOWNLOAD_QUEUE_FILE_LOCK: <()> = Mutex::new(());, which is invalid syntax and should bestatic DOWNLOAD_QUEUE_FILE_LOCK: Mutex<()> = Mutex::new(());to compile correctly. - The new
find_top_level_dirininstall.rsnow errors when there is not exactly one top-level directory (instead of returning an empty string as before), which changes behavior and may break extraction for archives that previously worked; consider either preserving the old fallback or explicitly validating that all existing archives are compatible with this stricter rule. - In
resume_pending_downloads_withyou useeprintln!for unknown image types; for consistency with the rest of the codebase and better log routing in Tauri, it would be preferable to use thelogcrate (e.g.,log::warn!) instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `downloader.rs`, the `DOWNLOAD_QUEUE_FILE_LOCK` static is declared as `static DOWNLOAD_QUEUE_FILE_LOCK: <()> = Mutex::new(());`, which is invalid syntax and should be `static DOWNLOAD_QUEUE_FILE_LOCK: Mutex<()> = Mutex::new(());` to compile correctly.
- The new `find_top_level_dir` in `install.rs` now errors when there is not exactly one top-level directory (instead of returning an empty string as before), which changes behavior and may break extraction for archives that previously worked; consider either preserving the old fallback or explicitly validating that all existing archives are compatible with this stricter rule.
- In `resume_pending_downloads_with` you use `eprintln!` for unknown image types; for consistency with the rest of the codebase and better log routing in Tauri, it would be preferable to use the `log` crate (e.g., `log::warn!`) instead.
## Individual Comments
### Comment 1
<location path="src-tauri/src/core/downloader.rs" line_range="13" />
<code_context>
use tokio::sync::Semaphore;
use ts_rs::TS;
+static DOWNLOAD_QUEUE_FILE_LOCK: <()> = Mutex::new(());
+
#[derive(Debug, Clone, Serialize, Deserialize, TS)]
</code_context>
<issue_to_address>
**issue (bug_risk):** The static DOWNLOAD_QUEUE_FILE_LOCK type annotation is invalid and will not compile.
This declaration should use a concrete type, e.g. `static DOWNLOAD_QUEUE_FILE_LOCK: Mutex<()> = Mutex::new(());`, so the module compiles correctly.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/core/downloader.rs" line_range="90" />
<code_context>
- /// Load download queue from file
- pub fn load(app_handle: &AppHandle) -> Self {
- let queue_path = app_handle
+ fn queue_path(app_handle: &AppHandle) -> Result<PathBuf, String> {
+ app_handle
.path()
</code_context>
<issue_to_address>
**issue (bug_risk):** queue_path uses map_err on an Option, which will not compile and also doesn’t handle the None case.
`app_handle.path().app_data_dir()` returns an `Option<PathBuf>`. You can convert it to a `Result` and make the failure explicit like this:
```rust
fn queue_path(app_handle: &AppHandle) -> Result<PathBuf, String> {
app_handle
.path()
.app_data_dir()
.map(|dir| dir.join("download_queue.json"))
.ok_or_else(|| "Failed to resolve app_data_dir for download queue".to_string())
}
```
This makes the error type explicit and avoids a panic when the path cannot be resolved.
</issue_to_address>
### Comment 3
<location path="src-tauri/src/utils/zip.rs" line_range="31" />
<code_context>
+ Ok(safe_path)
+}
+
pub fn extract_zip(zip_path: &Path, extract_to: &Path) -> Result<(), String> {
let file = fs::File::open(zip_path)
.map_err(|e| format!("Failed to open zip {}: {}", zip_path.display(), e))?;
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Path sanitization is applied to tar.gz entries but not to zip entries, leaving a potential directory traversal gap.
`sanitize_relative_path` now protects `extract_tar_gz` from `../` and absolute paths, but `extract_zip` still uses raw entry paths, so a crafted zip could write outside `extract_to`.
Please also sanitize zip entry paths (and skip empty results) before joining them with `extract_to` to close this gap.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Ok(safe_path) | ||
| } | ||
|
|
||
| pub fn extract_zip(zip_path: &Path, extract_to: &Path) -> Result<(), String> { |
Contributor
There was a problem hiding this comment.
🚨 suggestion (security): Path sanitization is applied to tar.gz entries but not to zip entries, leaving a potential directory traversal gap.
sanitize_relative_path now protects extract_tar_gz from ../ and absolute paths, but extract_zip still uses raw entry paths, so a crafted zip could write outside extract_to.
Please also sanitize zip entry paths (and skip empty results) before joining them with extract_to to close this gap.
Member
|
@BegoniaHe is this PR still in maintained? |
Contributor
Author
|
Yes @HsiangNianian |
add src-tauri/src/core/java/cache.rs for Java catalog caching reorganize provider interfaces and move detection/install changes update persistence, adoptium provider and validation Reviewed-by: Raptor mini (Preview)
- Update .github/copilot-instructions.md Reviewed-by: GitHub Copilot CLI
BegoniaHe
force-pushed
the
refactor/java-provider
branch
2 times, most recently
from
8afd383 to
dedc22d
Compare
33 tasks
HsiangNianian
added a commit
that referenced
this pull request
Mar 18, 2026
# fix(modpack): 将 CurseForge API Key 改为编译期可选常量 修复 `env!()` 宏在开发者本地无 `CURSEFORGE_API_KEY` 时导致编译失败的问题，改用 `option_env!()` + build.rs 中的 `dotenvy` 读取 .env 文件，实现编译期嵌入、缺失时优雅降级。 ## 更改类型 - [x] Bug 修复（修复问题的非破坏性更改） - [ ] 新功能（添加功能的非破坏性更改） - [ ] 破坏性更改（会导致现有功能无法正常工作的修复或功能） - [ ] 文档更新 - [ ] UI/UX 改进 - [ ] 性能优化 - [ ] 代码重构（无功能性更改） - [x] 配置更改 - [ ] 测试添加或更新 ## LLM 生成代码声明 - [x] 此 PR 不包含 LLM 生成的代码，我**提供**质量担保 ## 相关 Issue 相关 #110 #117 ## 更改内容 ### 后端 (Rust) - modpack.rs：将 `env!("CURSEFORGE_API_KEY")` 替换为 `const CURSEFORGE_API_KEY: Option<&str> = option_env!("CURSEFORGE_API_KEY")`，key 不存在时编译为 `None`，调用 CurseForge 功能时返回友好错误而非 panic - build.rs：添加 `dotenvy::dotenv()` 调用，允许通过 .env 文件在编译期注入 key，并注册 `cargo:rerun-if-changed` / `cargo:rerun-if-env-changed` 确保增量构建正确 ### 前端 (Svelte) - 无 ### 配置 - Cargo.toml：在 `[build-dependencies]` 中添加 `dotenvy = { version = "0.15", default-features = false }` - .gitignore：添加 .env / `.env.local` 忽略规则，防止 key 被意外提交 - .env.example：新增示例文件，说明可选配置项及获取方式 ## 测试 ### 测试环境 - **操作系统**：Fedora Linux 6.19.6-300.fc44.x86_64 x86_64 - **DropOut 版本**：0.2.0-alpha.5 - **测试的 Minecraft 版本**：N/A - **Mod 加载器**：N/A ### 测试用例 - [ ] 已在 Windows 上测试 - [ ] 已在 macOS 上测试 - [x] 已在 Linux 上测试 - [ ] 已测试原版 Minecraft - [ ] 已测试 Fabric - [ ] 已测试 Forge - [ ] 已测试游戏启动 - [ ] 已测试登录流程 - [ ] 已测试 Java 检测/下载 ### 测试步骤 1. 不设置 `CURSEFORGE_API_KEY`，不创建 .env 文件，直接执行 `cargo check` → 应编译通过（无报错） 2. 创建 .env 文件并写入 `CURSEFORGE_API_KEY=test_key`，执行 `cargo check` → 应编译通过，key 被嵌入二进制 3. 不含 key 的构建中触发 CurseForge modpack 导入 → 应返回友好错误提示而非 panic ## 检查清单 ### 代码质量 - [x] 我的代码遵循项目的代码风格指南 - [x] 我已对自己的代码进行了自审 - [ ] 我已对难以理解的区域添加了注释 - [x] 我的更改没有产生新的警告或错误 ### 测试验证 - [x] 我已在本地测试了我的更改 - [ ] 我已添加测试来证明我的修复有效或功能正常工作 - [x] 新的和现有的单元测试在本地通过 - [x] 我至少在一个目标平台上进行了测试 ### 文档更新 - [ ] 我已相应地更新了文档 - [ ] 如有需要，我已更新 README - [ ] 我已在必要处添加/更新代码注释 ### 依赖项 - [x] 我已检查没有添加不必要的依赖项 - [x] 所有新依赖项都已正确记录 - [x] Cargo.lock 已更新 ## 附加说明 `dotenvy` 仅作为 **build-dependency**，不会进入最终二进制。官方发布构建通过 CI 环境变量注入 key，普通开发者无需任何操作即可正常编译和运行。 Co-authored-by: 简律纯 <i@jyunko.cn>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
描述
对 Java 模块（
src-tauri/src/core/java/）进行系统性重构，将单体文件拆分为职责清晰的子模块，同时增强下载队列的线程安全性、错误处理能力和 Adoptium Provider 的并发请求能力。本 PR 不引入新的用户可见功能，专注于代码质量与可维护性提升。更改类型
LLM 生成代码声明
相关 Issue
关闭 #
相关 #
更改内容
后端 (Rust)
1. Java 检测模块拆分（
detection.rs→detection/子模块）将原来的单体
detection.rs（311 行）拆分为按平台职责分离的子模块：detection/common.rsPATH（which/where）与JAVA_HOME检测，带 2 秒超时保护防止挂起detection/linux.rs/usr/lib/jvm、/opt/java等）及 sdkman/mise 工具链detection/macos.rs/opt/homebrew/Cellar/openjdk）支持，sdkman/mise 工具链detection/windows.rsdetection/unix.rsdetection/mod.rs2. 新增 Java Catalog 缓存（
cache.rs）cache.rs，缓存 Adoptium Java Catalog 24 小时，避免每次启动均发起网络请求rename"的原子写入模式，防止缓存文件在写入中途崩溃时损坏load_cached_catalog_result()、save_catalog_cache()、clear_catalog_cache()公开 API3. 安装逻辑提取（
install.rs）mod.rs提取至独立的install.rs（179 行），降低mod.rs的复杂度4. 下载队列线程安全强化（
downloader.rs）Mutex（DOWNLOAD_QUEUE_FILE_LOCK）保护对download_queue.json的并发读写DownloadQueue::update(app_handle, mutator)— 持锁、加载、修改、原子写回的事务操作DownloadQueue::list_pending()— 线程安全读取待下载列表DownloadQueue::add_pending()/remove_pending()— 封装单次更新操作.json.tmp→rename）queue_path()改为返回Result，消除unwrap()panic 风险5. 持久化原子写入（
persistence.rs）save_java_config()改为通过临时文件原子写入，防止写入中断导致配置损坏load_java_config_result()返回Result<JavaConfig, JavaError>，提供更精确的错误传播路径6. 错误类型增强（
error.rs）ResumeJavaDownloadFailureReason枚举，将下载失败原因分类为：Network、Cancelled、ChecksumMismatch、ExtractionFailed、VerificationFailed、Filesystem、InvalidArchive、Unknownfrom_error_message(error: &str)从原始错误字符串中自动分类原因ts-rs导出类型到packages/ui-new/src/types/bindings/java/index.ts7. Adoptium Provider 并发优化（
providers/adoptium.rs）8. ZIP 提取改进（
utils/zip.rs）9.
Cargo.toml清理前端 (Svelte)
ts-rs自动生成到packages/ui-new）配置
测试
测试环境
测试用例
测试步骤
cargo test验证error.rs中的classify_resume_failure_reason_table_driven单元测试通过cargo tauri dev启动开发模式，进入设置页面验证 Java 自动检测功能正常download_queue.json）被正确原子写入java_catalog_cache.json在首次请求后被缓存，24 小时内复用缓存（可临时将超时改为 5 秒验证）检查清单
代码质量
测试验证
error.rs表驱动单元测试）文档更新
.github/copilot-instructions.md）Summary by Sourcery
Refactor the Java backend module to improve provider abstraction, caching, download resilience, and error reporting without changing user-visible behavior.
Enhancements:
Documentation:
Tests: