diff --git a/frontend/public/locales/en-US/application.json b/frontend/public/locales/en-US/application.json index e70152db..d0af75d8 100644 --- a/frontend/public/locales/en-US/application.json +++ b/frontend/public/locales/en-US/application.json @@ -486,6 +486,10 @@ "saveAs": "Save as", "versionConflict": "Version conflict", "discardUnsavedConfirm": "You have unsaved changes. Still discard?", + "nameConflict": "Name conflict", + "conflictOverwriteDes": "Replace the existing items in the destination.", + "conflictSkipDes": "Keep the existing items; conflicting items will not be moved or copied.", + "skip": "Skip", "overwrite": "Overwrite", "editShareLink": "Edit share link", "clearPermissions": "Clear permission settings", diff --git a/frontend/public/locales/zh-CN/application.json b/frontend/public/locales/zh-CN/application.json index 1e8d13ab..b0ed6e19 100644 --- a/frontend/public/locales/zh-CN/application.json +++ b/frontend/public/locales/zh-CN/application.json @@ -486,6 +486,10 @@ "saveAs": "另存为", "versionConflict": "版本冲突", "discardUnsavedConfirm": "有未保存的更改,确定要关闭吗?", + "nameConflict": "名称冲突", + "conflictOverwriteDes": "替换目标位置中的同名项目。", + "conflictSkipDes": "保留目标位置的同名项目,冲突的项目不会被移动或复制。", + "skip": "跳过", "overwrite": "覆盖", "editShareLink": "编辑分享链接", "clearPermissions": "清除权限设置", diff --git a/frontend/src/api/api.ts b/frontend/src/api/api.ts index 3332518a..d7fb7c17 100644 --- a/frontend/src/api/api.ts +++ b/frontend/src/api/api.ts @@ -468,12 +468,23 @@ export function sendMoveFile(req: MoveFileService): ThunkResponse { { ...defaultOpts, skipBatchError: req.uris.length == 1, + // Leave conflict failures silent: the caller prompts for + // overwrite/skip first (#3159). + bypassSnackbar: isNameConflictBatchError, }, ), ); }; } +export function isNameConflictBatchError(e: Error): boolean { + return ( + e instanceof AppError && + e.code == Code.BatchOperationNotFullyCompleted && + Object.values(e.aggregatedError ?? {}).some((r) => r.code == Code.ObjectExist) + ); +} + export function sendRestoreFile(req: DeleteFileService): ThunkResponse { return async (dispatch, _getState) => { return await dispatch( diff --git a/frontend/src/api/explorer.ts b/frontend/src/api/explorer.ts index ca6b84b9..86b6cd0e 100644 --- a/frontend/src/api/explorer.ts +++ b/frontend/src/api/explorer.ts @@ -294,6 +294,7 @@ export interface MoveFileService extends MultipleUriService { dst: string; copy?: boolean; expect_ids?: string[]; + on_conflict?: "skip" | "overwrite"; } export interface MetadataPatch { diff --git a/frontend/src/api/request.ts b/frontend/src/api/request.ts index bbb91d91..86a7afc9 100644 --- a/frontend/src/api/request.ts +++ b/frontend/src/api/request.ts @@ -137,6 +137,7 @@ export const Code = { IncorrectPassword: 40069, LockConflict: 40073, StaleVersion: 40076, + ObjectExist: 40004, BatchOperationNotFullyCompleted: 40081, DomainNotLicensed: 40087, AnonymouseAccessDenied: 40088, diff --git a/frontend/src/redux/thunks/file.ts b/frontend/src/redux/thunks/file.ts index b690a830..ea1811b5 100644 --- a/frontend/src/redux/thunks/file.ts +++ b/frontend/src/redux/thunks/file.ts @@ -6,6 +6,7 @@ import { getFileEntityUrl, getFileList, getFileThumb, + isNameConflictBatchError, sendCreateFile, sendDeleteFiles, sendEmptyTrash, @@ -75,7 +76,7 @@ import { } from "../globalStateSlice.ts"; import { ConfigLoadState, Viewers } from "../siteConfigSlice.ts"; import { AppThunk } from "../store.ts"; -import { confirmOperation, deleteConfirmation, renameForm, requestCreateNew, selectPath } from "./dialog.ts"; +import { confirmOperation, deleteConfirmation, renameForm, requestCreateNew, selectOption, selectPath } from "./dialog.ts"; import { downloadSingleFile } from "./download.ts"; import { navigateToPath, refreshFileList, updateUserCapacity } from "./filemanager.ts"; import { queueLoadShareInfo } from "./share.ts"; @@ -666,16 +667,47 @@ export function moveFiles(index: number, src: FileResponse[], dst: string, isCop return; } + const moveReq = (onConflict?: "skip" | "overwrite") => ({ + uris: src.map((f) => f.path), + dst, + copy: isCopy, + expect_ids: src.map((f) => f.id), + on_conflict: onConflict, + }); + const moveLabel = isCopy ? "application:modals.processingCopying" : "application:modals.processingMoving"; + let success = true; try { - await longRunningTaskWithSnackbar( - dispatch( - sendMoveFile({ uris: src.map((f) => f.path), dst, copy: isCopy, expect_ids: src.map((f) => f.id) }), - ), - isCopy ? "application:modals.processingCopying" : "application:modals.processingMoving", - ); + await longRunningTaskWithSnackbar(dispatch(sendMoveFile(moveReq())), moveLabel); } catch (e) { success = false; + if (isNameConflictBatchError(e as Error)) { + const onConflict = await dispatch( + selectOption( + [ + { + name: i18next.t("application:modals.overwrite"), + description: i18next.t("application:modals.conflictOverwriteDes"), + value: "overwrite", + }, + { + name: i18next.t("application:modals.skip"), + description: i18next.t("application:modals.conflictSkipDes"), + value: "skip", + }, + ], + "application:modals.nameConflict", + ), + ).catch(() => undefined); + if (onConflict == "skip" || onConflict == "overwrite") { + try { + await longRunningTaskWithSnackbar(dispatch(sendMoveFile(moveReq(onConflict))), moveLabel); + success = true; + } catch (e) { + // Retried operation failed; error snackbar already shown. + } + } + } } if (isCopy) { diff --git a/pkg/filemanager/fs/dbfs/dbfs.go b/pkg/filemanager/fs/dbfs/dbfs.go index 4cff6d5d..84b5719a 100644 --- a/pkg/filemanager/fs/dbfs/dbfs.go +++ b/pkg/filemanager/fs/dbfs/dbfs.go @@ -47,8 +47,23 @@ type ( // for move/copy/rename: a []int positionally aligned with the source // URI list. An entry of 0 disables the check for that position (#3565). ExpectedSourceIDsCtxKey struct{} + // MoveConflictCtxKey carries the on-conflict policy for move/copy: + // "skip" drops colliding entries, "overwrite" deletes the colliding + // destination first. Empty keeps the fail-fast default (#3159). + MoveConflictCtxKey struct{} ) +const ( + MoveConflictSkip = "skip" + MoveConflictOverwrite = "overwrite" +) + +// moveConflictMode returns the configured on-conflict policy. +func moveConflictMode(ctx context.Context) string { + mode, _ := ctx.Value(MoveConflictCtxKey{}).(string) + return mode +} + // WithExpectedSourceIDs records the expected database IDs of the source // files so a delayed/retried request cannot silently operate on a new // file that reused the same path. diff --git a/pkg/filemanager/fs/dbfs/manage.go b/pkg/filemanager/fs/dbfs/manage.go index 8a74cd32..a87596ea 100644 --- a/pkg/filemanager/fs/dbfs/manage.go +++ b/pkg/filemanager/fs/dbfs/manage.go @@ -562,6 +562,7 @@ func (f *DBFS) MoveOrCopy(ctx context.Context, path []*fs.URI, dst *fs.URI, isCo ae := serializer.NewAggregateError() fileNavGroup := make(map[Navigator][]*File) + navOf := make(map[*File]Navigator) dstRootPath := destination.Uri(true) ctx = context.WithValue(ctx, inventory.LoadFileEntity{}, true) ctx = context.WithValue(ctx, inventory.LoadFileMetadata{}, true) @@ -617,6 +618,7 @@ func (f *DBFS) MoveOrCopy(ctx context.Context, path []*fs.URI, dst *fs.URI, isCo } targets = append(targets, target) + navOf[target] = navigator if isCopy { if _, ok := fileNavGroup[navigator]; !ok { fileNavGroup[navigator] = make([]*File, 0) @@ -625,6 +627,13 @@ func (f *DBFS) MoveOrCopy(ctx context.Context, path []*fs.URI, dst *fs.URI, isCo } } + // Resolve name conflicts against existing destination children before + // locking: skip drops the colliding target, overwrite deletes the + // colliding destination object first (#3159). + if mode := moveConflictMode(ctx); mode != "" && len(targets) > 0 { + targets, fileNavGroup = f.resolveMoveConflicts(ctx, targets, navOf, destination, isCopy, mode, ae) + } + indexDiff := &fs.IndexDiff{} if len(targets) > 0 { // Lock all targets @@ -1053,3 +1062,48 @@ func (f *DBFS) moveFiles(ctx context.Context, targets []*File, destination *File return storageDiff, nil, nil } + +// resolveMoveConflicts filters or replaces targets whose names collide +// with existing destination children (#3159). "skip" drops the colliding +// target with a per-file ErrFileExisted; "overwrite" deletes the +// colliding destination object through the regular Delete path first. +func (f *DBFS) resolveMoveConflicts(ctx context.Context, targets []*File, navOf map[*File]Navigator, destination *File, isCopy bool, mode string, ae *serializer.AggregateError) ([]*File, map[Navigator][]*File) { + surviving := make([]*File, 0, len(targets)) + group := make(map[Navigator][]*File) + dstBase := destination.Uri(true) + + for _, target := range targets { + dstName := target.Name() + if !isCopy { + if _, ok := target.Metadata()[MetadataRestoreUri]; ok { + dstName = target.DisplayName() + } + } + + _, err := f.fileClient.GetChildFile(ctx, destination.Model, destination.OwnerID(), dstName, false) + if err != nil && !ent.IsNotFound(err) { + ae.Add(target.Uri(true).String(), fmt.Errorf("failed to check destination conflict: %w", err)) + continue + } + + if err == nil { + // Destination already holds an object under the same name. + if mode == MoveConflictSkip { + ae.Add(target.Uri(true).String(), fs.ErrFileExisted) + continue + } + if _, _, err := f.Delete(ctx, []*fs.URI{dstBase.Join(dstName)}); err != nil { + ae.Add(target.Uri(true).String(), err) + continue + } + } + + surviving = append(surviving, target) + if isCopy { + nav := navOf[target] + group[nav] = append(group[nav], target) + } + } + + return surviving, group +} diff --git a/pkg/filemanager/fs/dbfs/manage_conflict_test.go b/pkg/filemanager/fs/dbfs/manage_conflict_test.go new file mode 100644 index 00000000..bed145a1 --- /dev/null +++ b/pkg/filemanager/fs/dbfs/manage_conflict_test.go @@ -0,0 +1,119 @@ +package dbfs + +import ( + "context" + "fmt" + "testing" + + "github.com/cloudreve/Cloudreve/v4/ent" + "github.com/cloudreve/Cloudreve/v4/ent/enttest" + "github.com/cloudreve/Cloudreve/v4/inventory" + "github.com/cloudreve/Cloudreve/v4/inventory/types" + "github.com/cloudreve/Cloudreve/v4/pkg/boolset" + "github.com/cloudreve/Cloudreve/v4/pkg/conf" + "github.com/cloudreve/Cloudreve/v4/pkg/filemanager/fs" + "github.com/cloudreve/Cloudreve/v4/pkg/serializer" + "github.com/stretchr/testify/require" +) + +// conflictTestFixture creates user -> root -> dst folder, optionally with +// dstChildren file rows inside dst. Returns the user, the dst folder +// wrapped as *File and a real FileClient backed by the enttest client. +func conflictTestFixture(t *testing.T, client *ent.Client, dstChildren ...string) (*ent.User, *ent.File, *File, inventory.FileClient) { + t.Helper() + ctx := context.Background() + group := client.Group.Create().SetName("conflict").SetPermissions(&boolset.BooleanSet{}).SaveX(ctx) + user := client.User.Create().SetEmail("c@example.com").SetNick("c").SetGroup(group).SaveX(ctx) + root := client.File.Create().SetName(inventory.RootFolderName).SetType(int(types.FileTypeFolder)).SetOwner(user).SaveX(ctx) + dst := client.File.Create().SetName("dst").SetType(int(types.FileTypeFolder)).SetOwner(user).SetParent(root).SaveX(ctx) + for _, name := range dstChildren { + client.File.Create().SetName(name).SetType(int(types.FileTypeFile)).SetOwner(user).SetParent(dst).SaveX(ctx) + } + + dstUri, err := fs.NewUriFromString(fmt.Sprintf("%s/dst", fs.NewMyUri(""))) + require.NoError(t, err) + dstFile := newFile(nil, dst) + dstFile.OwnerModel = user + dstFile.Path[0] = dstUri + + return user, root, dstFile, inventory.NewFileClient(client, conf.SQLiteDB, nil) +} + +func conflictTestTarget(t *testing.T, model *ent.File, user *ent.User) *File { + t.Helper() + u, err := fs.NewUriFromString(fmt.Sprintf("%s/%s", fs.NewMyUri(""), model.Name)) + require.NoError(t, err) + f := newFile(nil, model) + f.OwnerModel = user + f.Path[0] = u + return f +} + +func TestResolveMoveConflictsSkipsColliding(t *testing.T) { + client := enttest.Open(t, "sqlite3", "file:"+t.Name()+"?mode=memory&cache=shared") + t.Cleanup(func() { require.NoError(t, client.Close()) }) + ctx := context.Background() + + user, root, dstFile, fc := conflictTestFixture(t, client, "a.txt") + + srcA := client.File.Create().SetName("a.txt").SetType(int(types.FileTypeFile)).SetOwner(user).SetParent(root).SaveX(ctx) + srcB := client.File.Create().SetName("b.txt").SetType(int(types.FileTypeFile)).SetOwner(user).SetParent(root).SaveX(ctx) + targetA := conflictTestTarget(t, srcA, user) + targetB := conflictTestTarget(t, srcB, user) + targets := []*File{targetA, targetB} + + f := &DBFS{fileClient: fc} + nav := Navigator(&myNavigator{}) + navOf := map[*File]Navigator{targetA: nav, targetB: nav} + ae := serializer.NewAggregateError() + + surviving, group := f.resolveMoveConflicts(ctx, targets, navOf, dstFile, true, MoveConflictSkip, ae) + + require.Equal(t, []*File{targetB}, surviving) + require.Equal(t, []*File{targetB}, group[nav]) + require.NotNil(t, ae.Aggregate()) +} + +func TestResolveMoveConflictsKeepsNonColliding(t *testing.T) { + client := enttest.Open(t, "sqlite3", "file:"+t.Name()+"?mode=memory&cache=shared") + t.Cleanup(func() { require.NoError(t, client.Close()) }) + ctx := context.Background() + + user, root, dstFile, fc := conflictTestFixture(t, client, "other.txt") + + srcA := client.File.Create().SetName("a.txt").SetType(int(types.FileTypeFile)).SetOwner(user).SetParent(root).SaveX(ctx) + targetA := conflictTestTarget(t, srcA, user) + + f := &DBFS{fileClient: fc} + ae := serializer.NewAggregateError() + + surviving, group := f.resolveMoveConflicts(ctx, []*File{targetA}, map[*File]Navigator{targetA: &myNavigator{}}, dstFile, true, MoveConflictSkip, ae) + + require.Equal(t, []*File{targetA}, surviving) + require.Len(t, group, 1) + require.Nil(t, ae.Aggregate()) +} + +func TestResolveMoveConflictsUsesRestoreDisplayName(t *testing.T) { + client := enttest.Open(t, "sqlite3", "file:"+t.Name()+"?mode=memory&cache=shared") + t.Cleanup(func() { require.NoError(t, client.Close()) }) + ctx := context.Background() + + // Destination holds a file under the restored display name, not the + // trash-internal UUID name. + user, root, dstFile, fc := conflictTestFixture(t, client, "original.txt") + + trashed := client.File.Create().SetName("uuid-name").SetType(int(types.FileTypeFile)).SetOwner(user).SetParent(root).SaveX(ctx) + trashed.Edges.Metadata = []*ent.Metadata{ + {Name: MetadataRestoreUri, Value: fmt.Sprintf("%s/original.txt", fs.NewMyUri(""))}, + } + target := conflictTestTarget(t, trashed, user) + + f := &DBFS{fileClient: fc} + ae := serializer.NewAggregateError() + + surviving, _ := f.resolveMoveConflicts(ctx, []*File{target}, map[*File]Navigator{}, dstFile, false, MoveConflictSkip, ae) + + require.Empty(t, surviving) + require.NotNil(t, ae.Aggregate()) +} diff --git a/service/explorer/file.go b/service/explorer/file.go index 9d85a8c2..6544bfb6 100644 --- a/service/explorer/file.go +++ b/service/explorer/file.go @@ -291,6 +291,11 @@ type ( // that entry with a conflict (#3565). An empty string entry skips // the check for that position. ExpectIDs []string `json:"expect_ids"` + // OnConflict selects the behaviour when a destination child with + // the same name exists: "skip" drops the colliding source, + // "overwrite" deletes the destination object first. Empty keeps + // the default fail-fast behaviour (#3159). + OnConflict string `json:"on_conflict" binding:"omitempty,eq=skip|eq=overwrite"` } ) @@ -332,6 +337,10 @@ func (s *MoveFileService) Move(c *gin.Context) error { util.WithValue(c, dbfs.ExpectedSourceIDsCtxKey{}, ids) } + if s.OnConflict != "" { + util.WithValue(c, dbfs.MoveConflictCtxKey{}, s.OnConflict) + } + return m.MoveOrCopy(c, uris, dst, s.Copy) }