feat(explorer): overwrite/skip resolution for move/copy name conflicts (#3159)

Moving or copying files onto same-name destination objects previously
failed the whole batch with a single object-existed error. The move/copy
endpoint now accepts an optional on_conflict policy: "skip" drops the
colliding source entries with per-file errors, "overwrite" deletes the
colliding destination object through the regular delete path first.

On a conflict-bearing batch failure the web client now prompts for
Overwrite / Skip instead of only reporting the error, and retries with
the chosen policy. Trash-restore moves resolve conflicts against the
restored display name rather than the internal UUID name.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pull/3582/head
Tomas Dvorak 2 weeks ago
parent 4c45620689
commit e22277680e

@ -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",

@ -486,6 +486,10 @@
"saveAs": "另存为",
"versionConflict": "版本冲突",
"discardUnsavedConfirm": "有未保存的更改,确定要关闭吗?",
"nameConflict": "名称冲突",
"conflictOverwriteDes": "替换目标位置中的同名项目。",
"conflictSkipDes": "保留目标位置的同名项目,冲突的项目不会被移动或复制。",
"skip": "跳过",
"overwrite": "覆盖",
"editShareLink": "编辑分享链接",
"clearPermissions": "清除权限设置",

@ -468,12 +468,23 @@ export function sendMoveFile(req: MoveFileService): ThunkResponse<void> {
{
...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<void> {
return async (dispatch, _getState) => {
return await dispatch(

@ -294,6 +294,7 @@ export interface MoveFileService extends MultipleUriService {
dst: string;
copy?: boolean;
expect_ids?: string[];
on_conflict?: "skip" | "overwrite";
}
export interface MetadataPatch {

@ -137,6 +137,7 @@ export const Code = {
IncorrectPassword: 40069,
LockConflict: 40073,
StaleVersion: 40076,
ObjectExist: 40004,
BatchOperationNotFullyCompleted: 40081,
DomainNotLicensed: 40087,
AnonymouseAccessDenied: 40088,

@ -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) {

@ -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.

@ -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
}

@ -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())
}

@ -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)
}

Loading…
Cancel
Save