fix(offline): expose native 115 tools for ED2K downloads - #2920
Conversation
- Treat 115 Cloud and 115 Open as ED2K-capable tools. - Keep automatic fallback limited to Thunder tools. - Add regression coverage for supported and unsupported tools. Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
- Allow ED2K requests initially using SimpleHttp to enter tool routing. - Prefer the matching 115 tool for 115 Cloud and 115 Open destinations. - Add regression coverage for native storage tool selection. Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
- Include the native destination tool in the path-aware tool list. - Keep existing ready-tool filtering for external destinations. - Add coverage for native storage to tool mapping. Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
PIKACHUIM
left a comment
PIKACHUIM
left a comment
There was a problem hiding this comment.
🙏 感谢贡献
感谢 @AmPlace 提交此PR！我已完成代码评审，以下是评审结果。
🤖 AI 自动审核声明
本评审报告由 AI 自动生成，当前使用 Claude Opus 5 模型进行分析。
⚠️ AI 分析结果仅供参考，可能存在误判或遗漏。如您发现任何问题或有不同意见，欢迎随时提出讨论和纠正。
⚠️ 重要提醒：即使 AI 评审认为代码质量良好且建议合并，最终是否合并仍需由项目维护者进行人工判定。项目维护者会综合考虑代码质量、项目规划、技术方向、团队资源等多方面因素做出决策。
📖 PR背景与需求
PR标题：fix(offline): expose native 115 tools for ED2K downloads
关联Issue：#2891
需求说明：
用户在使用 115 Cloud 和 115 Open 存储驱动时，尝试提交 ED2K 链接进行离线下载，系统提示协议不支持（"Ed2k protocol is not supported by 115 Cloud/Open. Please configure and use Thunder/ThunderX/ThunderBrowser for ed2k links"）。然而，115 网盘本身是原生支持 ED2K 下载的，不应该强制要求用户使用迅雷系工具。
预期目标：
- 当目标存储为 115 Cloud 或 115 Open 时，允许 ED2K 链接直接使用原生 115 工具进行下载
- 修复工具选择逻辑，让 SimpleHttp 不拦截 ED2K 链接，使其能进入正确的工具路由流程
- 前端能够获取并展示目标存储的原生下载工具
- 保持向后兼容：对于不支持 ED2K 的存储，仍然回退到迅雷系工具
📋 问题摘要
- ✅ 功能实现完整：核心逻辑清晰，成功解决了 issue 描述的问题
- ✅ 代码质量良好：新增了完整的单元测试覆盖，测试用例设计合理
- 💡 改进建议：3个小优化建议，主要涉及错误信息、API设计和边界情况处理
📂 逐文件分析
internal/offline_download/tool/add.go
改动意图：
这是核心修改文件，目的是修复 ED2K 下载的工具选择逻辑，使 115 Cloud/Open 能够使用原生 ED2K 下载能力。
代码修改逻辑：
-
新增 driver 包导入：引入
internal/driver包，用于获取存储驱动类型信息 -
修复 SimpleHttp 拦截问题（第73行）：
- 原逻辑：
if args.Tool == "SimpleHttp"会直接处理所有 SimpleHttp 请求 - 新逻辑：
if args.Tool == "SimpleHttp" && !isEd2kURL(args.URL)添加了 ED2K 检查 - 原理：让 ED2K 链接跳过 SimpleHttp 的早期返回，继续进入后续的工具路由逻辑
- 原逻辑：
-
智能工具路由（第87-96行）：
- 首先检查目标存储是否原生支持 ED2K（通过
ed2kToolForStorage函数） - 如果存储原生支持（如 115 Cloud/Open），优先使用存储自带工具
- 否则，回退到查找迅雷系工具的原有逻辑
- 这是一个优雅的渐进式降级策略
- 首先检查目标存储是否原生支持 ED2K（通过
-
新增
ed2kToolForStorage函数（第225-231行）：- 根据存储驱动类型返回对应的工具名称
- 当前仅支持 115 Cloud 和 115 Open
- 其他存储返回空字符串，表示不支持
-
新增
toolNameForStorage函数（第233-258行）：- 通过 Go 类型断言将存储驱动映射到工具名称
- 覆盖了项目中所有主要的存储驱动类型
- 这是一个通用的存储-工具映射函数，为未来扩展其他存储的原生下载能力奠定了基础
-
更新工具列表（第259-261行）：
ed2kCapableTools添加了 "115 Cloud" 和 "115 Open"- 新增
ed2kFallbackTools列表，仅包含迅雷系工具 - 原有的
findEd2kCapableTool现在使用ed2kFallbackTools，避免自动回退到 115 工具
合理性评估：
✅ 优点：
- 逻辑清晰，分层合理：存储类型识别、工具选择、回退策略各司其职
- 扩展性强：
toolNameForStorage函数已经为 9 种存储类型做了映射，方便未来扩展 - 向后兼容：保持了原有的迅雷系工具回退逻辑，不影响现有用户
- 代码风格一致：函数命名、注释语言（中英混合）与项目现有风格一致
💡 建议：
-
错误信息改进：第96行的错误信息 "Please configure and use Thunder/ThunderX/ThunderBrowser for ed2k links" 可能需要更新，因为现在 115 工具也支持 ED2K，建议改为更通用的提示
-
类型安全：
toolNameForStorage函数使用类型断言，如果未来新增存储类型但忘记添加映射，会默默返回空字符串。建议在开发文档或代码注释中明确说明新增存储驱动时需要更新此函数
internal/offline_download/tool/add_test.go
改动意图：
为新增的工具识别和路由逻辑添加单元测试，确保功能正确性和防止回归。
代码修改逻辑：
-
TestIsEd2kCapableTool：测试 ED2K 工具识别功能- 测试了 5 个支持 ED2K 的工具（115 Cloud, 115 Open, Thunder, ThunderX, ThunderBrowser）
- 测试了 2 个不支持的工具（aria2, SimpleHttp）
- 覆盖了正向和反向用例
-
TestEd2kToolForStorage：测试存储到工具的映射逻辑- 测试了 115 Cloud 和 115 Open 返回对应工具名
- 测试了其他存储返回空字符串
- 使用
nil作为 "other" 的示例有点不够严谨
-
TestToolNameForStorage：测试完整的存储类型映射- 覆盖了 9 种存储驱动的类型映射
- 包含了所有在
toolNameForStorage中定义的类型 - 测试用例与实现代码一致
合理性评估：
✅ 优点：
- 测试覆盖率高：核心函数都有对应的测试用例
- 测试用例设计合理：包含正向、反向和边界情况
- 使用表驱动测试模式（table-driven tests），代码简洁易维护
💡 建议：
-
改进
TestEd2kToolForStorage：nil作为测试用例不够真实，建议使用一个实际不支持 ED2K 的存储驱动 -
缺少集成测试：当前只有单元测试，建议在后续 PR 或维护中添加
AddURL函数的集成测试，模拟实际的 ED2K 下载请求
internal/offline_download/tool/tools.go
改动意图：
新增 NamesForPath 方法，允许前端根据目标路径获取包含原生工具的工具列表。
代码修改逻辑：
-
新增
NamesForPath方法（第39-61行）：- 调用
op.GetStorageAndActualPath获取目标路径对应的存储驱动 - 调用
toolNameForStorage获取存储的原生工具名称 - 如果原生工具不在现有工具列表中，将其添加进去
- 返回排序后的工具列表
- 调用
-
处理边界情况：
- 如果路径解析失败，返回原有的工具列表
- 如果存储没有原生工具，返回原有列表
- 如果原生工具已经在列表中，避免重复添加
- 确保工具在工具管理器中已注册
合理性评估：
✅ 优点：
- 逻辑严谨：处理了多种边界情况，避免了重复添加和未注册工具
- 保持向后兼容：出错时返回原有列表，不影响现有功能
- 排序保证一致性：使用
sort.Strings确保返回列表的顺序一致
-
API 设计考量：
NamesForPath方法会根据路径动态返回不同的工具列表，这意味着前端需要在用户选择目标路径后重新获取工具列表。这个交互流程是否合理？ -
性能考虑：每次调用都会执行路径解析和存储查询，对于高频调用场景（如前端实时预览）可能有性能影响，是否需要添加缓存机制？
💡 建议：
- 建议在 PR 描述或代码注释中说明前端的交互流程，确保前后端对这个 API 的理解一致
- 如果这个方法会被高频调用，考虑添加简单的缓存或者将路径解析逻辑优化
server/handles/offline_download.go
改动意图：
修改 HTTP API endpoint，使其调用新的 NamesForPath 方法，支持基于路径的工具列表查询。
代码修改逻辑：
- 原代码：
tools := tool.Tools.Names()返回所有已注册工具 - 新代码：
tools := tool.Tools.NamesForPath(c.Query("path"))根据path查询参数返回工具列表 - 这是一个非破坏性修改：如果前端不传
path参数，c.Query("path")返回空字符串，NamesForPath内部会处理并返回默认列表
合理性评估：
✅ 优点：
- 向后兼容：不传
path参数时行为与原来一致 - 代码改动最小化：只改了一行，减少了引入 bug 的风险
💡 建议：
- API 文档：这个改动修改了公开 API 的行为（虽然向后兼容），建议更新 API 文档说明
path参数的作用 - 错误处理：当前实现中，如果
path无效，会静默返回默认工具列表。考虑是否需要在响应中添加一个字段，告知前端路径是否有效
🎯 总体评价
功能性：⭐⭐⭐⭐⭐ - 完整实现了需求，逻辑清晰，考虑了多种场景
安全性：⭐⭐⭐⭐⭐ - 无明显安全问题，错误处理得当
代码质量：⭐⭐⭐⭐ - 代码结构清晰，测试覆盖完整，但有小优化空间
实现方案：⭐⭐⭐⭐⭐ - 技术方案合理，扩展性强，向后兼容性好
建议操作：
- ✅ Approve（建议合并）
理由：
这是一个高质量的 PR，核心功能实现完整且正确：
-
问题解决彻底：准确定位了 SimpleHttp 拦截和工具路由的问题根源，通过渐进式降级策略优雅地解决了 115 存储的 ED2K 下载需求
-
代码质量优秀：
- 新增了完整的单元测试（86 行测试代码），覆盖率高
- 函数职责单一，命名清晰
- 保持了向后兼容性
-
技术方案可扩展：
toolNameForStorage函数已经为 9 种存储类型做了映射，为未来其他存储扩展 ED2K 或其他协议支持奠定了基础 -
团队协作规范：
- 关联了 issue 和前端 PR
- commit message 清晰，包含 Co-Authored-By 标记
- 遵循了项目的贡献规范
建议的改进点都是非关键性的小优化，不影响合并：
- 错误信息可以更通用一些
- API 响应可以提供更多上下文信息
- 测试用例可以更真实一些
前置条件：
- 需要配合前端 PR #631 一起部署，确保前端正确传递
path参数 - 建议在合并后添加集成测试或手动验证实际的 115 ED2K 下载流程
总体而言，这是一个值得合并的高质量 PR ✅
2 participants
Summary / 摘要
Expose the native offline-download tool for the destination storage and route ED2K requests to a compatible tool.
The existing 115 ED2K download path was manually verified successfully before this change. This PR addresses the empty/incomplete tool selection for the destination storage and adds backend routing safeguards; it does not claim that this change was the cause of the already successful download.
/ 此 PR 包含破坏性变更。
/ 此 PR 修改了公开 API、配置、存储格式或迁移行为。
/ 此 PR 需要关联仓库同步修改。
Related repository PRs / 关联仓库 PR:
Related Issues / 关联 Issue
Fixes #2891
Testing / 测试
go test ./internal/offline_download/tool ./server/handlesgit diff --checkgofmt -don all changed Go files produced no output.go test ./...— NOT RUN.Checklist / 检查清单
/ 我已阅读 CONTRIBUTING。
/ 我确认此贡献符合仓库许可证、贡献规范和行为准则。
gofmt,go fmt, orprettierwhere applicable./ 我已按适用情况使用
gofmt、go fmt或prettier格式化变更代码。/ 我已在适用情况下请求相关维护者或代码所有者审查。
AI Disclosure / AI 使用声明
/ 此 PR 包含 AI 辅助内容。
Tools used / 使用工具:
Usage scope / 使用范围:
Code generation / 代码生成
Refactoring / 重构
Documentation / 文档
Tests / 测试
Translation / 翻译
Review assistance / 审查辅助
I have reviewed and validated all AI-assisted content included in this PR.
/ 我已审核并验证此 PR 中的所有 AI 辅助内容。
I have ensured that all AI-assisted commits include
Co-Authored-Byattribution./ 我已确保所有 AI 辅助提交都包含
Co-Authored-By归属信息。I can reproduce all AI-assisted content included in this PR without any AI tools.
/ 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。