Fix SFTP type-mismatch upload conflicts (#1449)
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import { useCallback, useRef, useMemo, useState } from "react";
|
||||
import { FileConflict, FileConflictAction, TransferStatus, SftpFilenameEncoding } from "../../../domain/models";
|
||||
import { getSftpConflictTypeKey } from "../../../domain/sftpConflict";
|
||||
import { netcattyBridge } from "../../../infrastructure/services/netcattyBridge";
|
||||
import { logger } from "../../../lib/logger";
|
||||
import { notify } from "../../notification";
|
||||
@@ -501,7 +502,7 @@ export const useSftpExternalOperations = (
|
||||
newModified: number;
|
||||
applyToAllCount: number;
|
||||
}): Promise<FileConflictAction> => {
|
||||
const conflictType = conflict.isDirectory ? "directory" : "file";
|
||||
const conflictType = getSftpConflictTypeKey(conflict.isDirectory, conflict.existingType);
|
||||
const defaultAction = conflictDefaults.get(conflictType);
|
||||
if (defaultAction) return defaultAction;
|
||||
|
||||
|
||||
@@ -7,6 +7,12 @@ import {
|
||||
TransferStatus,
|
||||
TransferTask,
|
||||
} from "../../../domain/models";
|
||||
import {
|
||||
canReplaceSftpConflict,
|
||||
describeSftpExistingKind,
|
||||
describeSftpIncomingKind,
|
||||
getSftpConflictTypeKey,
|
||||
} from "../../../domain/sftpConflict";
|
||||
import { netcattyBridge } from "../../../infrastructure/services/netcattyBridge";
|
||||
import { logger } from "../../../lib/logger";
|
||||
import { SftpPane } from "./types";
|
||||
@@ -69,8 +75,14 @@ export const useSftpTransfers = ({
|
||||
);
|
||||
|
||||
const conflictDefaultKey = useCallback(
|
||||
(batchId: string | undefined, isDirectory: boolean) =>
|
||||
`${batchId ?? "global"}:${isDirectory ? "directory" : "file"}`,
|
||||
(batchId: string | undefined, isDirectory: boolean, existingType?: "file" | "directory" | "symlink") =>
|
||||
`${batchId ?? "global"}:${getSftpConflictTypeKey(isDirectory, existingType)}`,
|
||||
[],
|
||||
);
|
||||
|
||||
const buildReplaceTypeMismatchError = useCallback(
|
||||
(isDirectory: boolean, existingType: "file" | "directory" | "symlink" | undefined, targetPath: string) =>
|
||||
`Cannot replace existing ${describeSftpExistingKind(existingType)} with ${describeSftpIncomingKind(isDirectory)}: ${targetPath}`,
|
||||
[],
|
||||
);
|
||||
|
||||
@@ -233,6 +245,33 @@ export const useSftpTransfers = ({
|
||||
const existingStat = await statTargetPath(targetPane, targetSftpId, task.targetPath, targetEncoding);
|
||||
|
||||
if (existingStat) {
|
||||
const applyToAllCount = task.batchId
|
||||
? await (async () => {
|
||||
const candidates = transfersRef.current.filter((candidate) =>
|
||||
candidate.batchId === task.batchId &&
|
||||
candidate.isDirectory === task.isDirectory &&
|
||||
!candidate.parentTaskId &&
|
||||
candidate.status !== "completed" &&
|
||||
candidate.status !== "cancelled",
|
||||
);
|
||||
const matches = await Promise.all(candidates.map(async (candidate) => {
|
||||
if (candidate.id === task.id) return true;
|
||||
try {
|
||||
const candidateStat = await statTargetPath(
|
||||
targetPane,
|
||||
targetSftpId,
|
||||
candidate.targetPath,
|
||||
targetEncoding,
|
||||
);
|
||||
return candidateStat?.type === existingStat.type;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}));
|
||||
return Math.max(1, matches.filter(Boolean).length);
|
||||
})()
|
||||
: 1;
|
||||
|
||||
return {
|
||||
transferId: task.id,
|
||||
batchId: task.batchId,
|
||||
@@ -241,15 +280,7 @@ export const useSftpTransfers = ({
|
||||
targetPath: task.targetPath,
|
||||
isDirectory: task.isDirectory,
|
||||
existingType: existingStat.type,
|
||||
applyToAllCount: task.batchId
|
||||
? transfersRef.current.filter((candidate) =>
|
||||
candidate.batchId === task.batchId &&
|
||||
candidate.isDirectory === task.isDirectory &&
|
||||
!candidate.parentTaskId &&
|
||||
candidate.status !== "completed" &&
|
||||
candidate.status !== "cancelled",
|
||||
).length
|
||||
: 1,
|
||||
applyToAllCount,
|
||||
existingSize: existingStat.size,
|
||||
newSize: sourceStat?.size || task.totalBytes || 0,
|
||||
existingModified: existingStat.mtime,
|
||||
@@ -271,7 +302,9 @@ export const useSftpTransfers = ({
|
||||
const conflict = await conflictCheckPromise;
|
||||
|
||||
if (conflict) {
|
||||
const defaultAction = conflictDefaultsRef.current.get(conflictDefaultKey(task.batchId, task.isDirectory));
|
||||
const defaultAction = conflictDefaultsRef.current.get(
|
||||
conflictDefaultKey(task.batchId, task.isDirectory, conflict.existingType),
|
||||
);
|
||||
if (defaultAction) {
|
||||
if (defaultAction === "stop") {
|
||||
await markBatchStopped(task);
|
||||
@@ -285,6 +318,16 @@ export const useSftpTransfers = ({
|
||||
return "cancelled";
|
||||
}
|
||||
|
||||
if (defaultAction === "replace" && !canReplaceSftpConflict(task.isDirectory, conflict.existingType)) {
|
||||
updateTask({
|
||||
status: "failed",
|
||||
endTime: Date.now(),
|
||||
error: buildReplaceTypeMismatchError(task.isDirectory, conflict.existingType, task.targetPath),
|
||||
retryable: false,
|
||||
});
|
||||
return "failed";
|
||||
}
|
||||
|
||||
const duplicateTarget = defaultAction === "duplicate"
|
||||
? await getDuplicateTarget(task, targetPane, targetSftpId, targetEncoding)
|
||||
: null;
|
||||
@@ -728,16 +771,19 @@ export const useSftpTransfers = ({
|
||||
return;
|
||||
}
|
||||
|
||||
const selectedConflictKey = conflictDefaultKey(task.batchId, task.isDirectory);
|
||||
const selectedConflictKey = conflictDefaultKey(conflict.batchId, conflict.isDirectory, conflict.existingType);
|
||||
const affectedConflicts = applyToAll
|
||||
? conflictsRef.current.filter((candidate) =>
|
||||
conflictDefaultKey(candidate.batchId, candidate.isDirectory) === selectedConflictKey,
|
||||
conflictDefaultKey(candidate.batchId, candidate.isDirectory, candidate.existingType) === selectedConflictKey,
|
||||
)
|
||||
: [conflict];
|
||||
const affectedConflictIds = new Set(affectedConflicts.map((candidate) => candidate.transferId));
|
||||
const affectedTasks = affectedConflicts
|
||||
.map((candidate) => transfersRef.current.find((transfer) => transfer.id === candidate.transferId))
|
||||
.filter((candidate): candidate is TransferTask => Boolean(candidate));
|
||||
const affectedConflictById = new Map<string, FileConflict>(
|
||||
affectedConflicts.map((candidate): [string, FileConflict] => [candidate.transferId, candidate]),
|
||||
);
|
||||
|
||||
if (applyToAll) {
|
||||
conflictDefaultsRef.current.set(selectedConflictKey, action);
|
||||
@@ -771,9 +817,11 @@ export const useSftpTransfers = ({
|
||||
}
|
||||
|
||||
const updatedTasks: TransferTask[] = [];
|
||||
const blockedReplaceTasks: Array<{ task: TransferTask; conflict: FileConflict }> = [];
|
||||
|
||||
for (const affectedTask of affectedTasks) {
|
||||
let updatedTask = { ...affectedTask };
|
||||
const affectedConflict = affectedConflictById.get(affectedTask.id);
|
||||
|
||||
if (action === "duplicate") {
|
||||
const endpoints = resolveTaskEndpoints(affectedTask);
|
||||
@@ -792,6 +840,13 @@ export const useSftpTransfers = ({
|
||||
skipConflictCheck: true,
|
||||
};
|
||||
} else if (action === "replace") {
|
||||
if (
|
||||
affectedConflict &&
|
||||
!canReplaceSftpConflict(affectedTask.isDirectory, affectedConflict.existingType)
|
||||
) {
|
||||
blockedReplaceTasks.push({ task: affectedTask, conflict: affectedConflict });
|
||||
continue;
|
||||
}
|
||||
updatedTask = {
|
||||
...affectedTask,
|
||||
skipConflictCheck: true,
|
||||
@@ -808,6 +863,28 @@ export const useSftpTransfers = ({
|
||||
updatedTasks.push(updatedTask);
|
||||
}
|
||||
|
||||
if (blockedReplaceTasks.length > 0) {
|
||||
const blockedTaskIds = new Set(blockedReplaceTasks.map(({ task }) => task.id));
|
||||
const blockedErrors = new Map(
|
||||
blockedReplaceTasks.map(({ task, conflict }) => [
|
||||
task.id,
|
||||
buildReplaceTypeMismatchError(task.isDirectory, conflict.existingType, task.targetPath),
|
||||
]),
|
||||
);
|
||||
setTransfers((prev) =>
|
||||
prev.map((t) => blockedTaskIds.has(t.id)
|
||||
? {
|
||||
...t,
|
||||
status: "failed" as TransferStatus,
|
||||
endTime: Date.now(),
|
||||
error: blockedErrors.get(t.id),
|
||||
retryable: false,
|
||||
}
|
||||
: t,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
const updatedTaskMap = new Map(updatedTasks.map((updatedTask) => [updatedTask.id, updatedTask]));
|
||||
setTransfers((prev) =>
|
||||
prev.map((t) => {
|
||||
|
||||
@@ -139,6 +139,86 @@ test("uploads picked folder files with their relative directory structure", asyn
|
||||
]);
|
||||
});
|
||||
|
||||
test("does not replace an existing directory when uploading a same-named file", async () => {
|
||||
const file = new File(["local"], "dddd", { lastModified: 1234 });
|
||||
const deletedPaths: string[] = [];
|
||||
const uploadedPaths: string[] = [];
|
||||
|
||||
const results = await uploadFromFileList(
|
||||
[file],
|
||||
{
|
||||
targetPath: "/target",
|
||||
sftpId: "sftp-1",
|
||||
isLocal: false,
|
||||
bridge: {
|
||||
mkdirSftp: async () => {},
|
||||
statSftp: async (_sftpId, path) =>
|
||||
path === "/target/dddd"
|
||||
? { type: "directory", size: 0, lastModified: 1000 }
|
||||
: null,
|
||||
deleteSftp: async (_sftpId, path) => {
|
||||
deletedPaths.push(path);
|
||||
},
|
||||
writeSftpBinary: async (_sftpId, path) => {
|
||||
uploadedPaths.push(path);
|
||||
},
|
||||
},
|
||||
joinPath: (base, name) => `${base}/${name}`,
|
||||
resolveConflict: async () => "replace",
|
||||
},
|
||||
);
|
||||
|
||||
assert.deepEqual(deletedPaths, []);
|
||||
assert.deepEqual(uploadedPaths, []);
|
||||
assert.equal(results.length, 1);
|
||||
assert.equal(results[0].fileName, "dddd");
|
||||
assert.equal(results[0].success, false);
|
||||
assert.match(results[0].error ?? "", /directory/i);
|
||||
});
|
||||
|
||||
test("counts apply-to-all upload conflicts by incoming and existing type", async () => {
|
||||
const files = [
|
||||
new File(["local"], "existing-file", { lastModified: 1234 }),
|
||||
new File(["local"], "existing-directory", { lastModified: 1234 }),
|
||||
];
|
||||
const conflictCounts: number[] = [];
|
||||
|
||||
const results = await uploadFromFileList(
|
||||
files,
|
||||
{
|
||||
targetPath: "/target",
|
||||
sftpId: "sftp-1",
|
||||
isLocal: false,
|
||||
bridge: {
|
||||
mkdirSftp: async () => {},
|
||||
statSftp: async (_sftpId, path) => {
|
||||
if (path === "/target/existing-file") {
|
||||
return { type: "file", size: 2, lastModified: 1000 };
|
||||
}
|
||||
if (path === "/target/existing-directory") {
|
||||
return { type: "directory", size: 0, lastModified: 1000 };
|
||||
}
|
||||
return null;
|
||||
},
|
||||
writeSftpBinary: async () => {
|
||||
throw new Error("skipped conflicts should not upload");
|
||||
},
|
||||
},
|
||||
joinPath: (base, name) => `${base}/${name}`,
|
||||
resolveConflict: async (conflict) => {
|
||||
conflictCounts.push(conflict.applyToAllCount);
|
||||
return "skip";
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
assert.deepEqual(conflictCounts, [1, 1]);
|
||||
assert.deepEqual(results, [
|
||||
{ fileName: "existing-file", success: false, cancelled: true },
|
||||
{ fileName: "existing-directory", success: false, cancelled: true },
|
||||
]);
|
||||
});
|
||||
|
||||
test("uploads path-backed clipboard files through stream transfer", async () => {
|
||||
const transfers: Array<{ sourcePath: string; targetPath: string; totalBytes?: number }> = [];
|
||||
const taskTotals: number[] = [];
|
||||
|
||||
25
components/sftp/SftpConflictDialog.test.ts
Normal file
25
components/sftp/SftpConflictDialog.test.ts
Normal file
@@ -0,0 +1,25 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { canReplaceConflict } from "./SftpConflictDialog.tsx";
|
||||
|
||||
test("does not offer replace when a file upload conflicts with an existing directory", () => {
|
||||
assert.equal(canReplaceConflict({
|
||||
isDirectory: false,
|
||||
existingType: "directory",
|
||||
}), false);
|
||||
});
|
||||
|
||||
test("does not offer replace when a directory upload conflicts with an existing file", () => {
|
||||
assert.equal(canReplaceConflict({
|
||||
isDirectory: true,
|
||||
existingType: "file",
|
||||
}), false);
|
||||
});
|
||||
|
||||
test("offers replace when a file upload conflicts with an existing file", () => {
|
||||
assert.equal(canReplaceConflict({
|
||||
isDirectory: false,
|
||||
existingType: "file",
|
||||
}), true);
|
||||
});
|
||||
@@ -5,6 +5,7 @@
|
||||
import { AlertCircle } from 'lucide-react';
|
||||
import React, { memo, useState } from 'react';
|
||||
import { useI18n } from '../../application/i18n/I18nProvider';
|
||||
import { canReplaceSftpConflict, getSftpConflictTypeKey } from '../../domain/sftpConflict';
|
||||
import { Button } from '../ui/button';
|
||||
import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from '../ui/dialog';
|
||||
import type { FileConflictAction } from '../../domain/models';
|
||||
@@ -23,6 +24,13 @@ interface ConflictItem {
|
||||
newModified: number;
|
||||
}
|
||||
|
||||
export const canReplaceConflict = (conflict: Pick<ConflictItem, 'isDirectory' | 'existingType'>): boolean => {
|
||||
return canReplaceSftpConflict(conflict.isDirectory, conflict.existingType);
|
||||
};
|
||||
|
||||
const getConflictTypeKey = (conflict: Pick<ConflictItem, 'isDirectory' | 'existingType'>): string =>
|
||||
getSftpConflictTypeKey(conflict.isDirectory, conflict.existingType);
|
||||
|
||||
interface SftpConflictDialogProps {
|
||||
conflicts: ConflictItem[];
|
||||
onResolve: (conflictId: string, action: FileConflictAction, applyToAll?: boolean) => void;
|
||||
@@ -42,9 +50,10 @@ const SftpConflictDialogInner: React.FC<SftpConflictDialogProps> = ({ conflicts,
|
||||
|
||||
const sameTypeConflictCount = Math.max(
|
||||
conflict.applyToAllCount ?? 1,
|
||||
conflicts.filter((item) => item.isDirectory === conflict.isDirectory).length,
|
||||
conflicts.filter((item) => getConflictTypeKey(item) === getConflictTypeKey(conflict)).length,
|
||||
);
|
||||
const canMerge = conflict.isDirectory && conflict.existingType === 'directory';
|
||||
const canReplace = canReplaceConflict(conflict);
|
||||
|
||||
const handleAction = (action: FileConflictAction) => {
|
||||
onResolve(conflict.transferId, action, applyToAll);
|
||||
@@ -145,13 +154,15 @@ const SftpConflictDialogInner: React.FC<SftpConflictDialogProps> = ({ conflicts,
|
||||
{t('sftp.conflict.action.merge')}
|
||||
</Button>
|
||||
)}
|
||||
<Button
|
||||
variant="default"
|
||||
onClick={() => handleAction('replace')}
|
||||
className="flex-1"
|
||||
>
|
||||
{t('sftp.conflict.action.replace')}
|
||||
</Button>
|
||||
{canReplace && (
|
||||
<Button
|
||||
variant="default"
|
||||
onClick={() => handleAction('replace')}
|
||||
className="flex-1"
|
||||
>
|
||||
{t('sftp.conflict.action.replace')}
|
||||
</Button>
|
||||
)}
|
||||
</DialogFooter>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
|
||||
20
domain/sftpConflict.ts
Normal file
20
domain/sftpConflict.ts
Normal file
@@ -0,0 +1,20 @@
|
||||
export type SftpConflictExistingType = "file" | "directory" | "symlink";
|
||||
|
||||
export const getSftpConflictTypeKey = (
|
||||
isDirectory: boolean,
|
||||
existingType?: SftpConflictExistingType,
|
||||
): string => `${isDirectory ? "directory" : "file"}:${existingType ?? "unknown"}`;
|
||||
|
||||
export const canReplaceSftpConflict = (
|
||||
isDirectory: boolean,
|
||||
existingType?: SftpConflictExistingType,
|
||||
): boolean => {
|
||||
if (!existingType) return true;
|
||||
return (existingType === "directory") === isDirectory;
|
||||
};
|
||||
|
||||
export const describeSftpIncomingKind = (isDirectory: boolean): string =>
|
||||
isDirectory ? "directory" : "file";
|
||||
|
||||
export const describeSftpExistingKind = (existingType?: SftpConflictExistingType): string =>
|
||||
existingType === "directory" ? "directory" : "file";
|
||||
@@ -9,6 +9,12 @@
|
||||
import { extractDropEntries, DropEntry, getPathForFile } from "./sftpFileUtils";
|
||||
import { logger } from "./logger";
|
||||
import { uploadFoldersCompressed } from "./uploadCompressed";
|
||||
import {
|
||||
canReplaceSftpConflict,
|
||||
describeSftpExistingKind,
|
||||
describeSftpIncomingKind,
|
||||
getSftpConflictTypeKey,
|
||||
} from "../domain/sftpConflict";
|
||||
|
||||
// ============================================================================
|
||||
// Types
|
||||
@@ -397,22 +403,37 @@ async function uploadEntries(
|
||||
if (resolveConflict) {
|
||||
const resolved: DropEntry[] = [];
|
||||
let stop = false;
|
||||
const groups = Array.from(rootFolders.entries());
|
||||
|
||||
for (const [key, groupEntries] of groups) {
|
||||
if (stop || controller?.isCancelled()) break;
|
||||
|
||||
const groupInfos = await Promise.all(Array.from(rootFolders.entries()).map(async ([key, groupEntries]) => {
|
||||
const isStandaloneFile = key.startsWith("__file__");
|
||||
const rootName = isStandaloneFile ? key.slice("__file__".length) : key;
|
||||
const isDirectory = !isStandaloneFile;
|
||||
const rootTargetPath = joinPath(targetPath, rootName);
|
||||
const existing = await statTarget(rootTargetPath);
|
||||
return {
|
||||
groupEntries,
|
||||
rootName,
|
||||
isDirectory,
|
||||
rootTargetPath,
|
||||
existing,
|
||||
};
|
||||
}));
|
||||
|
||||
const conflictCounts = new Map<string, number>();
|
||||
for (const info of groupInfos) {
|
||||
if (!info.existing) continue;
|
||||
const conflictKey = getSftpConflictTypeKey(info.isDirectory, info.existing.type);
|
||||
conflictCounts.set(conflictKey, (conflictCounts.get(conflictKey) ?? 0) + 1);
|
||||
}
|
||||
|
||||
for (const { groupEntries, rootName, isDirectory, rootTargetPath, existing } of groupInfos) {
|
||||
if (stop || controller?.isCancelled()) break;
|
||||
|
||||
if (!existing) {
|
||||
resolved.push(...groupEntries);
|
||||
continue;
|
||||
}
|
||||
|
||||
const conflictKey = getSftpConflictTypeKey(isDirectory, existing.type);
|
||||
const newSize = groupEntries.reduce((sum, entry) => sum + getDropEntrySize(entry), 0);
|
||||
const action = await resolveConflict({
|
||||
fileName: rootName,
|
||||
@@ -423,7 +444,7 @@ async function uploadEntries(
|
||||
newSize,
|
||||
existingModified: existing.lastModified,
|
||||
newModified: Date.now(),
|
||||
applyToAllCount: groups.filter(([groupKey]) => groupKey.startsWith("__file__") !== isDirectory).length,
|
||||
applyToAllCount: conflictCounts.get(conflictKey) ?? 1,
|
||||
});
|
||||
|
||||
if (action === "stop") {
|
||||
@@ -440,6 +461,14 @@ async function uploadEntries(
|
||||
}
|
||||
|
||||
if (action === "replace") {
|
||||
if (!canReplaceSftpConflict(isDirectory, existing.type)) {
|
||||
results.push({
|
||||
fileName: rootName,
|
||||
success: false,
|
||||
error: `Cannot replace existing ${describeSftpExistingKind(existing.type)} with ${describeSftpIncomingKind(isDirectory)}: ${rootTargetPath}`,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
await deleteTarget(rootTargetPath);
|
||||
resolved.push(...groupEntries);
|
||||
continue;
|
||||
@@ -451,6 +480,15 @@ async function uploadEntries(
|
||||
continue;
|
||||
}
|
||||
|
||||
if (action === "merge" && !(isDirectory && existing.type === "directory")) {
|
||||
results.push({
|
||||
fileName: rootName,
|
||||
success: false,
|
||||
error: `Cannot merge existing ${describeSftpExistingKind(existing.type)} with ${describeSftpIncomingKind(isDirectory)}: ${rootTargetPath}`,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
resolved.push(...groupEntries);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user