From c39bdd9724c09ffd3efe72398ce3836eb98e9193 Mon Sep 17 00:00:00 2001 From: Eric Doughty-Papassideris Date: Fri, 24 Jan 2025 00:03:04 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20back:=20immediately=20start=20st?= =?UTF-8?q?reaming=20downloading=20of=20zip=20#783?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/services/documents/services/index.ts | 37 ++++++++++--- .../node/src/services/documents/utils.ts | 8 +++ .../documents/web/controllers/documents.ts | 55 ++++++++++--------- 3 files changed, 67 insertions(+), 33 deletions(-) diff --git a/tdrive/backend/node/src/services/documents/services/index.ts b/tdrive/backend/node/src/services/documents/services/index.ts index bf64ec71..6323cf40 100644 --- a/tdrive/backend/node/src/services/documents/services/index.ts +++ b/tdrive/backend/node/src/services/documents/services/index.ts @@ -1505,10 +1505,17 @@ export class DocumentsService { if (token) throw new CrudException("Invalid token", 401); } }; - + /** + * + * @param id DriveItemID to download. If it's a folder, a zip file will be returned + * @param versionId Optional specific version to download + * @param {Function} beginArchiveTransmit If a folder, + * called when the zip file can begin streaming, otherwise not called at all. + */ download = async ( id: string, versionId: string | null, + beginArchiveTransmit: (readable: archiver.Archiver) => void, context: DriveExecutionContext, ): Promise<{ archive?: archiver.Archiver; @@ -1522,7 +1529,7 @@ export class DocumentsService { const item = await this.get(id, null, context); if (item.item.is_directory) { - return { archive: await this.createZip([id], context) }; + return { archive: await this.createZip([id], beginArchiveTransmit, context) }; } let version = item.item.last_version_cache; @@ -1541,11 +1548,13 @@ export class DocumentsService { * Creates a zip archive containing the drive items. * * @param {string[]} ids - the drive item list + * @param {Function} beginArchiveTransmit - Called when the zip file can begin streaming * @param {DriveExecutionContext} context - the execution context * @returns {Promise} the created archive. */ createZip = async ( ids: string[] = [], + beginArchiveTransmit: (archive: archiver.Archiver) => void, context: DriveExecutionContext, ): Promise => { if (!context) { @@ -1561,19 +1570,31 @@ export class DocumentsService { this.logger.error("error while creating ZIP file: ", error); }); + let didBeginTransmission = false; for (const id of ids) { if (!(await checkAccess(id, null, "read", this.repository, context))) { - this.logger.warn(`not enough permissions to download ${id}, skipping`); - return; + this.logger.warn({ id }, `not enough permissions to download ${id}, skipping`); + continue; } try { - await addDriveItemToArchive(id, null, archive, this.repository, context); - } catch (error) { - console.error(error); - this.logger.warn("failed to add item to archive", error); + await addDriveItemToArchive( + id, + null, + archive, + archive => { + if (didBeginTransmission) return; + didBeginTransmission = true; + beginArchiveTransmit(archive); + }, + this.repository, + context, + ); + } catch (err) { + this.logger.warn({ err, id }, "failed to add item to archive"); } } + if (!didBeginTransmission) beginArchiveTransmit(archive); //TODO[ASH] why do we need this call?? archive.finalize(); diff --git a/tdrive/backend/node/src/services/documents/utils.ts b/tdrive/backend/node/src/services/documents/utils.ts index 6bae8632..eb43e176 100644 --- a/tdrive/backend/node/src/services/documents/utils.ts +++ b/tdrive/backend/node/src/services/documents/utils.ts @@ -345,6 +345,9 @@ export const addDriveItemToArchive = async ( id: string, entity: DriveFile | null, archive: archiver.Archiver, + beginArchiveTransmit: + | "ONLY_SET_THIS_VALUE_IF_ALREADY_CALLED" + | ((archive: archiver.Archiver) => void), repository: Repository, context: CompanyExecutionContext, prefix?: string, @@ -368,6 +371,10 @@ export const addDriveItemToArchive = async ( } archive.append(file.file, { name: item.name, prefix: prefix ?? "" }); + if (beginArchiveTransmit !== "ONLY_SET_THIS_VALUE_IF_ALREADY_CALLED") { + beginArchiveTransmit(archive); + beginArchiveTransmit = "ONLY_SET_THIS_VALUE_IF_ALREADY_CALLED"; + } return; } else { let nextPage = ""; @@ -384,6 +391,7 @@ export const addDriveItemToArchive = async ( child.id, child, archive, + beginArchiveTransmit, repository, context, `${prefix || ""}${item.name}/`, diff --git a/tdrive/backend/node/src/services/documents/web/controllers/documents.ts b/tdrive/backend/node/src/services/documents/web/controllers/documents.ts index 2811de95..6e3dd6fe 100644 --- a/tdrive/backend/node/src/services/documents/web/controllers/documents.ts +++ b/tdrive/backend/node/src/services/documents/web/controllers/documents.ts @@ -551,21 +551,21 @@ export class DocumentsController { const archiveOrFile = await globalResolver.services.documents.documents.download( id, versionId, + archive => { + archive.on("finish", () => { + response.status(200); + }); + + archive.on("error", () => { + response.internalServerError(); + }); + + archive.pipe(response.raw); + }, context, ); if (archiveOrFile.archive) { - const archive = archiveOrFile.archive; - - archive.on("finish", () => { - response.status(200); - }); - - archive.on("error", () => { - response.internalServerError(); - }); - - archive.pipe(response.raw); return response; } else if (archiveOrFile.file) { const data = archiveOrFile.file; @@ -623,22 +623,27 @@ export class DocumentsController { } try { - const archive = await globalResolver.services.documents.documents.createZip(ids, context); - reply.raw.setHeader( - "content-disposition", - formatAttachmentContentDispositionHeader("twake_drive.zip"), + await globalResolver.services.documents.documents.createZip( + ids, + archive => { + reply.raw.setHeader( + "content-disposition", + formatAttachmentContentDispositionHeader("twake_drive.zip"), + ); + + archive.on("finish", () => { + reply.status(200); + }); + + archive.on("error", () => { + reply.internalServerError(); + }); + + archive.pipe(reply.raw); + }, + context, ); - archive.on("finish", () => { - reply.status(200); - }); - - archive.on("error", () => { - reply.internalServerError(); - }); - - archive.pipe(reply.raw); - return reply; } catch (error) { logger.error({ error: `${error}` }, "failed to send zip file");