From b36df8d7ad95f24eaa720a920a71b2099c774158 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Thu, 28 Nov 2024 08:50:00 +0530 Subject: [PATCH] [web] Rework upload URL handling --- web/apps/photos/src/services/logout.ts | 7 ++ .../services/upload/publicUploadHttpClient.ts | 81 ++++++++++--------- .../src/services/upload/upload-service.ts | 50 ++++++++++-- .../src/services/upload/uploadHttpClient.ts | 70 +++++++++------- .../src/services/upload/uploadManager.ts | 5 ++ 5 files changed, 137 insertions(+), 76 deletions(-) diff --git a/web/apps/photos/src/services/logout.ts b/web/apps/photos/src/services/logout.ts index 9049f0b114..cd245bb99b 100644 --- a/web/apps/photos/src/services/logout.ts +++ b/web/apps/photos/src/services/logout.ts @@ -10,6 +10,7 @@ import { logoutSearch } from "@/new/photos/services/search"; import { logoutSettings } from "@/new/photos/services/settings"; import { logoutUserDetails } from "@/new/photos/services/user-details"; import exportService from "./export"; +import uploadManager from "./upload/uploadManager"; /** * Logout sequence for the photos app. @@ -59,6 +60,12 @@ export const photosLogout = async () => { ignoreError("Upload", e); } + try { + uploadManager.logout(); + } catch (e) { + ignoreError("Upload", e); + } + try { downloadManager.logout(); } catch (e) { diff --git a/web/apps/photos/src/services/upload/publicUploadHttpClient.ts b/web/apps/photos/src/services/upload/publicUploadHttpClient.ts index 60bf21f69b..f683331ef1 100644 --- a/web/apps/photos/src/services/upload/publicUploadHttpClient.ts +++ b/web/apps/photos/src/services/upload/publicUploadHttpClient.ts @@ -1,16 +1,29 @@ +import { ensureOk } from "@/base/http"; import log from "@/base/log"; import { apiURL } from "@/base/origins"; import { EnteFile } from "@/media/file"; import { retryAsyncOperation } from "@/utils/promise"; import { CustomError, handleUploadError } from "@ente/shared/error"; import HTTPService from "@ente/shared/network/HTTPService"; -import { MultipartUploadURLs, UploadFile, UploadURL } from "./upload-service"; +import { z } from "zod"; +import { + MultipartUploadURLs, + UploadFile, + type UploadURL, +} from "./upload-service"; -const MAX_URL_REQUESTS = 50; +/** + * Zod schema for {@link UploadURL}. + * + * TODO: Duplicated with uploadHttpClient, can be removed after we refactor this + * code. + */ +const UploadURL = z.object({ + objectKey: z.string(), + url: z.string(), +}); class PublicUploadHttpClient { - private uploadURLFetchInProgress = null; - async uploadFile( uploadFile: UploadFile, token: string, @@ -38,43 +51,35 @@ class PublicUploadHttpClient { } } + /** + * Sibling of {@link fetchUploadURLs} for public albums. + */ async fetchUploadURLs( - count: number, - urlStore: UploadURL[], + countHint: number, token: string, passwordToken: string, - ): Promise { - try { - if (!this.uploadURLFetchInProgress) { - try { - if (!token) { - throw Error(CustomError.TOKEN_MISSING); - } - this.uploadURLFetchInProgress = HTTPService.get( - await apiURL("/public-collection/upload-urls"), - { - count: Math.min(MAX_URL_REQUESTS, count * 2), - }, - { - "X-Auth-Access-Token": token, - ...(passwordToken && { - "X-Auth-Access-Token-JWT": passwordToken, - }), - }, - ); - const response = await this.uploadURLFetchInProgress; - for (const url of response.data.urls) { - urlStore.push(url); - } - } finally { - this.uploadURLFetchInProgress = null; - } - } - return this.uploadURLFetchInProgress; - } catch (e) { - log.error("fetch public upload-url failed ", e); - throw e; - } + ) { + const count = Math.min(50, countHint * 2).toString(); + const params = new URLSearchParams({ count }); + const url = await apiURL("/public-collection/upload-urls"); + const res = await fetch(`${url}?${params.toString()}`, { + // TODO: Use authenticatedPublicAlbumsRequestHeaders after the public + // albums refactor branch is merged. + // headers: await authenticatedRequestHeaders(), + headers: { + "X-Auth-Access-Token": token, + ...(passwordToken && { + "X-Auth-Access-Token-JWT": passwordToken, + }), + }, + }); + ensureOk(res); + return ( + // TODO: The as cast will not be needed when tsc strict mode is + // enabled for this code. + z.object({ urls: UploadURL.array() }).parse(await res.json()) + .urls as UploadURL[] + ); } async fetchMultipartUploadURLs( diff --git a/web/apps/photos/src/services/upload/upload-service.ts b/web/apps/photos/src/services/upload/upload-service.ts index 07150eab60..d94e3a3d60 100644 --- a/web/apps/photos/src/services/upload/upload-service.ts +++ b/web/apps/photos/src/services/upload/upload-service.ts @@ -111,11 +111,19 @@ class UploadService { private uploadURLs: UploadURL[] = []; private pendingUploadCount: number = 0; private publicUploadProps: PublicUploadProps = undefined; + private activeUploadURLRefill: Promise | undefined; init(publicUploadProps: PublicUploadProps) { this.publicUploadProps = publicUploadProps; } + logout() { + this.uploadURLs = []; + this.pendingUploadCount = 0; + this.publicUploadProps = undefined; + this.activeUploadURLRefill = undefined; + } + async setFileCount(fileCount: number) { this.pendingUploadCount = fileCount; await this.preFetchUploadURLs(); @@ -127,14 +135,14 @@ class UploadService { async getUploadURL() { if (this.uploadURLs.length === 0 && this.pendingUploadCount) { - await this.fetchUploadURLs(); + await this.refillUploadURLs(); } return this.uploadURLs.pop(); } private async preFetchUploadURLs() { try { - await this.fetchUploadURLs(); + await this.refillUploadURLs(); // checking for any subscription related errors } catch (e) { log.error("prefetch uploadURL failed", e); @@ -154,20 +162,43 @@ class UploadService { } } - private async fetchUploadURLs() { + private async refillUploadURLs() { + try { + if (!this.activeUploadURLRefill) { + this.activeUploadURLRefill = this._refillUploadURLs(); + } + await this.activeUploadURLRefill; + } finally { + this.activeUploadURLRefill = undefined; + } + + // TODO: Sanity check added on new implementation Nov 2024, remove after + // a while (tag: Migration). + if ( + this.uploadURLs.length != + new Set(this.uploadURLs.map((u) => u.url)).size + ) { + throw new Error("Duplicate upload URLs detected"); + } + } + + private async _refillUploadURLs() { + let urls: UploadURL[]; if (this.publicUploadProps.accessedThroughSharedURL) { - await publicUploadHttpClient.fetchUploadURLs( + if (!this.publicUploadProps.token) { + throw Error(CustomError.TOKEN_MISSING); + } + urls = await publicUploadHttpClient.fetchUploadURLs( this.pendingUploadCount, - this.uploadURLs, this.publicUploadProps.token, this.publicUploadProps.passwordToken, ); } else { - await UploadHttpClient.fetchUploadURLs( + urls = await UploadHttpClient.fetchUploadURLs( this.pendingUploadCount, - this.uploadURLs, ); } + urls.forEach((u) => this.uploadURLs.push(u)); } async fetchMultipartUploadURLs(count: number) { @@ -291,8 +322,13 @@ export interface MultipartUploadURLs { completeURL: string; } +/** + * A pre-signed URL alongwith the associated object key. + */ export interface UploadURL { + /** A pre-signed URL that can be used to upload data to S3. */ url: string; + /** The objectKey with which remote will refer to this object. */ objectKey: string; } diff --git a/web/apps/photos/src/services/upload/uploadHttpClient.ts b/web/apps/photos/src/services/upload/uploadHttpClient.ts index 553bcfea89..cdfc5ea3c4 100644 --- a/web/apps/photos/src/services/upload/uploadHttpClient.ts +++ b/web/apps/photos/src/services/upload/uploadHttpClient.ts @@ -1,3 +1,4 @@ +import { authenticatedRequestHeaders, ensureOk } from "@/base/http"; import log from "@/base/log"; import { apiURL, uploaderOrigin } from "@/base/origins"; import { EnteFile } from "@/media/file"; @@ -5,11 +6,22 @@ import { retryAsyncOperation } from "@/utils/promise"; import { CustomError, handleUploadError } from "@ente/shared/error"; import HTTPService from "@ente/shared/network/HTTPService"; import { getToken } from "@ente/shared/storage/localStorage/helpers"; -import { MultipartUploadURLs, UploadFile, UploadURL } from "./upload-service"; +import { z } from "zod"; +import { + MultipartUploadURLs, + UploadFile, + type UploadURL, +} from "./upload-service"; + +/** + * Zod schema for {@link UploadURL}. + */ +const UploadURL = z.object({ + objectKey: z.string(), + url: z.string(), +}); class UploadHttpClient { - private uploadURLFetchInProgress = null; - async uploadFile(uploadFile: UploadFile): Promise { try { const token = getToken(); @@ -31,34 +43,30 @@ class UploadHttpClient { } } - async fetchUploadURLs(count: number, urlStore: UploadURL[]): Promise { - try { - if (!this.uploadURLFetchInProgress) { - try { - const token = getToken(); - if (!token) { - return; - } - this.uploadURLFetchInProgress = HTTPService.get( - await apiURL("/files/upload-urls"), - { - count: Math.min(50, count * 2), - }, - { "X-Auth-Token": token }, - ); - const response = await this.uploadURLFetchInProgress; - for (const url of response.data.urls) { - urlStore.push(url); - } - } finally { - this.uploadURLFetchInProgress = null; - } - } - return this.uploadURLFetchInProgress; - } catch (e) { - log.error("fetch upload-url failed ", e); - throw e; - } + /** + * Fetch a fresh list of URLs from remote that can be used to upload files + * and thumbnails to. + * + * @param countHint An approximate number of files that we're expecting to + * upload. + * + * @returns A list of pre-signed object URLs that can be used to upload data + * to the S3 bucket. + */ + async fetchUploadURLs(countHint: number) { + const count = Math.min(50, countHint * 2).toString(); + const params = new URLSearchParams({ count }); + const url = await apiURL("/files/upload-urls"); + const res = await fetch(`${url}?${params.toString()}`, { + headers: await authenticatedRequestHeaders(), + }); + ensureOk(res); + return ( + // TODO: The as cast will not be needed when tsc strict mode is + // enabled for this code. + z.object({ urls: UploadURL.array() }).parse(await res.json()) + .urls as UploadURL[] + ); } async fetchMultipartUploadURLs( diff --git a/web/apps/photos/src/services/upload/uploadManager.ts b/web/apps/photos/src/services/upload/uploadManager.ts index 262c00c187..f49925210e 100644 --- a/web/apps/photos/src/services/upload/uploadManager.ts +++ b/web/apps/photos/src/services/upload/uploadManager.ts @@ -344,6 +344,11 @@ class UploadManager { this.publicUploadProps = publicCollectProps; } + logout() { + // TODO: Consolidate state in one place instead of spreading it. + UploadService.logout(); + } + public isUploadRunning() { return this.uploadInProgress; }