feat(drivers/alias): add autoMakeDir for missing dest - #2817
Conversation
…n folder if it does not exist Co-authored-by: Claude <claude@users.noreply.github.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3ed6c944f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return nil, errs.PermissionDenied | ||
| } | ||
| return d.getAllObjs(ctx, obj, getWriteAndPutFilterFunc(d.WriteConflictPolicy)) | ||
| return d.getAllObjs(ctx, obj, getWriteAndPutFilterFunc(d.WriteConflictPolicy), isAllWritePolicy(d.WriteConflictPolicy)) |
There was a problem hiding this comment.
When WriteConflictPolicy is all/all_strict, getWriteObjs is also used by Rename and Remove for the object being operated on, not just by MakeDir for a destination parent. If /foo is a directory that exists on backend A but is missing on backend B, renaming it now creates B's /foo and then renames that empty directory to /bar, so the alias gains a new empty destination on a backend that never had the source content. Limit autoMakeDir to destination-parent resolution paths such as upload/copy/move/mkdir, not source-object resolution.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
| // 目标为目录但该后端不存在时，按需自动创建，使新加入的存储也能参与写入 | ||
| if err != nil && autoMakeDir && bObj.IsDir() && errs.IsObjectNotFound(err) { |
There was a problem hiding this comment.
The auto-create branch only has a backend path for *tempObjs, but Alias.Get collapses any missing backend that appears before the first existing copy into nil, so this still misses those roots. For example, with alias paths [new, old] and /album present only on old, /album resolves to the old object plus nil; an all upload/mkdir under /album never sees new/album to create it, and all_strict still fails. The missing backend path needs to be preserved instead of represented only as nil when auto-creation is enabled.
Useful? React with 👍 / 👎.
| return nil, nil, errs.PermissionDenied | ||
| } | ||
| dstObjs, err := d.getAllObjs(ctx, dstDir, getWriteAndPutFilterFunc(d.PutConflictPolicy)) | ||
| dstObjs, err := d.getAllObjs(ctx, dstDir, getWriteAndPutFilterFunc(d.PutConflictPolicy), isAllWritePolicy(d.PutConflictPolicy)) |
There was a problem hiding this comment.
For PutConflictPolicy all/all_strict, resolving dstDir now creates missing destination parent directories before getMoveObjs checks whether there are enough source copies to move. If a file or folder exists on only one backend but the destination parent is auto-created on two backends, the later len(tmpSrcObjs) < len(dstObjs) check returns ErrNotEnoughSrcObjs, leaving the newly-created destination directory behind even though the move failed. Resolve the source count before performing mkdir side effects, or skip auto-creation for move cases that cannot be satisfied.
Useful? React with 👍 / 👎.
| } | ||
| // 目标为目录但该后端不存在时，按需自动创建，使新加入的存储也能参与写入 | ||
| if err != nil && autoMakeDir && bObj.IsDir() && errs.IsObjectNotFound(err) { | ||
| if mkErr := fs.MakeDir(ctx, o.GetPath()); mkErr == nil { |
There was a problem hiding this comment.
With all_strict, this loop creates each missing backend directory as it is encountered and immediately clears the error, but a later backend can still fail MakeDir; in that case getAllObjs returns before the upload/copy/mkdir runs while any earlier auto-created directories remain. For a destination that is missing on two backends where only the first can be created, a failed strict operation now leaks an empty directory onto the first backend. Preflight the targets or undo directories created during this resolution step when a later strict target cannot be prepared.
Useful? React with 👍 / 👎.
xrgzs
changed the title
autoMakeDir to automatically create the destinatio…autoMakeDir for missing dest
xrgzs
changed the title
autoMakeDir for missing destautoMakeDir for missing dest
PIKACHUIM
left a comment
PIKACHUIM
left a comment
There was a problem hiding this comment.
🙏 感谢贡献
感谢 @qiuxiuya 提交此PR！我已完成代码评审，以下是评审结果。
🤖 AI 自动审核声明
本评审报告由 AI 自动生成，当前使用 Claude Opus 5 模型进行分析，部分复杂场景可能辅助使用 ChatGPT、DeepSeek 等模型进行交叉验证。
⚠️ AI 分析结果仅供参考，可能存在误判或遗漏。如您发现任何问题或有不同意见，欢迎随时提出讨论和纠正。
⚠️ 重要提醒：即使 AI 评审认为代码质量良好且建议合并，最终是否合并仍需由项目维护者进行人工判定。项目维护者会综合考虑代码质量、项目规划、技术方向、团队资源等多方面因素做出决策。
📖 PR背景与需求
PR标题：feat(drivers/alias): add autoMakeDir for missing dest
关联Issue：无
需求说明：为 alias 驱动的 all / all_strict 写入策略增加目标目录缺失时自动创建功能，使新加入的存储也能参与写入。此前，写入操作的「有效路径」仅指目标对象已存在且类型匹配的后端路径。当某个后端（例如新加入的存储）缺少对应的子目录时，会被判定为无效路径而跳过，导致数据不会写入该后端。
预期目标：在 all / all_strict 策略下，当目标为目录、后端返回 ObjectNotFound 时，自动调用 fs.MakeDir 创建目录后纳入写入目标，使新加入的存储能够自动参与写入。
📋 问题摘要
- ✅ 功能性：功能设计合理，解决了实际需求
- ✅ 代码质量：代码结构清晰，改动最小化
- 💡 改进建议：有1处可优化点
📂 逐文件分析
drivers/alias/util.go
改动意图：在 getAllObjs() 函数中新增 autoMakeDir 参数，当目标为目录且后端返回 ObjectNotFound 时，自动创建目录。
代码修改逻辑：
getAllObjs()新增autoMakeDir bool参数（第123行）- 在目标为目录但该后端不存在时，如果
autoMakeDir为 true 且bObj.IsDir()为 true 且错误是ObjectNotFound，则自动调用fs.MakeDir(ctx, o.GetPath())创建目录（第143-150行） - 新增
isAllWritePolicy()辅助函数，判断策略是否为all/all_strict（第195-198行） - 在
getWriteObjs()、getPutObjs()、getCopyObjs()、getMoveObjs()中，当策略为all/all_strict时启用自动建目录（传入isAllWritePolicy(d.WriteConflictPolicy)或isAllWritePolicy(d.PutConflictPolicy)） - 读操作和源对象路径解析保持
autoMakeDir = false，不受影响
合理性评估：
-
✅ 优点：
- 功能实现精准，仅在
all/all_strict策略下启用自动建目录，不影响其他策略 - 改动最小化，仅在
getAllObjs()中增加一个参数和一段逻辑 - 使用
isAllWritePolicy()辅助函数提高代码可读性 - 自动创建失败时仅记录 Debug 日志，不会中断整个写入流程
- 读操作和源对象路径解析保持原逻辑，不受影响
- 功能实现精准，仅在
-
⚠️ 疑问：- 自动创建目录失败时（
mkErr != nil）仅记录 Debug 日志，是否应该记录 Warn 或 Error 级别的日志，便于用户排查问题？当前行为是静默跳过该后端，可能导致用户不知道某个后端没有参与写入。
- 自动创建目录失败时（
详细建议：
- 考虑提升日志级别：
这样用户可以在日志中看到目录创建失败的警告，便于排查配置问题（如权限不足、后端不支持创建目录等）。
if err != nil && autoMakeDir && bObj.IsDir() && errs.IsObjectNotFound(err) { if mkErr := fs.MakeDir(ctx, o.GetPath()); mkErr == nil { err = nil } else { log.Warnf("auto make dir %s failed: %+v, path will be skipped", o.GetPath(), mkErr) } }
drivers/alias/driver.go
改动意图：更新 Link() 方法中的 getAllObjs() 调用，传入新增的 autoMakeDir 参数。
代码修改逻辑：
- 在
Link()方法中，将d.getAllObjs(ctx, file, getWriteAndPutFilterFunc(AllRWP))改为d.getAllObjs(ctx, file, getWriteAndPutFilterFunc(AllRWP), false)（第246行） - 读操作不启用自动建目录
合理性评估：
- ✅ 优点：
- 正确传入
false，读操作不应该自动创建目录 - 保持原有行为不变
- 正确传入
🎯 总体评价
功能性：⭐⭐⭐⭐⭐ - 功能设计合理，解决了新加入存储无法参与写入的实际需求
安全性：⭐⭐⭐⭐⭐ - 无安全隐患，自动创建目录失败时不会影响其他后端
代码质量：⭐⭐⭐⭐ - 代码清晰，改动最小化，有小幅改进空间
实现方案：⭐⭐⭐⭐⭐ - 实现简洁高效，使用辅助函数提高可读性
建议操作：
- ✅ Approve（建议合并）
- 🔄 Request Changes（需要修改）
- ❌ Close（建议关闭）
理由：此 PR 功能完整、实现合理、改动最小化，可以安全合并。建议的改进点（提升日志级别）是锦上添花，不阻碍合并。如果维护者认同，可在后续 PR 中优化。
Next Steps / 后续建议：
- 考虑在日志中增加自动创建目录的成功信息（Info 级别），便于用户确认新存储已成功参与写入
- 可以在文档中补充此功能的使用场景示例（如动态添加新存储时无需手动创建目录结构）
再次感谢你的贡献！👏
pikachuren
left a comment
pikachuren
left a comment
There was a problem hiding this comment.
🙏 感谢 @qiuxiuya 提交！
🤖 AI 自动审核声明：本评审报告由 AI 自动生成，当前使用 Claude Opus 5 模型进行分析。
🎯 结论
🔄 Request Changes — 功能有价值且实现克制，但自动建目录的失败处理与副作用需要加强
📖 概要
feat(drivers/alias): add autoMakeDir for missing dest · 为 alias 驱动的 all / all_strict 写入策略增加目标目录缺失时自动创建。
核心改动：getAllObjs 新增 autoMakeDir 参数，遇到 ObjectNotFound 且目标为目录时调用 fs.MakeDir 补建。
🧭 整体方案
技术路线是在「解析写入目标」这一步顺带补建缺失目录，使新加入的后端存储也能参与写入。通过给 getAllObjs 增加布尔参数、并用 isAllWritePolicy 收敛策略判断，只在 all / all_strict 下启用，作用域控制得比较克制，方案合理。
📊 变更统计
2 个文件（+23 / -9 行） | 功能 ⭐⭐⭐⭐ | 最小改动 ⭐⭐⭐⭐ | 前向兼容 ⭐⭐⭐⭐ | 方案设计 ⭐⭐⭐
🚨 关键问题
P0（阻塞合并）：无
P1（建议修复）：
⚠️ drivers/alias/util.go:143—fs.MakeDir失败时仅log.Debugf记录后就继续，该后端被静默跳过。用户视角看到的是「写入成功」，但实际上某个后端根本没写进去，属于静默的数据不一致。尤其在all_strict（严格全写）策略下，这似乎与策略语义相悖。请问是否考虑：all_strict下建目录失败即返回错误，all下至少提升为log.Warnf呢？⚠️ 自动建目录是有副作用的写操作，却发生在getAllObjs这个从命名上看是「查询」的函数里。当上层因其他校验失败而中止时，已经创建的空目录不会回滚，会在后端留下残留。是否考虑把建目录动作上移到真正的写入流程中，或在文档中说明这一行为？
P2（可选）：
- 💡
fs.MakeDir是否为递归创建？如果目标路径有多级父目录缺失，需要确认单次调用能否建成，否则深层路径场景仍会失败~ - 💡 并发写入同一路径时，多个 goroutine 可能同时
MakeDir。多数驱动对此幂等，但建议确认一下是否所有后端都能安全处理重复创建~ - 💡 新增的
autoMakeDir布尔参数在 6 处调用点中有 3 处传false。若后续还要加开关，建议考虑改用 options 结构体，避免布尔参数堆积~
📂 逐文件分析
drivers/alias/util.go
改动意图：写入时自动补建缺失的后端目录。
代码逻辑：getAllObjs 增加 autoMakeDir 形参；在错误分支中判断 errs.IsObjectNotFound(err) && bObj.IsDir() 后调用 fs.MakeDir，成功则清空 err 让该后端重新纳入写入目标。新增 isAllWritePolicy 统一判定 AllRWP / AllStrictWP。
问题分析：判断条件精确（同时校验了 autoMakeDir、IsDir、ObjectNotFound 三个前提），作用域控制良好；主要问题是失败静默（P1）与查询函数产生写副作用（P1）。
详细建议：建议按策略区分失败处理：
if mkErr := fs.MakeDir(ctx, o.GetPath()); mkErr == nil {
err = nil
} else if d.WriteConflictPolicy == AllStrictWP {
return nil, errors.WithMessagef(mkErr, "auto make dir %s failed", o.GetPath())
} else {
log.Warnf("auto make dir %s failed: %+v", o.GetPath(), mkErr)
}drivers/alias/driver.go
改动意图：适配 getAllObjs 新签名。
问题分析：Link 处传 false（读路径不应建目录）正确，无问题。
✅ 待处理清单
- [P1]
all_strict策略下建目录失败应返回错误，all策略下至少升级为 Warn 日志 - [P1] 评估建目录副作用的位置，或明确文档说明可能留下空目录
- [P2] 确认
fs.MakeDir对多级缺失路径的递归行为 - [P2] 确认并发重复
MakeDir在各后端的幂等性
🎯 结论：🔄 Request Changes — 功能方向好、作用域克制，但静默失败在严格写策略下需要修正。
5 participants
Summary / 摘要
为
alias驱动的all/all_strict写入策略增加目标目录缺失时自动创建，使新加入的存储也能参与写入。此前，写入操作的「有效路径」仅指目标对象已存在且类型匹配的后端路径。当某个后端（例如新加入的存储）缺少对应的子目录时，会被判定为无效路径而跳过，导致数据不会写入该后端。本次改动在解析写入目标时，对返回
ObjectNotFound的目录后端自动调用fs.MakeDir创建目录后纳入写入目标。实现要点：
getAllObjs()新增autoMakeDir bool参数；当目标为目录、后端返回ObjectNotFound且开关开启时，自动fs.MakeDir创建后继续参与写入。新增
isAllWritePolicy()辅助函数，判断策略是否为all/all_strict。写入目标（Put/Copy/Mkdir/Rename/Remove 的目标目录）在
all/all_strict策略下启用自动建目录；源对象与读操作（Link 等）保持false，不受影响。This PR has breaking changes.
/ 此 PR 包含破坏性变更。
This PR changes public API, config, storage format, or migration behavior.
/ 此 PR 修改了公开 API、配置、存储格式或迁移行为。
This PR requires corresponding changes in related repositories.
/ 此 PR 需要关联仓库同步修改。
Related repository PRs / 关联仓库 PR:
Testing / 测试
Checklist / 检查清单
/ 我已阅读 CONTRIBUTING。
/ 我确认此贡献符合仓库许可证、贡献规范和行为准则。
gofmt,go fmt, orprettierwhere applicable./ 我已按适用情况使用
gofmt、go fmt或prettier格式化变更代码。/ 我已在适用情况下请求相关维护者或代码所有者审查。
AI Disclosure / AI 使用声明
/ 此 PR 包含 AI 辅助内容。
Tools used / 使用工具:
Usage scope / 使用范围:
/ 我已审核并验证此 PR 中的所有 AI 辅助内容。
Co-Authored-Byattribution./ 我已确保所有 AI 辅助提交都包含
Co-Authored-By归属信息。/ 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。