Adds zip downloads for all of a document's attachments

app/server/lib/ActiveDoc.ts

@@ -90,6 +90,7 @@ import {Share} from 'app/gen-server/entity/Share';
 import {RecordWithStringId} from 'app/plugin/DocApiTypes';
 import {ParseFileResult, ParseOptions} from 'app/plugin/FileParserAPI';
 import {AccessTokenOptions, AccessTokenResult, GristDocAPI, UIRowId} from 'app/plugin/GristAPI';
+import {Archive, ArchiveEntry, create_zip_archive} from 'app/server/lib/Archive';
 import {AssistanceSchemaPromptV1Context} from 'app/server/lib/Assistance';
 import {AssistanceContext} from 'app/common/AssistancePrompts';
 import {AuditEventAction} from 'app/server/lib/AuditEvent';
@@ -959,6 +960,32 @@ export class ActiveDoc extends EventEmitter {
     return data;
   }
 
+  public async getAttachmentsArchive(): Promise<Archive> {
+    if (!this.docData) {
+      throw new Error("No doc data");
+    }
+    const attachments = this.docData.getMetaTable('_grist_Attachments').getRecords();
+    const attachmentFileManager = this._attachmentFileManager;
+
+    async function* fileGenerator(): AsyncGenerator<ArchiveEntry> {
+      const filesSeen = new Set<string>();
+      for (const attachment of attachments) {
+        if (filesSeen.has(attachment.fileIdent)) {
+          continue;
+        }
+        filesSeen.add(attachment.fileIdent);
+        const file = await attachmentFileManager.getFile(attachment.fileIdent);
+        yield({
+          name: attachment.fileName,
+          data: file.contentStream,
+        });
+        // TODO - Abort this on shutdown by throwing an error
+      }
+    }
+
+    return create_zip_archive({ store: true }, fileGenerator());
+  }
+
   @ActiveDoc.keepDocOpen
   public async startTransferringAllAttachmentsToDefaultStore() {
     const attachmentStoreId = this._getDocumentSettings().attachmentStoreId;

app/server/lib/Archive.ts

@@ -0,0 +1,59 @@
+import {ZipArchiveEntry} from 'compress-commons';
+import stream from 'node:stream';
+import ZipStream, {ZipStreamOptions} from 'zip-stream';
+
+export interface ArchiveEntry {
+  name: string;
+  data: stream.Readable | Buffer;
+}
+
+export interface Archive {
+  dataStream: stream.Readable,
+  completed: Promise<void>
+}
+
+/**
+ *
+ * Creates a streamable zip archive, reading files on-demand from the entries iterator.
+ * Entries are provided as an async iterable, to ensure the archive is constructed
+ * correctly. A generator can be used for convenience.
+ * @param {ZipStreamOptions} options - Settings for the zip archive
+ * @param {AsyncIterable<ArchiveEntry>} entries - Entries to add.
+ * @returns {Archive}
+ */
+export async function create_zip_archive(
+  options: ZipStreamOptions, entries: AsyncIterable<ArchiveEntry>
+): Promise<Archive> {
+  // Dynamic import needed because zip-stream is only an ESM module, and we use module: CommonJS
+  // However, typescript dynamic imports are broken. So we need to dynamic eval to fix it. :(
+  // eslint-disable-next-line @typescript-eslint/no-implied-eval
+  const ZipStreamModule = await (Function('return import("zip-stream")')() as Promise<typeof import('zip-stream')>);
+
+  const _archive = new ZipStreamModule.default(options);
+
+  return {
+    dataStream: _archive,
+    // TODO - Should we add a default 'catch' here that logs errors?
+    completed: (async () => {
+      for await (const entry of entries) {
+        // ZipStream will break if multiple entries try to be added at the same time.
+        // TODo - test what happens to the stream if an error is thrown here.
+        // Do we need to manually destroy the archive?
+        await addEntryToZipArchive(_archive, entry);
+      }
+      _archive.finish();
+      await stream.promises.finished(_archive);
+    })()
+  };
+}
+
+function addEntryToZipArchive(archive: ZipStream, file: ArchiveEntry): Promise<ZipArchiveEntry | undefined> {
+  return new Promise((resolve, reject) => {
+    archive.entry(file.data, { name: file.name }, function(err, entry) {
+      if (err) {
+        return reject(err);
+      }
+      return resolve(entry);
+    });
+  });
+}

app/server/lib/AttachmentFileManager.ts

@@ -1,16 +1,16 @@
 import {AttachmentTransferStatus, DocAttachmentsLocation} from 'app/common/UserAPI';
 import {
+  AttachmentFile,
   AttachmentStoreDocInfo,
   DocPoolId,
   getDocPoolIdFromDocInfo,
-  IAttachmentStore
+  IAttachmentStore, loadAttachmentFileIntoMemory
 } from 'app/server/lib/AttachmentStore';
 import {AttachmentStoreId, IAttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider';
 import {checksumFileStream} from 'app/server/lib/checksumFile';
 import {DocStorage} from 'app/server/lib/DocStorage';
 import log from 'app/server/lib/log';
 import {LogMethods} from 'app/server/lib/LogMethods';
-import {MemoryWritableStream} from 'app/server/utils/MemoryWritableStream';
 import {EventEmitter} from 'events';
 import {Readable} from 'node:stream';
 import {AbortController} from 'node-abort-controller';
@@ -141,7 +141,12 @@ export class AttachmentFileManager extends EventEmitter {
 
 
   public async getFileData(fileIdent: string): Promise<Buffer> {
-    return (await this._getFile(fileIdent)).data;
+    const file = await this.getFile(fileIdent);
+    return (await loadAttachmentFileIntoMemory(file)).contents;
+  }
+
+  public async getFile(fileIdent: string): Promise<AttachmentFile> {
+    return (await this._getFile(fileIdent)).file;
   }
 
   public async locationSummary(): Promise<DocAttachmentsLocation> {
@@ -211,12 +216,17 @@ export class AttachmentFileManager extends EventEmitter {
     // re-check.
     // Streaming isn't an option here, as SQLite only supports buffers (meaning we need to keep at
     // least 1 full copy of the file in memory during transfers).
-    const file = await this._getFile(fileIdent);
-    if (!await validateFileChecksum(fileIdent, file.data)) {
-      throw new AttachmentRetrievalError(file.storageId, file.ident, "checksum verification failed for retrieved file");
+    const fileInfo = await this._getFile(fileIdent);
+    // Cache this to avoid undefined warnings everywhere we use `dataInMemory`.
+    const fileInMemory = await loadAttachmentFileIntoMemory(fileInfo.file);
+
+    if (!await validateFileChecksum(fileIdent, fileInMemory.contents)) {
+      throw new AttachmentRetrievalError(
+        fileInfo.storageId, fileInfo.ident, "checksum verification failed for retrieved file"
+      );
     }
     if (!newStoreId) {
-      await this._storeFileInLocalStorage(fileIdent, file.data);
+      await this._storeFileInLocalStorage(fileIdent, fileInMemory.contents);
       return;
     }
     const newStore = await this._getStore(newStoreId);
@@ -228,7 +238,7 @@ export class AttachmentFileManager extends EventEmitter {
       throw new StoreNotAvailableError(newStoreId);
     }
     // Store should error if the upload fails in any way.
-    await this._storeFileInAttachmentStore(newStore, fileIdent, file.data);
+    await this._storeFileInAttachmentStore(newStore, fileIdent, fileInMemory.contents);
 
     // Don't remove the file from the previous store, in case we need to roll back to an earlier
     // snapshot.
@@ -436,7 +446,13 @@ export class AttachmentFileManager extends EventEmitter {
       return {
         ident: fileIdent,
         storageId: null,
-        data: fileInfo.data,
+        file: {
+          metadata: {
+            size: fileInfo.data.length,
+          },
+          contentStream: Readable.from(fileInfo.data),
+          contents: fileInfo.data,
+        }
       };
     }
     const store = await this._getStore(fileInfo.storageId);
@@ -447,7 +463,7 @@ export class AttachmentFileManager extends EventEmitter {
     return {
       ident: fileIdent,
       storageId: store.id,
-      data: await this._getFileDataFromAttachmentStore(store, fileIdent),
+      file: await this._getFileDataFromAttachmentStore(store, fileIdent),
     };
   }
 
@@ -494,11 +510,9 @@ export class AttachmentFileManager extends EventEmitter {
     await this._docStorage.attachOrUpdateFile(fileIdent, undefined, store.id);
   }
 
-  private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise<Buffer> {
+  private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise<AttachmentFile> {
     try {
-      const outputStream = new MemoryWritableStream();
-      await store.download(this._getDocPoolId(), fileIdent, outputStream);
-      return outputStream.getBuffer();
+      return await store.download(this._getDocPoolId(), fileIdent);
     } catch(e) {
       throw new AttachmentRetrievalError(store.id, fileIdent, e);
     }
@@ -525,7 +539,7 @@ interface AttachmentFileManagerLogInfo {
 interface AttachmentFileInfo {
   ident: string;
   storageId: string | null;
-  data: Buffer;
+  file: AttachmentFile;
 }
 
 type TransferJob = Promise<void>

app/server/lib/AttachmentStore.ts

@@ -2,9 +2,12 @@ import {
   ExternalStorage,
   joinKeySegments,
 } from 'app/server/lib/ExternalStorage';
+import {MemoryWritableStream} from 'app/server/utils/MemoryWritableStream';
 import * as fse from 'fs-extra';
 import * as stream from 'node:stream';
 import * as path from 'path';
+import {Readable} from 'node:stream';
+import {pipeline} from 'node:stream/promises';
 
 export type DocPoolId = string;
 type FileId = string;
@@ -20,6 +23,40 @@ export interface AttachmentStoreDocInfo {
   trunkId: string | null | undefined;
 }
 
+interface FileMetadata {
+  size: number;
+}
+
+export interface AttachmentFile {
+  metadata: FileMetadata,
+  contentStream: Readable,
+  // Used to optimise certain scenarios where the data *must* be in memory (e.g. SQLite read/writes)
+  contents?: Buffer
+}
+
+export interface AttachmentFileInMemory extends AttachmentFile {
+  contents: Buffer;
+}
+
+export function isAttachmentFileInMemory(file: AttachmentFile): file is AttachmentFileInMemory {
+  return file.contents !== undefined;
+}
+
+export async function loadAttachmentFileIntoMemory(file: AttachmentFile): Promise<AttachmentFileInMemory> {
+  if (isAttachmentFileInMemory(file)) {
+    return file;
+  }
+  const memoryStream = new MemoryWritableStream();
+  await pipeline(file.contentStream, memoryStream);
+  const buffer = memoryStream.getBuffer();
+
+  // Use Object.assign because it gives type safety, without having to us `as` or copy the object.
+  return Object.assign(file, {
+    contents: buffer,
+    contentStream: Readable.from(buffer),
+  });
+}
+
 /**
  * Gets the correct pool id for a given document, given the document's id and trunk id.
  *
@@ -74,10 +111,8 @@ export interface IAttachmentStore {
   // Upload attachment to the store.
   upload(docPoolId: DocPoolId, fileId: FileId, fileData: stream.Readable): Promise<void>;
 
-  // Download attachment to an in-memory buffer.
-  // It's preferable to accept an output stream as a parameter, as it simplifies attachment store
-  // implementation and gives them control over local buffering.
-  download(docPoolId: DocPoolId, fileId: FileId, outputStream: stream.Writable): Promise<void>;
+  // Fetch the attachment from the store, including a readable stream for the attachment's contents.
+  download(docPoolId: DocPoolId, fileId: FileId): Promise<AttachmentFile>;
 
   // Remove attachment from the store
   delete(docPoolId: DocPoolId, fileId: FileId): Promise<void>;
@@ -127,8 +162,8 @@ export class ExternalStorageAttachmentStore implements IAttachmentStore {
     await this._storage.uploadStream(this._getKey(docPoolId, fileId), fileData);
   }
 
-  public async download(docPoolId: string, fileId: string, outputStream: stream.Writable): Promise<void> {
-    await this._storage.downloadStream(this._getKey(docPoolId, fileId), outputStream);
+  public async download(docPoolId: string, fileId: string): Promise<AttachmentFile> {
+    return await this._storage.downloadStream(this._getKey(docPoolId, fileId));
   }
 
   public async delete(docPoolId: string, fileId: string): Promise<void> {
@@ -171,11 +206,15 @@ export class FilesystemAttachmentStore implements IAttachmentStore {
     );
   }
 
-  public async download(docPoolId: DocPoolId, fileId: FileId, output: stream.Writable): Promise<void> {
-    await stream.promises.pipeline(
-      fse.createReadStream(this._createPath(docPoolId, fileId)),
-      output,
-    );
+  public async download(docPoolId: DocPoolId, fileId: FileId): Promise<AttachmentFile> {
+    const filePath = this._createPath(docPoolId, fileId);
+    const stat = await fse.stat(filePath);
+    return {
+      metadata: {
+        size: stat.size,
+      },
+      contentStream: fse.createReadStream(filePath)
+    };
   }
 
   public async delete(docPoolId: string, fileId: string): Promise<void> {

app/server/lib/DocApi.ts

@@ -573,6 +573,19 @@ export class DocWorkerApi {
       })
     );
 
+    // Responds with attachment contents, with suitable Content-Type and Content-Disposition.
+    this._app.get('/api/docs/:docId/attachments/download', canView, withDoc(async (activeDoc, req, res) => {
+      const archive = await activeDoc.getAttachmentsArchive();
+      res.status(200)
+        .type("application/zip")
+        // Construct a content-disposition header of the form 'attachment; filename="NAME"'
+        .set('Content-Disposition', contentDisposition(`${activeDoc.docName}.zip`, {type: 'attachment'}))
+        // Avoid storing because this could be huge.
+        .set('Cache-Control', 'no-store');
+
+      archive.dataStream.pipe(res);
+    }));
+
     // Returns cleaned metadata for a given attachment ID (i.e. a rowId in _grist_Attachments table).
     this._app.get('/api/docs/:docId/attachments/:attId', canView, withDoc(async (activeDoc, req, res) => {
       const attId = integerParam(req.params.attId, 'attId');

app/server/lib/ExternalStorage.ts

@@ -11,6 +11,16 @@ import stream from 'node:stream';
 // checksum is expected otherwise.
 export const DELETED_TOKEN = '*DELETED*';
 
+export interface FileMetadata {
+  size: number;
+  snapshotId: string;
+}
+
+export interface StreamDownloadResult {
+  metadata: FileMetadata,
+  contentStream: stream.Readable,
+}
+
 /**
  * An external store for the content of files.  The store may be either consistent
  * or eventually consistent.  Specifically, the `exists`, `download`, and `versions`
@@ -59,7 +69,7 @@ export interface ExternalStorage {
   close(): Promise<void>;
 
   uploadStream?(key: string, inStream: stream.Readable, metadata?: ObjMetadata): Promise<string|null|typeof Unchanged>;
-  downloadStream?(key: string, outStream: stream.Writable, snapshotId?: string ): Promise<string>;
+  downloadStream?(key: string, snapshotId?: string ): Promise<StreamDownloadResult>;
 }
 
 /**
@@ -81,7 +91,7 @@ export class KeyMappedExternalStorage implements ExternalStorage {
     if (_ext.downloadStream !== undefined) {
       const extDownloadStream = _ext.downloadStream;
       this.downloadStream =
-        (key, outStream, snapshotId) => extDownloadStream.call(_ext, this._map(key), outStream, snapshotId);
+        (key, snapshotId) => extDownloadStream.call(_ext, this._map(key), snapshotId);
     }
     if (_ext.removeAllWithPrefix !== undefined) {
       const extRemoveAllWithPrefix = _ext.removeAllWithPrefix;