From ca35e5557bbd2d361fbe389b172022a0e02e1910 Mon Sep 17 00:00:00 2001 From: Villena Guillaume Date: Fri, 1 Sep 2023 18:00:51 +0200 Subject: [PATCH] feat(web): Improved assets upload (#3850) * Improved asset upload algorithm. - Upload Queue: New process algorithm - Upload Queue: Concurrency correctly respected when dragging / adding multiple group of files to the queue - Upload Task: Add more information about progress (upload speed and remaining time) - Upload Panel: Add more information to about the queue status (Remaining, Errors, Duplicated, Uploaded) - Error recovery: asset information are kept in the queue to give the user a chance to read the error message - Error recovery: on error allow the user to retry the upload or hide the error / all errors * Support "live" editing of the upload concurrency * Fixed some issues * Reformat * fix: merge, linting, dark mode, upload to share --------- Co-authored-by: Jason Rasmussen --- web/src/app.css | 2 +- .../buttons/circle-icon-button.svelte | 3 +- .../upload-asset-preview.svelte | 141 ++++++++++++----- .../shared-components/upload-panel.svelte | 148 ++++++++++++------ web/src/lib/models/upload-asset.ts | 17 +- web/src/lib/stores/upload.ts | 75 +++++++-- web/src/lib/utils/executor-queue.spec.ts | 54 +++++++ web/src/lib/utils/executor-queue.ts | 69 ++++++++ web/src/lib/utils/file-uploader.ts | 137 ++++++++-------- web/tailwind.config.cjs | 6 + 10 files changed, 483 insertions(+), 169 deletions(-) create mode 100644 web/src/lib/utils/executor-queue.spec.ts create mode 100644 web/src/lib/utils/executor-queue.ts diff --git a/web/src/app.css b/web/src/app.css index eb55ec4c86..c38700ba9d 100644 --- a/web/src/app.css +++ b/web/src/app.css @@ -66,7 +66,7 @@ input:focus-visible { @layer utilities { .immich-form-input { - @apply rounded-xl bg-slate-200 p-4 text-sm focus:border-immich-primary disabled:cursor-not-allowed disabled:bg-gray-400 disabled:text-gray-200 dark:bg-gray-600 dark:text-immich-dark-fg dark:disabled:bg-gray-800; + @apply rounded-xl bg-slate-200 px-3 py-3 text-sm focus:border-immich-primary disabled:cursor-not-allowed disabled:bg-gray-400 disabled:text-gray-200 dark:bg-gray-600 dark:text-immich-dark-fg dark:disabled:bg-gray-800; } .immich-form-label { diff --git a/web/src/lib/components/elements/buttons/circle-icon-button.svelte b/web/src/lib/components/elements/buttons/circle-icon-button.svelte index 1ade341791..45cfbd259f 100644 --- a/web/src/lib/components/elements/buttons/circle-icon-button.svelte +++ b/web/src/lib/components/elements/buttons/circle-icon-button.svelte @@ -4,6 +4,7 @@ export let logo: typeof Icon; export let backgroundColor = 'transparent'; export let hoverColor = '#e2e7e9'; + export let padding = '3'; export let size = '24'; export let title = ''; export let isOpacity = false; @@ -16,7 +17,7 @@ style:background-color={backgroundColor} style:--immich-icon-button-hover-color={hoverColor} class:dark:text-immich-dark-fg={!forceDark} - class="flex place-content-center place-items-center rounded-full p-3 transition-all + class="flex place-content-center place-items-center rounded-full p-{padding} transition-all {isOpacity ? 'hover:bg-immich-bg/30' : 'immich-circle-icon-button hover:dark:text-immich-dark-gray'} {forceDark && 'hover:text-black'} {hideMobile && 'hidden sm:flex'}" diff --git a/web/src/lib/components/shared-components/upload-asset-preview.svelte b/web/src/lib/components/shared-components/upload-asset-preview.svelte index d4219b3363..71ba7e3fb1 100644 --- a/web/src/lib/components/shared-components/upload-asset-preview.svelte +++ b/web/src/lib/components/shared-components/upload-asset-preview.svelte @@ -1,65 +1,128 @@
-
- {#if showFallbackImage} -
- +
+
+ {#if showFallbackImage || true} +
+ +
+ {:else} + { + URL.revokeObjectURL(previewURL); + }} + on:error={() => { + URL.revokeObjectURL(previewURL); + showFallbackImage = true; + }} + src={previewURL} + alt="Preview of asset" + class="h-[65px] w-[65px] rounded-bl-lg rounded-tl-lg object-cover" + draggable="false" + /> + {/if} + +
+

+ .{getFilenameExtension(uploadAsset.file.name)} +

- {:else} - { - URL.revokeObjectURL(previewURL); - }} - on:error={() => { - URL.revokeObjectURL(previewURL); - showFallbackImage = true; - }} - src={previewURL} - alt="Preview of asset" - class="h-[70px] w-[70px] rounded-bl-lg rounded-tl-lg object-cover" - draggable="false" +
+
+ - {/if} -
-

- .{getFilenameExtension(uploadAsset.file.name)} -

+ {#if uploadAsset.state === UploadState.STARTED} +
+

+ {#if uploadAsset.message} + {uploadAsset.message} + {:else} + {uploadAsset.progress}/100 - {asByteUnitString(uploadAsset.speed || 0, $locale)}/s - {uploadAsset.eta}s + {/if} +

+ {:else if uploadAsset.state === UploadState.PENDING} +
+

Pending

+ {:else if uploadAsset.state === UploadState.ERROR} +
+

Error

+ {:else if uploadAsset.state === UploadState.DUPLICATED} +
+

+ Skipped + {#if uploadAsset.message} ({uploadAsset.message}){/if} +

+ {:else if uploadAsset.state === UploadState.DONE} +
+

+ Uploaded + {#if uploadAsset.message} ({uploadAsset.message}){/if} +

+ {/if} +
+ {#if uploadAsset.state === UploadState.ERROR} +
+ + +
+ {/if}
-
- - -
-
-

- {uploadAsset.progress}/100 + {#if uploadAsset.state === UploadState.ERROR} +

+

+ {uploadAsset.error}

-
+ {/if}
diff --git a/web/src/lib/components/shared-components/upload-panel.svelte b/web/src/lib/components/shared-components/upload-panel.svelte index e2e46265d2..4ca4c91810 100644 --- a/web/src/lib/components/shared-components/upload-panel.svelte +++ b/web/src/lib/components/shared-components/upload-panel.svelte @@ -1,84 +1,129 @@ -{#if isUploading} +{#if $hasError || $isUploading}
{ const errorInfo = - errorCount > 0 ? `Upload completed with ${errorCount} error${errorCount > 1 ? 's' : ''}` : 'Upload success'; - const type = errorCount > 0 ? NotificationType.Warning : NotificationType.Info; + $errorCounter > 0 + ? `Upload completed with ${$errorCounter} error${$errorCounter > 1 ? 's' : ''}` + : 'Upload success'; + const type = $errorCounter > 0 ? NotificationType.Warning : NotificationType.Info; notificationController.show({ message: `${errorInfo}, refresh the page to see new upload assets`, type, }); - uploadAssetsStore.errorCounter.set(0); - - if (duplicateCount > 0) { + if ($duplicateCounter > 0) { notificationController.show({ - message: `Skipped ${duplicateCount} duplicate picture${duplicateCount > 1 ? 's' : ''}`, + message: `Skipped ${$duplicateCounter} duplicate picture${$duplicateCounter > 1 ? 's' : ''}`, type: NotificationType.Warning, }); - uploadAssetsStore.duplicateCounter.set(0); } + + uploadAssetsStore.resetStore(); }} class="absolute bottom-6 right-6 z-[10000]" > {#if showDetail}
-

UPLOADING {$uploadAssetsStore.length}

- +
+

+ Remaining {$remainingUploads} - Processed {$successCounter + $errorCounter}/{$totalUploadCounter} +

+

+ Uploaded {$successCounter} - Error + {$errorCounter} + - Duplicates {$duplicateCounter} +

+
+
+
+ (showOptions = !showOptions)} + /> + (showDetail = false)} + /> +
+ {#if $hasError} + uploadAssetsStore.dismissErrors()} + /> + {/if} +
- -
- {#each $uploadAssetsStore as uploadAsset} - {#key uploadAsset.id} - - {/key} + {#if showOptions} +
+
+ +
+ (uploadExecutionQueue.concurrency = concurrency)} + /> +
+ {/if} +
+ {#each $uploadAssetsStore as uploadAsset (uploadAsset.id)} + {/each}
@@ -89,15 +134,24 @@ on:click={() => (showDetail = true)} class="absolute -left-4 -top-4 flex h-10 w-10 place-content-center place-items-center rounded-full bg-immich-primary p-5 text-xs text-gray-200" > - {$uploadAssetsStore.length} + {$remainingUploads} + {#if $hasError} + + {/if}
diff --git a/web/src/lib/models/upload-asset.ts b/web/src/lib/models/upload-asset.ts index 1b8bb0a256..4fd56bf720 100644 --- a/web/src/lib/models/upload-asset.ts +++ b/web/src/lib/models/upload-asset.ts @@ -1,5 +1,20 @@ +export enum UploadState { + PENDING, + STARTED, + DONE, + ERROR, + DUPLICATED, +} + export type UploadAsset = { id: string; file: File; - progress: number; + albumId?: string; + progress?: number; + state?: UploadState; + startDate?: number; + eta?: number; + speed?: number; + error?: unknown; + message?: string; }; diff --git a/web/src/lib/stores/upload.ts b/web/src/lib/stores/upload.ts index 1efa0be024..37733fd9c5 100644 --- a/web/src/lib/stores/upload.ts +++ b/web/src/lib/stores/upload.ts @@ -1,47 +1,102 @@ import { derived, writable } from 'svelte/store'; -import type { UploadAsset } from '../models/upload-asset'; +import { UploadState, type UploadAsset } from '../models/upload-asset'; function createUploadStore() { const uploadAssets = writable>([]); + const duplicateCounter = writable(0); - const errorCounter = writable(0); + const successCounter = writable(0); + const totalUploadCounter = writable(0); const { subscribe } = uploadAssets; const isUploading = derived(uploadAssets, ($uploadAssets) => { - return $uploadAssets.length > 0 ? true : false; + return $uploadAssets.length > 0; }); + const errorsAssets = derived(uploadAssets, (a) => a.filter((e) => e.state === UploadState.ERROR)); + const errorCounter = derived(errorsAssets, (values) => values.length); + const hasError = derived(errorCounter, (values) => values > 0); + const remainingUploads = derived( + uploadAssets, + (values) => values.filter((a) => a.state === UploadState.PENDING || a.state === UploadState.STARTED).length, + ); const addNewUploadAsset = (newAsset: UploadAsset) => { - uploadAssets.update((currentSet) => [...currentSet, newAsset]); + totalUploadCounter.update((c) => c + 1); + uploadAssets.update((assets) => [ + ...assets, + { + ...newAsset, + speed: 0, + state: UploadState.PENDING, + progress: 0, + eta: 0, + }, + ]); }; - const updateProgress = (id: string, progress: number) => { + const updateProgress = (id: string, loaded: number, total: number) => { + updateAssetMap(id, (v) => { + const uploadSpeed = v.startDate ? loaded / ((Date.now() - v.startDate) / 1000) : 0; + return { + ...v, + progress: Math.floor((loaded / total) * 100), + speed: uploadSpeed, + eta: Math.ceil((total - loaded) / uploadSpeed), + }; + }); + }; + + const markStarted = (id: string) => { + updateAsset(id, { + state: UploadState.STARTED, + startDate: Date.now(), + }); + }; + + const updateAssetMap = (id: string, mapper: (assets: UploadAsset) => UploadAsset) => { uploadAssets.update((uploadingAssets) => { return uploadingAssets.map((asset) => { if (asset.id == id) { - return { - ...asset, - progress: progress, - }; + return mapper(asset); } - return asset; }); }); }; + const updateAsset = (id: string, partialObj: Partial) => { + updateAssetMap(id, (v) => ({ ...v, ...partialObj })); + }; + const removeUploadAsset = (id: string) => { uploadAssets.update((uploadingAsset) => uploadingAsset.filter((a) => a.id != id)); }; + const dismissErrors = () => uploadAssets.update((value) => value.filter((e) => e.state !== UploadState.ERROR)); + + const resetStore = () => { + uploadAssets.set([]); + duplicateCounter.set(0); + successCounter.set(0); + totalUploadCounter.set(0); + }; + return { subscribe, errorCounter, duplicateCounter, + successCounter, + totalUploadCounter, + remainingUploads, + hasError, + dismissErrors, isUploading, + resetStore, addNewUploadAsset, + markStarted, updateProgress, + updateAsset, removeUploadAsset, }; } diff --git a/web/src/lib/utils/executor-queue.spec.ts b/web/src/lib/utils/executor-queue.spec.ts new file mode 100644 index 0000000000..aea847e9f2 --- /dev/null +++ b/web/src/lib/utils/executor-queue.spec.ts @@ -0,0 +1,54 @@ +import { ExecutorQueue } from '$lib/utils/executor-queue'; + +describe('Executor Queue test', function () { + it('should run all promises', async function () { + const eq = new ExecutorQueue({ concurrency: 1 }); + const n1 = await eq.addTask(() => Promise.resolve(10)); + expect(n1).toBe(10); + const n2 = await eq.addTask(() => Promise.resolve(11)); + expect(n2).toBe(11); + const n3 = await eq.addTask(() => Promise.resolve(12)); + expect(n3).toBe(12); + }); + + it('should respect concurrency parameter', function () { + jest.useFakeTimers(); + const eq = new ExecutorQueue({ concurrency: 3 }); + + const finished = jest.fn(); + const started = jest.fn(); + + const timeoutPromiseBuilder = (delay: number, id: string) => + new Promise((resolve) => { + console.log('Task is running: ', id); + started(); + setTimeout(() => { + console.log('Finished ' + id + ' after', delay, 'ms'); + finished(); + resolve(undefined); + }, delay); + }); + + // The first 3 should be finished within 200ms (concurrency 3) + eq.addTask(() => timeoutPromiseBuilder(100, 'T1')); + eq.addTask(() => timeoutPromiseBuilder(200, 'T2')); + eq.addTask(() => timeoutPromiseBuilder(150, 'T3')); + // The last task will be executed after 200ms and will finish at 400ms + eq.addTask(() => timeoutPromiseBuilder(200, 'T4')); + + expect(finished).not.toBeCalled(); + expect(started).toHaveBeenCalledTimes(3); + + jest.advanceTimersByTime(100); + expect(finished).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(250); + expect(finished).toHaveBeenCalledTimes(3); + // expect(started).toHaveBeenCalledTimes(4) + + //TODO : fix The test ... + + jest.runAllTimers(); + jest.useRealTimers(); + }); +}); diff --git a/web/src/lib/utils/executor-queue.ts b/web/src/lib/utils/executor-queue.ts new file mode 100644 index 0000000000..0816befd6d --- /dev/null +++ b/web/src/lib/utils/executor-queue.ts @@ -0,0 +1,69 @@ +interface Options { + concurrency: number; +} + +type Runnable = () => Promise; + +export class ExecutorQueue { + private queue: Array = []; + private running = 0; + private _concurrency: number; + + constructor(options?: Options) { + this._concurrency = options?.concurrency || 2; + } + + get concurrency() { + return this._concurrency; + } + + set concurrency(concurrency: number) { + if (concurrency < 1) { + return; + } + + this._concurrency = concurrency; + + const v = concurrency - this.running; + if (v > 0) { + [...new Array(this._concurrency)].forEach(() => this.tryRun()); + } + } + + addTask(task: () => Promise): Promise { + return new Promise((resolve, reject) => { + // Add a custom task that wrap the original one; + this.queue.push(async () => { + try { + this.running++; + const result = task(); + resolve(await result); + } catch (e) { + reject(e); + } finally { + this.taskFinished(); + } + }); + // Then run it if possible ! + this.tryRun(); + }); + } + + private taskFinished(): void { + this.running--; + this.tryRun(); + } + + private tryRun() { + if (this.running >= this.concurrency) { + return; + } + + const runnable = this.queue.shift(); + if (!runnable) { + return; + } + + runnable(); + } +} diff --git a/web/src/lib/utils/file-uploader.ts b/web/src/lib/utils/file-uploader.ts index dab80fd7cf..2cf8dec652 100644 --- a/web/src/lib/utils/file-uploader.ts +++ b/web/src/lib/utils/file-uploader.ts @@ -1,11 +1,14 @@ import { uploadAssetsStore } from '$lib/stores/upload'; import { addAssetsToAlbum } from '$lib/utils/asset-utils'; import { api, AssetFileUploadResponseDto } from '@api'; -import axios from 'axios'; import { notificationController, NotificationType } from './../components/shared-components/notification/notification'; +import { UploadState } from '$lib/models/upload-asset'; +import { ExecutorQueue } from '$lib/utils/executor-queue'; let _extensions: string[]; +export const uploadExecutionQueue = new ExecutorQueue({ concurrency: 2 }); + const getExtensions = async () => { if (!_extensions) { const { data } = await api.serverInfoApi.getSupportedMediaTypes(); @@ -42,93 +45,87 @@ export const openFileUploadDialog = async (albumId: string | undefined = undefin }); }; -export const fileUploadHandler = async (files: File[], albumId: string | undefined = undefined) => { +export const fileUploadHandler = async (files: File[], albumId: string | undefined = undefined): Promise => { const extensions = await getExtensions(); - const iterable = { - files: files.filter((file) => extensions.some((ext) => file.name.toLowerCase().endsWith(ext)))[Symbol.iterator](), - - async *[Symbol.asyncIterator]() { - for (const file of this.files) { - yield fileUploader(file, albumId); - } - }, - }; - - const concurrency = 2; - // TODO: use Array.fromAsync instead when it's available universally. - return Promise.all([...Array(concurrency)].map(() => fromAsync(iterable))).then((res) => res.flat()); -}; - -// polyfill for Array.fromAsync. -// -// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/fromAsync -const fromAsync = async function (iterable: AsyncIterable) { - const result = []; - for await (const value of iterable) { - result.push(value); + const promises = []; + for (const file of files) { + const name = file.name.toLowerCase(); + if (extensions.some((ext) => name.endsWith(ext))) { + uploadAssetsStore.addNewUploadAsset({ id: getDeviceAssetId(file), file, albumId }); + promises.push(uploadExecutionQueue.addTask(() => fileUploader(file, albumId))); + } } - return result; + + const results = await Promise.all(promises); + return results.filter((result): result is string => !!result); }; +function getDeviceAssetId(asset: File) { + return 'web' + '-' + asset.name + '-' + asset.lastModified; +} + // TODO: should probably use the @api SDK async function fileUploader(asset: File, albumId: string | undefined = undefined): Promise { - const formData = new FormData(); const fileCreatedAt = new Date(asset.lastModified).toISOString(); - const deviceAssetId = 'web' + '-' + asset.name + '-' + asset.lastModified; + const deviceAssetId = getDeviceAssetId(asset); - try { - formData.append('deviceAssetId', deviceAssetId); - formData.append('deviceId', 'WEB'); - formData.append('fileCreatedAt', fileCreatedAt); - formData.append('fileModifiedAt', new Date(asset.lastModified).toISOString()); - formData.append('isFavorite', 'false'); - formData.append('duration', '0:00:00.000000'); - formData.append('assetData', new File([asset], asset.name)); + return new Promise((resolve) => resolve(uploadAssetsStore.markStarted(deviceAssetId))) + .then(() => + api.assetApi.uploadFile( + { + deviceAssetId, + deviceId: 'WEB', + fileCreatedAt, + fileModifiedAt: new Date(asset.lastModified).toISOString(), + isFavorite: false, + duration: '0:00:00.000000', + assetData: new File([asset], asset.name), + key: api.getKey(), + }, + { + onUploadProgress: ({ loaded, total }) => { + uploadAssetsStore.updateProgress(deviceAssetId, loaded, total); + }, + }, + ), + ) + .then(async (response) => { + if (response.status == 200 || response.status == 201) { + const res: AssetFileUploadResponseDto = response.data; - uploadAssetsStore.addNewUploadAsset({ - id: deviceAssetId, - file: asset, - progress: 0, - }); + if (res.duplicate) { + uploadAssetsStore.duplicateCounter.update((count) => count + 1); + } - const response = await axios.post('/api/asset/upload', formData, { - params: { key: api.getKey() }, - onUploadProgress: (event) => { - const percentComplete = Math.floor((event.loaded / event.total) * 100); - uploadAssetsStore.updateProgress(deviceAssetId, percentComplete); - }, - }); + if (albumId && res.id) { + uploadAssetsStore.updateAsset(deviceAssetId, { message: 'Adding to album...' }); + await addAssetsToAlbum(albumId, [res.id]); + uploadAssetsStore.updateAsset(deviceAssetId, { message: 'Added to album' }); + } - if (response.status == 200 || response.status == 201) { - const res: AssetFileUploadResponseDto = response.data; + uploadAssetsStore.updateAsset(deviceAssetId, { + state: res.duplicate ? UploadState.DUPLICATED : UploadState.DONE, + }); + uploadAssetsStore.successCounter.update((c) => c + 1); - if (res.duplicate) { - uploadAssetsStore.duplicateCounter.update((count) => count + 1); + setTimeout(() => { + uploadAssetsStore.removeUploadAsset(deviceAssetId); + }, 1000); + + return res.id; } - - if (albumId && res.id) { - await addAssetsToAlbum(albumId, [res.id]); - } - - setTimeout(() => { - uploadAssetsStore.removeUploadAsset(deviceAssetId); - }, 1000); - - return res.id; - } - } catch (e) { - console.log('error uploading file ', e); - handleUploadError(asset, JSON.stringify(e)); - uploadAssetsStore.removeUploadAsset(deviceAssetId); - } + }) + .catch((reason) => { + console.log('error uploading file ', reason); + uploadAssetsStore.updateAsset(deviceAssetId, { state: UploadState.ERROR, error: reason }); + handleUploadError(asset, JSON.stringify(reason)); + return undefined; + }); } function handleUploadError(asset: File, respBody = '{}', extraMessage?: string) { - uploadAssetsStore.errorCounter.update((count) => count + 1); - try { const res = JSON.parse(respBody); - const extraMsg = res ? ' ' + res?.message : ''; notificationController.show({ diff --git a/web/tailwind.config.cjs b/web/tailwind.config.cjs index acb5bae30f..03e8d6055f 100644 --- a/web/tailwind.config.cjs +++ b/web/tailwind.config.cjs @@ -10,12 +10,18 @@ module.exports = { 'immich-bg': 'white', 'immich-fg': 'black', 'immich-gray': '#F6F6F4', + 'immich-error': '#e57373', + 'immich-success': '#81c784', + 'immich-warning': '#ffb74d', // Dark Theme 'immich-dark-primary': '#adcbfa', 'immich-dark-bg': 'black', 'immich-dark-fg': '#e5e7eb', 'immich-dark-gray': '#212121', + 'immich-dark-error': '#d32f2f', + 'immich-dark-success': '#388e3c', + 'immich-dark-warning': '#f57c00', }, fontFamily: { 'immich-title': ['Snowburst One', 'cursive'],