feat: 添加从 server.jar 提取版本 ID 的功能#434
Open
MimiRabbit87 wants to merge 4 commits intoSeaLantern-Studio:devfrom
Open
feat: 添加从 server.jar 提取版本 ID 的功能#434MimiRabbit87 wants to merge 4 commits intoSeaLantern-Studio:devfrom
MimiRabbit87 wants to merge 4 commits intoSeaLantern-Studio:devfrom
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Contributor
Reviewer's Guide添加了一个后端功能，通过读取 server.jar 中的 version.json 来提取 Minecraft 服务器版本 ID，并将其暴露为一个新的 Tauri 命令，另外还包括一些生成的 Vite 依赖元数据文件。 从 server.jar 提取服务器版本 ID 的时序图sequenceDiagram
actor Frontend
participant TauriCore
participant ServerCommands
participant ServerManager
participant FileSystem
participant ZipArchive
participant JSONParser
Frontend->>TauriCore: invoke get_server_version(jar_path)
TauriCore->>ServerCommands: get_server_version(jar_path)
ServerCommands->>ServerManager: get_server_version(&jar_path)
ServerManager->>FileSystem: File::open(jar_path)
FileSystem-->>ServerManager: Result<File, String>
ServerManager->>ZipArchive: ZipArchive::new(file)
ZipArchive-->>ServerManager: Result<ZipArchive, String>
ServerManager->>ZipArchive: by_name(version.json)
ZipArchive-->>ServerManager: Result<version_file, String>
ServerManager->>ServerManager: read_to_string(content)
ServerManager->>JSONParser: serde_json::from_str(content)
JSONParser-->>ServerManager: Result<Value, String>
ServerManager->>ServerManager: extract json["id"].as_str()
ServerManager-->>ServerCommands: Ok(id)
ServerCommands-->>TauriCore: Ok(id)
TauriCore-->>Frontend: Ok(id)
ServerManager 和新版本提取命令的类图classDiagram
class ServerManager {
+get_server_version(jar_path: &String) Result~String, String~
+delete_server(id: &str) Result~(), String~
+get_server_status(id: &str) ServerStatusInfo
}
class ServerCommands {
<<module>>
+get_server_version(jar_path: String) Result~String, String~
+get_server_status(id: String) ServerStatusInfo
+delete_server(id: String) Result~(), String~
}
class TauriLib {
+run() void
}
class ManagerProvider {
<<utility>>
+manager() ServerManager
}
ServerCommands --> ManagerProvider : uses
ManagerProvider --> ServerManager : returns
TauriLib --> ServerCommands : registers Tauri commands
TauriLib --> ManagerProvider : indirectly uses via commands
文件级变更
Tips and commands与 Sourcery 交互
自定义你的体验访问你的 dashboard 以：
获取帮助Original review guide in EnglishReviewer's GuideAdds a backend feature to extract the Minecraft server version ID by reading version.json inside server.jar and exposes it as a new Tauri command, plus some generated Vite dependency metadata files. Sequence diagram for extracting server version ID from server.jarsequenceDiagram
actor Frontend
participant TauriCore
participant ServerCommands
participant ServerManager
participant FileSystem
participant ZipArchive
participant JSONParser
Frontend->>TauriCore: invoke get_server_version(jar_path)
TauriCore->>ServerCommands: get_server_version(jar_path)
ServerCommands->>ServerManager: get_server_version(&jar_path)
ServerManager->>FileSystem: File::open(jar_path)
FileSystem-->>ServerManager: Result<File, String>
ServerManager->>ZipArchive: ZipArchive::new(file)
ZipArchive-->>ServerManager: Result<ZipArchive, String>
ServerManager->>ZipArchive: by_name(version.json)
ZipArchive-->>ServerManager: Result<version_file, String>
ServerManager->>ServerManager: read_to_string(content)
ServerManager->>JSONParser: serde_json::from_str(content)
JSONParser-->>ServerManager: Result<Value, String>
ServerManager->>ServerManager: extract json["id"].as_str()
ServerManager-->>ServerCommands: Ok(id)
ServerCommands-->>TauriCore: Ok(id)
TauriCore-->>Frontend: Ok(id)
Class diagram for ServerManager and new version extraction commandclassDiagram
class ServerManager {
+get_server_version(jar_path: &String) Result~String, String~
+delete_server(id: &str) Result~(), String~
+get_server_status(id: &str) ServerStatusInfo
}
class ServerCommands {
<<module>>
+get_server_version(jar_path: String) Result~String, String~
+get_server_status(id: String) ServerStatusInfo
+delete_server(id: String) Result~(), String~
}
class TauriLib {
+run() void
}
class ManagerProvider {
<<utility>>
+manager() ServerManager
}
ServerCommands --> ManagerProvider : uses
ManagerProvider --> ServerManager : returns
TauriLib --> ServerCommands : registers Tauri commands
TauriLib --> ManagerProvider : indirectly uses via commands
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 - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 建议将
get_server_version(&self, jar_path: &String)修改为接收&Path/&PathBuf，或者至少是&str，这样在使用上会更符合 Rust 习惯，也更好地与文件系统相关 API 保持一致。 - 当前在
get_server_version中使用的map_err(|e| e.to_string())错误处理链会丢失错误的上下文和类型信息；使用自定义错误类型，或至少为每个错误添加更具描述性的上下文，有助于更容易地诊断失败原因。 - 新增的
.vite/deps/package.json（以及.vite/deps下的_metadata.json）看起来像是构建产物，通常不应该提交到版本控制中。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- Consider changing `get_server_version(&self, jar_path: &String)` to take a `&Path`/`&PathBuf` or at least `&str` so usage is more idiomatic and better aligned with filesystem APIs.
- The current `map_err(|e| e.to_string())` chains in `get_server_version` lose error context and type information; using a custom error type or at least adding more descriptive context to each error would make failures easier to diagnose.
- The newly added `.vite/deps/package.json` (and `_metadata.json`) under `.vite/deps` looks like a build artifact and probably shouldn’t be checked into source control.
## Individual Comments
### Comment 1
<location path="src-tauri/src/services/server_manager.rs" line_range="1229-1238" />
<code_context>
}
+ /// 从 server.jar 中提取 version.json 的 id 字段
+ pub fn get_server_version(&self, jar_path: &String) -> Result<String, String> {
+ let file = File::open(jar_path).map_err(|e| e.to_string())?;
+ let mut archive = ZipArchive::new(file).map_err(|e| e.to_string())?;
+ let mut version_file = archive.by_name("version.json").map_err(|e| e.to_string())?;
+ let mut content = String::new();
+ version_file
+ .read_to_string(&mut content)
+ .map_err(|e| e.to_string())?;
+ let json: serde_json::Value = serde_json::from_str(&content).map_err(|e| e.to_string())?;
+ let id = json["id"]
+ .as_str()
+ .ok_or_else(|| "Missing or invalid 'id' field in version.json".to_string())?
</code_context>
<issue_to_address>
**suggestion:** Use more ergonomic argument type and avoid repeated `to_string()` conversions in error handling.
1. Prefer `jar_path: &str` for a read-only path to keep the API more flexible and decoupled from `String`.
2. Instead of converting every error to `String` with `map_err(|e| e.to_string())?`, use a richer error type (e.g. a custom error or `anyhow::Error`) and add context where needed (e.g. `format!("failed to open {jar_path}: {e}")`), or let `?` propagate the original error so you don’t lose structured information.
Suggested implementation:
```rust
use std::fs::File;
use std::io::Read;
use std::sync::{Arc, Mutex};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use zip::ZipArchive;
use anyhow::{anyhow, Context};
use crate::models::server::*;
use serde::{Deserialize, Serialize};
use serde_json::Value;
```
```rust
/// 从 server.jar 中提取 version.json 的 id 字段
pub fn get_server_version(&self, jar_path: &str) -> anyhow::Result<String> {
let file = File::open(jar_path)
.with_context(|| format!("failed to open server jar at path: {jar_path}"))?;
let mut archive =
ZipArchive::new(file).context("failed to read server jar as ZIP archive")?;
let mut version_file = archive
.by_name("version.json")
.context("version.json not found in server jar")?;
let mut content = String::new();
version_file
.read_to_string(&mut content)
.context("failed to read version.json from server jar")?;
let json: Value =
serde_json::from_str(&content).context("failed to parse version.json as JSON")?;
let id = json
.get("id")
.and_then(|v| v.as_str())
.ok_or_else(|| anyhow!("missing or invalid 'id' field in version.json"))?;
Ok(id.to_owned())
}
```
1. All call sites of `get_server_version` must be updated from `get_server_version(&some_string)` to `get_server_version(some_string)` or `get_server_version(some_str)` to match the new `&str` parameter type.
2. Callers must handle the new `anyhow::Result<String>` return type:
- Either propagate it with `?` from functions that also return `anyhow::Result<_>`, or
- Convert it back to `Result<_, String>` where needed, e.g. `get_server_version(...).map_err(|e| e.to_string())`.
3. Ensure `anyhow` is included in `Cargo.toml` (e.g. `anyhow = "1"`). If your project has a centralized error type, you may instead want to wrap or convert the `anyhow::Error` into that type at the call sites.
</issue_to_address>请帮我变得更有用！你可以在每条评论上点 👍 或 👎，我会根据这些反馈来改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Consider changing
get_server_version(&self, jar_path: &String)to take a&Path/&PathBufor at least&strso usage is more idiomatic and better aligned with filesystem APIs. - The current
map_err(|e| e.to_string())chains inget_server_versionlose error context and type information; using a custom error type or at least adding more descriptive context to each error would make failures easier to diagnose. - The newly added
.vite/deps/package.json(and_metadata.json) under.vite/depslooks like a build artifact and probably shouldn’t be checked into source control.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider changing `get_server_version(&self, jar_path: &String)` to take a `&Path`/`&PathBuf` or at least `&str` so usage is more idiomatic and better aligned with filesystem APIs.
- The current `map_err(|e| e.to_string())` chains in `get_server_version` lose error context and type information; using a custom error type or at least adding more descriptive context to each error would make failures easier to diagnose.
- The newly added `.vite/deps/package.json` (and `_metadata.json`) under `.vite/deps` looks like a build artifact and probably shouldn’t be checked into source control.
## Individual Comments
### Comment 1
<location path="src-tauri/src/services/server_manager.rs" line_range="1229-1238" />
<code_context>
}
+ /// 从 server.jar 中提取 version.json 的 id 字段
+ pub fn get_server_version(&self, jar_path: &String) -> Result<String, String> {
+ let file = File::open(jar_path).map_err(|e| e.to_string())?;
+ let mut archive = ZipArchive::new(file).map_err(|e| e.to_string())?;
+ let mut version_file = archive.by_name("version.json").map_err(|e| e.to_string())?;
+ let mut content = String::new();
+ version_file
+ .read_to_string(&mut content)
+ .map_err(|e| e.to_string())?;
+ let json: serde_json::Value = serde_json::from_str(&content).map_err(|e| e.to_string())?;
+ let id = json["id"]
+ .as_str()
+ .ok_or_else(|| "Missing or invalid 'id' field in version.json".to_string())?
</code_context>
<issue_to_address>
**suggestion:** Use more ergonomic argument type and avoid repeated `to_string()` conversions in error handling.
1. Prefer `jar_path: &str` for a read-only path to keep the API more flexible and decoupled from `String`.
2. Instead of converting every error to `String` with `map_err(|e| e.to_string())?`, use a richer error type (e.g. a custom error or `anyhow::Error`) and add context where needed (e.g. `format!("failed to open {jar_path}: {e}")`), or let `?` propagate the original error so you don’t lose structured information.
Suggested implementation:
```rust
use std::fs::File;
use std::io::Read;
use std::sync::{Arc, Mutex};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use zip::ZipArchive;
use anyhow::{anyhow, Context};
use crate::models::server::*;
use serde::{Deserialize, Serialize};
use serde_json::Value;
```
```rust
/// 从 server.jar 中提取 version.json 的 id 字段
pub fn get_server_version(&self, jar_path: &str) -> anyhow::Result<String> {
let file = File::open(jar_path)
.with_context(|| format!("failed to open server jar at path: {jar_path}"))?;
let mut archive =
ZipArchive::new(file).context("failed to read server jar as ZIP archive")?;
let mut version_file = archive
.by_name("version.json")
.context("version.json not found in server jar")?;
let mut content = String::new();
version_file
.read_to_string(&mut content)
.context("failed to read version.json from server jar")?;
let json: Value =
serde_json::from_str(&content).context("failed to parse version.json as JSON")?;
let id = json
.get("id")
.and_then(|v| v.as_str())
.ok_or_else(|| anyhow!("missing or invalid 'id' field in version.json"))?;
Ok(id.to_owned())
}
```
1. All call sites of `get_server_version` must be updated from `get_server_version(&some_string)` to `get_server_version(some_string)` or `get_server_version(some_str)` to match the new `&str` parameter type.
2. Callers must handle the new `anyhow::Result<String>` return type:
- Either propagate it with `?` from functions that also return `anyhow::Result<_>`, or
- Convert it back to `Result<_, String>` where needed, e.g. `get_server_version(...).map_err(|e| e.to_string())`.
3. Ensure `anyhow` is included in `Cargo.toml` (e.g. `anyhow = "1"`). If your project has a centralized error type, you may instead want to wrap or convert the `anyhow::Error` into that type at the call sites.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
1229
to
1238
| pub fn get_server_version(&self, jar_path: &String) -> Result<String, String> { | ||
| let file = File::open(jar_path).map_err(|e| e.to_string())?; | ||
| let mut archive = ZipArchive::new(file).map_err(|e| e.to_string())?; | ||
| let mut version_file = archive.by_name("version.json").map_err(|e| e.to_string())?; | ||
| let mut content = String::new(); | ||
| version_file | ||
| .read_to_string(&mut content) | ||
| .map_err(|e| e.to_string())?; | ||
| let json: serde_json::Value = serde_json::from_str(&content).map_err(|e| e.to_string())?; | ||
| let id = json["id"] |
Contributor
There was a problem hiding this comment.
suggestion: 使用更符合习惯的参数类型，并避免在错误处理时重复调用 to_string()。
-
对于只读路径，更推荐使用
jar_path: &str，以便让 API 更加灵活，也不要和String强耦合。 -
与其在每个错误分支中通过
map_err(|e| e.to_string())?把错误都转换成String，不如使用更丰富的错误类型（例如自定义错误或anyhow::Error），并在需要时添加上下文信息（例如format!("failed to open {jar_path}: {e}")）；或者让?直接向上传播原始错误，这样就不会丢失结构化的错误信息。
推荐实现方式：
use std::fs::File;
use std::io::Read;
use std::sync::{Arc, Mutex};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use zip::ZipArchive;
use anyhow::{anyhow, Context};
use crate::models::server::*;
use serde::{Deserialize, Serialize};
use serde_json::Value; /// 从 server.jar 中提取 version.json 的 id 字段
pub fn get_server_version(&self, jar_path: &str) -> anyhow::Result<String> {
let file = File::open(jar_path)
.with_context(|| format!("failed to open server jar at path: {jar_path}"))?;
let mut archive =
ZipArchive::new(file).context("failed to read server jar as ZIP archive")?;
let mut version_file = archive
.by_name("version.json")
.context("version.json not found in server jar")?;
let mut content = String::new();
version_file
.read_to_string(&mut content)
.context("failed to read version.json from server jar")?;
let json: Value =
serde_json::from_str(&content).context("failed to parse version.json as JSON")?;
let id = json
.get("id")
.and_then(|v| v.as_str())
.ok_or_else(|| anyhow!("missing or invalid 'id' field in version.json"))?;
Ok(id.to_owned())
}- 所有调用
get_server_version的地方，都需要从get_server_version(&some_string)更新为get_server_version(some_string)或get_server_version(some_str)，以匹配新的&str参数类型。 - 调用方需要处理新的返回类型
anyhow::Result<String>：- 要么在同样返回
anyhow::Result<_>的函数中使用?继续向外传播； - 要么在需要的地方把它转换回
Result<_, String>，例如使用get_server_version(...).map_err(|e| e.to_string())。
- 要么在同样返回
- 确保在
Cargo.toml中已经引入anyhow（例如anyhow = "1"）。如果你的项目有统一的错误类型，也可以在调用端把anyhow::Error包装或转换为该错误类型。
Original comment in English
suggestion: Use more ergonomic argument type and avoid repeated to_string() conversions in error handling.
-
Prefer
jar_path: &strfor a read-only path to keep the API more flexible and decoupled fromString. -
Instead of converting every error to
Stringwithmap_err(|e| e.to_string())?, use a richer error type (e.g. a custom error oranyhow::Error) and add context where needed (e.g.format!("failed to open {jar_path}: {e}")), or let?propagate the original error so you don’t lose structured information.
Suggested implementation:
use std::fs::File;
use std::io::Read;
use std::sync::{Arc, Mutex};
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use zip::ZipArchive;
use anyhow::{anyhow, Context};
use crate::models::server::*;
use serde::{Deserialize, Serialize};
use serde_json::Value; /// 从 server.jar 中提取 version.json 的 id 字段
pub fn get_server_version(&self, jar_path: &str) -> anyhow::Result<String> {
let file = File::open(jar_path)
.with_context(|| format!("failed to open server jar at path: {jar_path}"))?;
let mut archive =
ZipArchive::new(file).context("failed to read server jar as ZIP archive")?;
let mut version_file = archive
.by_name("version.json")
.context("version.json not found in server jar")?;
let mut content = String::new();
version_file
.read_to_string(&mut content)
.context("failed to read version.json from server jar")?;
let json: Value =
serde_json::from_str(&content).context("failed to parse version.json as JSON")?;
let id = json
.get("id")
.and_then(|v| v.as_str())
.ok_or_else(|| anyhow!("missing or invalid 'id' field in version.json"))?;
Ok(id.to_owned())
}- All call sites of
get_server_versionmust be updated fromget_server_version(&some_string)toget_server_version(some_string)orget_server_version(some_str)to match the new&strparameter type. - Callers must handle the new
anyhow::Result<String>return type:- Either propagate it with
?from functions that also returnanyhow::Result<_>, or - Convert it back to
Result<_, String>where needed, e.g.get_server_version(...).map_err(|e| e.to_string()).
- Either propagate it with
- Ensure
anyhowis included inCargo.toml(e.g.anyhow = "1"). If your project has a centralized error type, you may instead want to wrap or convert theanyhow::Errorinto that type at the call sites.
Collaborator
|
功能没问题。位置有问题，目前创建服务器时会解包找服务端类型，你做成独立函数会导致多次重复的解包行为，应当做到一起。 请解决以上问题或者close。 |
Author
好的，请问是加到 server_installer.rs 里吗，我看 detect_core_type_with_main_class() 那边把路径传过去拿了主类，然后继续判断服务器的核心种类。我在那里新增一个 JAR 包的解包管理，让 read_jar_main_class() 从解包管理那边拿 ZipAchieve，然后在 把 server_manager.rs 那边的实现搬到 server_installer.rs 来，与 detect_mc_version_from_mods() 的逻辑合并，接着从 server.rs scan_startup_candidates_blocking() 根据模组判断版本号那边调用，这样可以吗？ 以及 Issue #426 那边似乎是 accepted 了呀？ |
Author
或者可以在 ParsedServerCoreInfo 结构体中增加 version_id 字段，把逻辑塞进 read_jar_main_class() 里 |
Collaborator
|
对的，这样写可以的👍。 426我以为你只说了多版本命令。 |
Collaborator
|
目前的这个地方是条件显示，你改成固定显示好了。把结果放进去。取id的方式比统计模组名好一点，优先按取id的结果显示mc版本吧 
|
- 在 ParsedServerCoreInfo 中添加 version_id 字段 - 重构 read_jar_main_class 为 read_jar_metadata，同时读取 version.json - 更新 detect_core_type_with_metadata 返回包含 id 的完整信息 - 在 parse_server_core_type 中集成新逻辑 - 优化错误处理
MimiRabbit87
force-pushed
the
feat/extract-version-id-new
branch
from
a998fd1 to
3f5bccb
Compare
- 在 scan_startup_candidates_blocking 中优先使用 parsed.version_id 作为 detected_mc_version；若 JAR 中无 version.json，则回退到原有的 mods 目录检测 - 更新前端类型 ParsedServerCoreInfo 和 API 映射以传递 version_id
- 在 scan_startup_candidates_blocking 中优先使用 parsed.version_id 作为 detected_mc_version；若 JAR 中无 version.json，则回退到原有的 mods 目录检测 - 更新前端类型 ParsedServerCoreInfo 和 API 映射以传递 version_id
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
5 participants
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.
提交前检查清单
提交前测试必读！！！.md并完成要求测试变更分类（必选其一）
feat新功能fixBug 修复docs文档/模板style代码格式（不影响功能）refactor重构（既不修复 bug 也不添加功能）perf性能优化test测试相关chore构建/CI/依赖/工具链revert回滚security安全修复影响范围（可多选）
前端 Frontend
后端 Backend
基础设施 Infrastructure
导入规范检查： 使用别名导入，避免相对路径
../变更详情
摘要
加了个功能获取 server.jar 里的 version.json .id
动机/背景
#426 中提到的自动检测服务器版本
具体改动
server_manager.rs 里面加了个 get_server_version()，可以获取 server.jar 里的 version.json .id
界面变动（如适用）
没有
关联 Issue
Addresses #426
自动化审查说明
无