From e8e01c054a867447e07a71960d45f9cb36926e61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A1lint=20Ujv=C3=A1ri?= <58116288+bosi95@users.noreply.github.com> Date: Thu, 12 Mar 2026 15:19:53 +0100 Subject: [PATCH] fix: download and upload files (#223) * fix: download for unknown mime types * fix: async process two files from the queue * refactor: upload functions --- .../VersionHistoryModal.tsx | 8 +- .../filemanager/constants/transfers.ts | 1 - src/modules/filemanager/hooks/useTransfers.ts | 188 +++++++++++------- src/modules/filemanager/utils/download.ts | 146 ++++++++++---- src/pages/filemanager/index.tsx | 9 +- 5 files changed, 223 insertions(+), 129 deletions(-) diff --git a/src/modules/filemanager/components/VersionHistoryModal/VersionHistoryModal.tsx b/src/modules/filemanager/components/VersionHistoryModal/VersionHistoryModal.tsx index 327ce7b..dfc62d1 100644 --- a/src/modules/filemanager/components/VersionHistoryModal/VersionHistoryModal.tsx +++ b/src/modules/filemanager/components/VersionHistoryModal/VersionHistoryModal.tsx @@ -7,7 +7,6 @@ import HistoryIcon from 'remixicon-react/HistoryLineIcon' import { Context as FMContext } from '../../../../providers/FileManager' import { TOOLTIPS } from '../../constants/tooltips' import { ActionTag, DownloadProgress, TrackDownloadProps } from '../../constants/transfers' -import { useTransfers } from '../../hooks/useTransfers' import { ConflictAction, useUploadConflictDialog } from '../../hooks/useUploadConflictDialog' import { verifyDriveSpace } from '../../utils/bee' import { indexStrToBigint, truncateNameMiddle } from '../../utils/common' @@ -31,15 +30,12 @@ type RenameConfirmState = { interface VersionHistoryModalProps { fileInfo: FileInfo onCancelClick: () => void - onDownload?: (props: TrackDownloadProps) => (dp: DownloadProgress) => void + onDownload: (props: TrackDownloadProps) => (dp: DownloadProgress) => void } export function VersionHistoryModal({ fileInfo, onCancelClick, onDownload }: VersionHistoryModalProps): ReactElement { const { fm, files, currentDrive, currentStamp, refreshStamp } = useContext(FMContext) - const localTransfers = useTransfers({}) - const trackDownload = onDownload ?? localTransfers.trackDownload - const [openConflict, conflictPortal] = useUploadConflictDialog() const modalRoot = document.querySelector('.fm-main') || document.body @@ -345,7 +341,7 @@ export function VersionHistoryModal({ fileInfo, onCancelClick, onDownload }: Ver versions={!error && !loading ? pageVersions : []} headFi={fileInfo} restoreVersion={restoreVersion} - onDownload={trackDownload} + onDownload={onDownload} /> diff --git a/src/modules/filemanager/constants/transfers.ts b/src/modules/filemanager/constants/transfers.ts index d07ea46..83d7a64 100644 --- a/src/modules/filemanager/constants/transfers.ts +++ b/src/modules/filemanager/constants/transfers.ts @@ -39,7 +39,6 @@ export enum FileAction { export enum DownloadState { InProgress = 'in-progress', - Completed = 'completed', Cancelled = 'cancelled', Error = 'error', } diff --git a/src/modules/filemanager/hooks/useTransfers.ts b/src/modules/filemanager/hooks/useTransfers.ts index ea3244d..43c59bd 100644 --- a/src/modules/filemanager/hooks/useTransfers.ts +++ b/src/modules/filemanager/hooks/useTransfers.ts @@ -22,6 +22,7 @@ import { ConflictAction, useUploadConflictDialog } from './useUploadConflictDial const SAMPLE_WINDOW_MS = 500 const ETA_SMOOTHING = 0.3 const MAX_UPLOAD_FILES = 10 +const MAX_PARALLEL_UPLOAD_FILES = 2 const ABORT_EVENT = 'abort' type ResolveResult = { @@ -156,6 +157,19 @@ const updateTransferItems = (items: T[], uuid: string, u }) } +const getUploadCancelPromise = (signal: AbortSignal | undefined): { promise: Promise; cleanup: () => void } => { + let reject: (reason?: Error) => void + const abortHandler = () => { + reject(new Error('Upload cancelled')) + } + const promise = new Promise((_, rej) => { + reject = rej + signal?.addEventListener(ABORT_EVENT, abortHandler) + }) + + return { promise, cleanup: () => signal?.removeEventListener(ABORT_EVENT, abortHandler) } +} + const createTransferItem = ( uuid: string, name: string, @@ -185,10 +199,10 @@ export function useTransfers({ setErrorMessage }: TransferProps) { const { beeApi } = useContext(SettingsContext) const [openConflict, conflictPortal] = useUploadConflictDialog() - const isMountedRef = useRef(true) + const isMountedRef = useRef(true) const uploadAbortsRef = useRef(new AbortManager()) const uploadTaskQueueRef = useRef([]) - const runningRef = useRef(false) + const runningRef = useRef(false) const cancelledQueuedRef = useRef>(new Set()) const cancelledUploadingRef = useRef>(new Set()) const cancelledDownloadingRef = useRef>(new Set()) @@ -221,11 +235,10 @@ export function useTransfers({ setErrorMessage }: TransferProps) { const idx = prev.findIndex(p => p.uuid === uuid && p.status !== TransferStatus.Done) const base = createTransferItem(uuid, name, size, kind, driveName, TransferStatus.Queued) - if (idx !== -1) { - clearAllFlagsFor(uuid) - } - if (idx === -1) return [...prev, base] + + clearAllFlagsFor(uuid) + const copy = [...prev] copy[idx] = base @@ -378,14 +391,6 @@ export function useTransfers({ setErrorMessage }: TransferProps) { topic: task.isReplace ? task.replaceTopic : undefined, } - const progressCb = trackUpload( - task.uuid, - task.finalName, - task.prettySize, - task.isReplace ? FileTransferType.Update : FileTransferType.Upload, - taskDrive.name, - ) - safeSetState( isMountedRef, setUploadItems, @@ -400,33 +405,29 @@ export function useTransfers({ setErrorMessage }: TransferProps) { uploadAbortsRef.current.create(task.uuid) const signal = uploadAbortsRef.current.getSignal(task.uuid) - let reject: (reason?: Error) => void - const abortHandler = () => { - reject(new Error('Upload cancelled')) - } - - const checkCancellation = new Promise((_, rej) => { - reject = rej - signal?.addEventListener(ABORT_EVENT, abortHandler) - }) + const { promise: checkCancellation, cleanup: cleanupCancelPromise } = getUploadCancelPromise(signal) try { if (signal?.aborted) { throw new Error('Upload cancelled') } + const onUploadProgress = trackUpload( + task.uuid, + task.finalName, + task.prettySize, + task.isReplace ? FileTransferType.Update : FileTransferType.Upload, + taskDrive.name, + ) + const uploadPromise = fm.upload( taskDrive, - { ...info, onUploadProgress: progressCb }, + { ...info, onUploadProgress }, { actHistoryAddress: task.isReplace ? task.replaceHistory : undefined }, { signal }, ) await Promise.race([uploadPromise, checkCancellation]) - - if (currentStamp) { - await refreshStamp(currentStamp.batchID.toString()) - } } catch (error) { const wasCancelled = cancelledUploadingRef.current.has(task.uuid) || signal?.aborted @@ -445,7 +446,7 @@ export function useTransfers({ setErrorMessage }: TransferProps) { }), ) } finally { - signal?.removeEventListener(ABORT_EVENT, abortHandler) + cleanupCancelPromise() const wasCancelled = cancelledUploadingRef.current.has(task.uuid) || signal?.aborted @@ -456,7 +457,7 @@ export function useTransfers({ setErrorMessage }: TransferProps) { } } }, - [fm, files, currentStamp, trackUpload, refreshStamp, setShowError, setErrorMessage], + [fm, files, trackUpload, setShowError, setErrorMessage], ) const trackDownload = useCallback((props: TrackDownloadProps) => { @@ -686,37 +687,65 @@ export function useTransfers({ setErrorMessage }: TransferProps) { if (runningRef.current) return runningRef.current = true - while (uploadTaskQueueRef.current.length > 0) { - const task = uploadTaskQueueRef.current[0] + const processNextTask = async (): Promise => { + while (uploadTaskQueueRef.current.length > 0) { + const task = uploadTaskQueueRef.current.shift() - if (!task) break + if (!task) break - if (cancelledQueuedRef.current.has(task.uuid)) { - safeSetState( - isMountedRef, - setUploadItems, - )(prev => updateTransferItems(prev, task.uuid, { status: TransferStatus.Cancelled })) - cancelledQueuedRef.current.delete(task.uuid) - } else { - await executeUploadTask(task) + if (cancelledQueuedRef.current.has(task.uuid)) { + safeSetState( + isMountedRef, + setUploadItems, + )(prev => updateTransferItems(prev, task.uuid, { status: TransferStatus.Cancelled })) + cancelledQueuedRef.current.delete(task.uuid) + } else { + await executeUploadTask(task) + } } - - uploadTaskQueueRef.current.shift() } + const workers: Promise[] = [] + for (let i = 0; i < MAX_PARALLEL_UPLOAD_FILES; i++) { + workers.push(processNextTask()) + } + + await Promise.all(workers) + runningRef.current = false // Race guard: uploadFiles may have appended tasks and called runUploadQueue() again if (uploadTaskQueueRef.current.length > 0) { - runUploadQueue() + void runUploadQueue() } - }, [executeUploadTask]) - const uploadFiles = useCallback( - (picked: FileList | File[]): void => { - const filesArr = Array.from(picked) + if (currentStamp) { + await refreshStamp(currentStamp.batchID.toString()) + } + }, [currentStamp, executeUploadTask, refreshStamp]) - if (filesArr.length === 0 || !fm || !currentDrive || !currentStamp) return + const verifyUploadConditions = useCallback( + async (filesArr: File[]): Promise => { + if (filesArr.length === 0) { + setErrorMessage?.('Nothing to upload.') + setShowError(true) + + return false + } + + if (!fm || !currentDrive) { + setErrorMessage?.('File manager is not ready or no drive is selected.') + setShowError(true) + + return false + } + + if (!currentStamp || !currentStamp.usable) { + setErrorMessage?.('Stamp is not usable.') + setShowError(true) + + return false + } const currentlyQueued = uploadTaskQueueRef.current.length const newFilesCount = filesArr.length @@ -728,36 +757,41 @@ export function useTransfers({ setErrorMessage }: TransferProps) { ) setShowError(true) + return false + } + + if (beeApi) { + const batchID = currentStamp.batchID + const stampValid = await validateStampStillExists(beeApi, batchID) + + if (!stampValid) { + setErrorMessage?.( + `The selected stamp ${batchID.toString().slice(0, 4)} is no longer valid or has been deleted. Please select a different stamp.`, + ) + setShowError(true) + + return false + } + } + + return true + }, + [fm, currentStamp, currentDrive, beeApi, setShowError, setErrorMessage], + ) + + const uploadFiles = useCallback( + async (picked: FileList | File[]): Promise => { + const filesArr = Array.from(picked) + + if (!(await verifyUploadConditions(filesArr))) { return } - void (async () => { - if (!currentStamp || !currentStamp.usable) { - setErrorMessage?.('Stamp is not usable.') - setShowError(true) - - return - } - - if (beeApi) { - const stampValid = await validateStampStillExists(beeApi, currentStamp.batchID) - - if (!stampValid) { - setErrorMessage?.( - 'The selected stamp is no longer valid or has been deleted. Please select a different stamp.', - ) - setShowError(true) - - return - } - } - - const tasks = await preflight(filesArr) - uploadTaskQueueRef.current = uploadTaskQueueRef.current.concat(tasks) - runUploadQueue() - })() + const tasks = await preflight(filesArr) + uploadTaskQueueRef.current = uploadTaskQueueRef.current.concat(tasks) + runUploadQueue() }, - [fm, currentStamp, currentDrive, beeApi, setShowError, setErrorMessage, preflight, runUploadQueue], + [verifyUploadConditions, preflight, runUploadQueue], ) const cancelOrDismissUpload = useCallback( @@ -865,13 +899,13 @@ export function useTransfers({ setErrorMessage }: TransferProps) { }, []) return { - uploadFiles, isUploading, uploadItems, - trackDownload, isDownloading, downloadItems, conflictPortal, + uploadFiles, + trackDownload, cancelOrDismissUpload, cancelOrDismissDownload, dismissAllUploads, diff --git a/src/modules/filemanager/utils/download.ts b/src/modules/filemanager/utils/download.ts index 262ab7a..151d926 100644 --- a/src/modules/filemanager/utils/download.ts +++ b/src/modules/filemanager/utils/download.ts @@ -6,6 +6,8 @@ import { AbortManager } from './abortManager' import { isDirectoryPickerSupported, isPickerSupported } from './fileOperations' import { guessMime, VIEWERS } from './view' +const DefaultDownloadFolder = 'downloads' + const downloadAborts = new AbortManager() enum Errors { @@ -148,10 +150,7 @@ const isUserCancellation = (error: unknown): boolean => { return errName === Errors.AbortError || errName === Errors.NotAllowedError || errName === Errors.SecurityError } -const getSingleFileHandle = async ( - infoWithId: FileInfoWithUUID, - defaultDownloadFolder: string, -): Promise => { +const getSingleFileHandle = async (infoWithId: FileInfoWithUUID): Promise => { const { mime, ext } = guessMime(infoWithId.info.name, infoWithId.info.customMetadata) const pickerOptions: { @@ -160,7 +159,7 @@ const getSingleFileHandle = async ( types?: Array<{ accept: Record }> } = { suggestedName: infoWithId.info.name, - startIn: defaultDownloadFolder, + startIn: DefaultDownloadFolder, } if (ext) { @@ -171,24 +170,24 @@ const getSingleFileHandle = async ( // eslint-disable-next-line @typescript-eslint/no-explicit-any const handle = (await (window as any).showSaveFilePicker(pickerOptions)) as FileSystemFileHandle - return [{ infoWithId, handle }] + return { infoWithId, handle } } catch (error: unknown) { - return isUserCancellation(error) ? [{ infoWithId, cancelled: true }] : undefined + return isUserCancellation(error) ? { infoWithId, cancelled: true } : undefined } } const getMultipleFileHandles = async ( infoWithIdList: FileInfoWithUUID[], - defaultDownloadFolder: string, ): Promise => { if (!isDirectoryPickerSupported()) { const handles: FileInfoWithHandle[] = [] for (const info of infoWithIdList) { - const result = await getSingleFileHandle(info, defaultDownloadFolder) + const result = await getSingleFileHandle(info) if (!result) return undefined - handles.push(result[0]) + + handles.push(result) } return handles @@ -198,7 +197,7 @@ const getMultipleFileHandles = async ( // eslint-disable-next-line @typescript-eslint/no-explicit-any const dirHandle = (await (window as any).showDirectoryPicker({ mode: 'readwrite', - startIn: defaultDownloadFolder, + startIn: DefaultDownloadFolder, })) as FileSystemDirectoryHandle const handles: FileInfoWithHandle[] = [] @@ -224,16 +223,16 @@ const getMultipleFileHandles = async ( } } -const getFileHandles = (infoWithIdList: FileInfoWithUUID[]): Promise => { - const defaultDownloadFolder = 'downloads' - +const getFileHandles = async (infoWithIdList: FileInfoWithUUID[]): Promise => { if (!isPickerSupported()) return Promise.resolve(infoWithIdList.map(infoWithId => ({ infoWithId }))) if (infoWithIdList.length === 1) { - return getSingleFileHandle(infoWithIdList[0], defaultDownloadFolder) + const fh = await getSingleFileHandle(infoWithIdList[0]) + + return fh ? [fh] : undefined } - return getMultipleFileHandles(infoWithIdList, defaultDownloadFolder) + return getMultipleFileHandles(infoWithIdList) } const downloadToDisk = async ( @@ -258,20 +257,25 @@ const downloadToDisk = async ( } } +interface BlobDownloadResult { + success: boolean + cancelled: boolean +} + const downloadToBlob = async ( streams: ReadableStream[], info: FileInfo, onDownloadProgress?: (progress: DownloadProgress) => void, isOpenWindow?: boolean, signal?: AbortSignal, -): Promise => { +): Promise => { try { for (const stream of streams) { const { mime } = guessMime(info.name, info.customMetadata) const blob = await streamToBlob(stream, mime, onDownloadProgress, signal) if (!blob) { - return false + return { success: false, cancelled: false } } const url = URL.createObjectURL(blob) @@ -282,50 +286,104 @@ const downloadToBlob = async ( } if (!opened) { + if (isOpenWindow && isPickerSupported()) { + const result = await saveBlobWithPicker(blob, info, onDownloadProgress, signal) + URL.revokeObjectURL(url) + + return result + } + downloadFromUrl(url, info.name) } } - return true + return { success: true, cancelled: false } } catch (error: unknown) { if ((error as { name?: string }).name !== Errors.AbortError) { // eslint-disable-next-line no-console console.error('Error during download and open: ', error) } - return false + return { success: false, cancelled: false } } } const openNewWindow = (name: string, mime: string, url: string): boolean => { const viewer = VIEWERS.find(v => v.test(mime)) + + if (!viewer) return false + const win = window.open('', '_blank') - if (viewer && win) { - viewer.render(win, url, mime, name) + if (!win) return false - return true + try { + viewer.render(win, url, mime, name) + } catch (err: unknown) { + win.close() + // eslint-disable-next-line no-console + console.error('Failed to render file in a new window: ', err) + + return false } - win?.close() - - return false + return true } +const saveBlobWithPicker = async ( + blob: Blob, + info: FileInfo, + onDownloadProgress?: (progress: DownloadProgress) => void, + signal?: AbortSignal, +): Promise => { + const infoWithId: FileInfoWithUUID = { uuid: '', info } + + try { + if (signal?.aborted) { + throw new DOMException('Aborted', Errors.AbortError) + } + + const fh = await getSingleFileHandle(infoWithId) + + if (!fh || !fh.handle) { + return { success: false, cancelled: false } + } + + if (fh.cancelled) { + return { success: false, cancelled: true } + } + + await processStream(blob.stream(), fh.handle, onDownloadProgress, signal) + + return { success: true, cancelled: false } + } catch (err: unknown) { + if (isUserCancellation(err)) { + return { success: false, cancelled: true } + } + + // eslint-disable-next-line no-console + console.error('Failed to save blob using file picker: ', err) + + return { success: false, cancelled: false } + } +} + +const RevokeUrlTimeout = 1000 + const downloadFromUrl = (url: string, fileName: string): void => { const a = document.createElement('a') a.href = url a.download = fileName document.body.appendChild(a) a.click() - window.URL.revokeObjectURL(url) + window.setTimeout(() => window.URL.revokeObjectURL(url), RevokeUrlTimeout) document.body.removeChild(a) } export const startDownloadingQueue = async ( fm: FileManager, infoListWithIds: FileInfoWithUUID[], - trackers?: Array<(progress: DownloadProgress) => void>, + trackers: Array<(progress: DownloadProgress) => void>, isOpenWindow?: boolean, ): Promise => { if (!infoListWithIds.length || (trackers && trackers.length !== infoListWithIds.length)) return @@ -339,7 +397,7 @@ export const startDownloadingQueue = async ( await Promise.all( fileHandles.map(async (fh, i) => { - const tracker = trackers ? trackers[i] : undefined + const tracker = trackers[i] const uuid = fh.infoWithId.uuid createDownloadAbort(uuid) @@ -347,7 +405,7 @@ export const startDownloadingQueue = async ( try { if (fh.cancelled) { - tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Cancelled }) + tracker({ progress: 0, isDownloading: false, state: DownloadState.Cancelled }) return } @@ -359,23 +417,29 @@ export const startDownloadingQueue = async ( if (!dataStreams || dataStreams.length === 0) { // eslint-disable-next-line no-console console.error(`No data streams returned for ${fh.infoWithId.info.name}`) - tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Error }) + tracker({ progress: 0, isDownloading: false, state: DownloadState.Error }) return } let success = false + let userCancelled = false if (isOpenWindow || !fh.handle) { - success = await downloadToBlob(dataStreams, fh.infoWithId.info, tracker, isOpenWindow, signal) + const { success: saved, cancelled } = await downloadToBlob( + dataStreams, + fh.infoWithId.info, + tracker, + isOpenWindow, + signal, + ) + + success = saved + userCancelled = cancelled } else { success = await downloadToDisk(dataStreams, fh.handle, tracker, signal) } - if (!tracker) { - return - } - if (success) { const size = fh.infoWithId.info.customMetadata?.size const finalProgress = size ? Number(size) : 0 @@ -385,18 +449,22 @@ export const startDownloadingQueue = async ( } if (!signal?.aborted) { - tracker({ progress: 0, isDownloading: false, state: DownloadState.Error }) + tracker({ + progress: 0, + isDownloading: false, + state: userCancelled ? DownloadState.Cancelled : DownloadState.Error, + }) } } catch (error: unknown) { const isAbortError = (error as { name?: string }).name === Errors.AbortError if (!isAbortError) { - tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Error }) + tracker({ progress: 0, isDownloading: false, state: DownloadState.Error }) // eslint-disable-next-line no-console console.error('download queue error: ', error) } else { - tracker?.({ progress: 0, isDownloading: false, state: DownloadState.Cancelled }) + tracker({ progress: 0, isDownloading: false, state: DownloadState.Cancelled }) } } finally { downloadAborts.abort(uuid) diff --git a/src/pages/filemanager/index.tsx b/src/pages/filemanager/index.tsx index a88215d..a047bb6 100644 --- a/src/pages/filemanager/index.tsx +++ b/src/pages/filemanager/index.tsx @@ -186,12 +186,9 @@ export function FileManagerPage(): ReactElement { }, []) useEffect(() => { - if (status.all !== CheckState.OK) { - setShowConnectionError(true) - } else { - setShowConnectionError(false) - } - }, [status.all]) + const isApiError = status.apiConnection.checkState !== CheckState.OK || !status.apiConnection.isEnabled + setShowConnectionError(isApiError) + }, [status.apiConnection]) useEffect(() => { if (!beeApi) {