[web] Rework upload URL handling

2024-11-28 08:50:00 +05:30
parent d2761b6be9
commit b36df8d7ad
5 changed files with 137 additions and 76 deletions
--- 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) {
--- 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<void> {
-        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(
--- 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<void> | 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;
 }

--- 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<EnteFile> {
        try {
            const token = getToken();
@@ -31,34 +43,30 @@ class UploadHttpClient {
        }
    }

-    async fetchUploadURLs(count: number, urlStore: UploadURL[]): Promise<void> {
-        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(
--- 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;
    }