diff --git a/application/state/sftp/useSftpExternalOperations.ts b/application/state/sftp/useSftpExternalOperations.ts index 5a7147d7..a283135c 100644 --- a/application/state/sftp/useSftpExternalOperations.ts +++ b/application/state/sftp/useSftpExternalOperations.ts @@ -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 => { - const conflictType = conflict.isDirectory ? "directory" : "file"; + const conflictType = getSftpConflictTypeKey(conflict.isDirectory, conflict.existingType); const defaultAction = conflictDefaults.get(conflictType); if (defaultAction) return defaultAction; diff --git a/application/state/sftp/useSftpTransfers.ts b/application/state/sftp/useSftpTransfers.ts index ba388209..f926894d 100644 --- a/application/state/sftp/useSftpTransfers.ts +++ b/application/state/sftp/useSftpTransfers.ts @@ -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( + 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) => { diff --git a/application/state/uploadService.test.ts b/application/state/uploadService.test.ts index 6c64ebdb..d56af708 100644 --- a/application/state/uploadService.test.ts +++ b/application/state/uploadService.test.ts @@ -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[] = []; diff --git a/components/sftp/SftpConflictDialog.test.ts b/components/sftp/SftpConflictDialog.test.ts new file mode 100644 index 00000000..fd29683b --- /dev/null +++ b/components/sftp/SftpConflictDialog.test.ts @@ -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); +}); diff --git a/components/sftp/SftpConflictDialog.tsx b/components/sftp/SftpConflictDialog.tsx index 8a8a0f67..98dd5136 100644 --- a/components/sftp/SftpConflictDialog.tsx +++ b/components/sftp/SftpConflictDialog.tsx @@ -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): boolean => { + return canReplaceSftpConflict(conflict.isDirectory, conflict.existingType); +}; + +const getConflictTypeKey = (conflict: Pick): 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 = ({ 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 = ({ conflicts, {t('sftp.conflict.action.merge')} )} - + {canReplace && ( + + )} diff --git a/domain/sftpConflict.ts b/domain/sftpConflict.ts new file mode 100644 index 00000000..865d1804 --- /dev/null +++ b/domain/sftpConflict.ts @@ -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"; diff --git a/lib/uploadService.ts b/lib/uploadService.ts index 21dea3a1..6c15355f 100644 --- a/lib/uploadService.ts +++ b/lib/uploadService.ts @@ -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(); + 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); }