feat(task): add path-prefix batch cancel, delete and retry#2846
feat(task): add path-prefix batch cancel, delete and retry#2846ifloppy wants to merge 2 commits into
Conversation
- Add `TaskWithPaths` interface and `MatchTaskPath` helper for matching tasks by virtual path prefix - Expose `GetSrcPath`/`GetDstPath` on transfer, upload, download and archive tasks - Add `delete_by_path`, `cancel_by_path` and `retry_by_path` endpoints for every task type - Cancel unfinished tasks before removing them so queued work stops - Scope matching to the requesting user and their base path - Exclude local temp and URL sources from path matching - Add unit and HTTP-level tests covering matching, permissions and large task sets Signed-off-by: ifloppy <68799904+ifloppy@users.noreply.github.com>
|
I reviewed the path matching, permission boundaries, state filtering, and test coverage. The overall direction looks reasonable, but I have several concerns before this can be merged.
For normal uploads, the destination path omits the uploaded file name. Archive-content uploads similarly omit the object name. As a result, filtering by an exact destination file path will not match the corresponding task, even though the API describes this as matching the task's source or destination path. Please either include the final object name in these paths and add tests for exact-file matching, or explicitly define and document that these task types use destination-directory semantics.
Please add a test with a genuinely running/blocking task that verifies cancellation is observed, execution stops, cleanup completes, and the task is then removed without continuing in the background.
Inputs such as Please require an explicit
The returned count currently represents tasks selected by the initial filter, not necessarily operations that completed successfully. Concurrent task changes can make
Please add or link API documentation covering path normalization, source-or-destination matching, user scoping, root-path behavior, state filtering, and count semantics. The existing tests cover matching boundaries, ownership, traversal attempts, state filtering, and scale well. Once the lifecycle and path-contract questions above are addressed, the backend and OpenList-Frontend#608 should be reviewed and merged as one coordinated change. |
- Match upload and archive-content tasks by their final destination object path - Require an explicit slash for operations that resolve to the global root - Wait for running task cancellation and cleanup before removing tasks - Return separate matched and processed counts for concurrent state changes - Document normalization, scoping, state filters, lifecycle and response semantics - Cover exact-file matching, lifecycle cleanup, implicit root rejection and count races Signed-off-by: ifloppy <68799904+ifloppy@users.noreply.github.com>
|
Thank you for the detailed review. I addressed each concern in follow-up commit e8879b3, with the corresponding frontend adjustment in OpenList-Frontend#608 commit ef01089.
Validation performed:
All of the above pass. The follow-up implementation and this response were AI-assisted with Claude, as disclosed in the updated PR AI Disclosure section, and the changes/tests were reviewed and validated before pushing. |
Summary / 摘要
Adds path-prefix batch operations for tasks, so large numbers of tasks can be cancelled, deleted or retried without selecting them one by one.
为任务增加按路径前缀的批量操作,使大量任务无需逐条勾选即可取消、删除或重试。
Motivation: with tens of thousands of queued tasks, the existing
delete_some/cancel_someendpoints require sending every task ID, andclear_donecannot filter by path. Cleaning up all tasks under one directory was impractical.动机:在有上万条排队任务时,现有
delete_some/cancel_some需要传入每个任务 ID,而clear_done无法按路径过滤,导致按目录清理任务难以实施。User-visible changes / 用户可感知的变化:
upload,copy,move,offline_download,offline_download_transfer,decompress,decompress_upload):POST /api/task/{type}/delete_by_pathPOST /api/task/{type}/cancel_by_pathPOST /api/task/{type}/retry_by_path{"path": "/prefix"}{"matched": N, "processed": M}delete_by_pathcancels all matching unfinished tasks first, waits for active execution and cleanup to finish, then removes only tasks confirmed safe to removecancel_by_pathonly affects unfinished tasks;retry_by_pathonly affects failed tasks/; ambiguous values such as.,./, or//are rejected if they resolve to/Implementation changes / 重要实现变化:
Added
task.TaskWithPathsinterface andtask.MatchTaskPathhelperImplemented final-object
GetSrcPath/GetDstPathvalues onTaskData,UploadTask,ArchiveContentUploadTaskandDownloadTaskAdded two-phase running-task deletion: cancel all, wait up to the shared deadline for terminal state/cleanup, then remove confirmed stopped tasks
Matching reuses
utils.IsSubPath, so/adoes not match/abLocal temp paths and offline-download URLs are excluded from matching to avoid unintended deletions
Non-admin requests are resolved through
user.JoinPath, and matching is limited to the requesting user's own tasksExisting endpoints are untouched
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 / 测试
Commands run on Windows (
go1.26.4):go test ./internal/task/ ./internal/fs/ ./internal/offline_download/tool/ ./server/handles/ ./pkg/utils/— all passgo vetandgofmt -lon the changed packages — cleango build ./...— succeedsAutomated coverage added:
/avs/ab, backslash and relative path cleaning, empty path sides.,./,//, and repeated separators are rejected when they would resolve to global/; explicit/is acceptedcancel_by_pathskips terminal states,retry_by_pathonly touches failed taskshttptest): a non-admin cannot affect another user's tasks, and path traversal such as../../etc/secretcannot reach tasks outside the user's base pathRunexits and delayedOnFailedcleanup finishes, and the task remains inactive after removalmatched=1, processed=0Manual test / 手动测试:
linux/amd64binary with the frontend embedded and ran it on Linuxpath is requiredundone,done,clear_doneanddelete_somestill respond normallyNote:
go test ./...also reports pre-existing failures indrivers/189,drivers/chaoxing,drivers/google_drive,drivers/lanzou,pkg/aria2/rpcandinternal/net. These are unrelated to this change; I confirmed they reproduce identically with this branch's changes stashed (non-constant format stringvet checks from a newer Go, and tests requiring a local aria2 instance).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 辅助内容。
The commits in this PR do not carry a
Co-Authored-Bytrailer. The AI assistance is disclosed here instead. Please let me know if you would prefer the trailer added to the commits, and I will amend them.此 PR 的提交未附带
Co-Authored-Bytrailer,AI 辅助情况改在此处声明。如维护者希望在提交中补充该 trailer,请告知,我会修改提交。