Splits findOrAttach file into 2 functions

gristlabs · Jan 14, 2025 · 5372657 · 5372657
1 parent af5f08d
commit 5372657
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 7 deletions.
diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts
@@ -401,7 +401,7 @@ export class AttachmentFileManager implements IAttachmentFileManager {
   // Uploads the file to local storage, overwriting the current DB record for the file.
   private async _storeFileInLocalStorage(fileIdent: string, fileData: Buffer): Promise<AddFileResult> {
     // Insert (or overwrite) the entry for this file in the document database.
-    const isNewFile = await this._docStorage.findOrAttachFile(fileIdent, fileData, undefined, true);
+    const isNewFile = await this._docStorage.attachOrUpdateFile(fileIdent, fileData, undefined);
 
     return {
       fileIdent,
@@ -418,7 +418,7 @@ export class AttachmentFileManager implements IAttachmentFileManager {
     await store.upload(this._getDocPoolId(), fileIdent, Readable.from(fileData));
 
     // Insert (or overwrite) the entry for this file in the document database.
-    await this._docStorage.findOrAttachFile(fileIdent, undefined, store.id, true);
+    await this._docStorage.attachOrUpdateFile(fileIdent, undefined, store.id);
 
     return fileIdent;
   }

diff --git a/test/server/lib/AttachmentFileManager.ts b/test/server/lib/AttachmentFileManager.ts
@@ -13,7 +13,10 @@ import * as sinon from "sinon";
 import * as stream from "node:stream";
 
 // Minimum features of doc storage that are needed to make AttachmentFileManager work.
-type IMinimalDocStorage = Pick<DocStorage, 'docName' | 'getFileInfo' | 'findOrAttachFile' | 'listAllFiles'>
+type IMinimalDocStorage = Pick<
+  DocStorage,
+  'docName' | 'getFileInfo' | 'findOrAttachFile' | 'attachOrUpdateFile' | 'listAllFiles'
+>
 
 // Implements the minimal functionality needed for the AttachmentFileManager to work.
 class DocStorageFake implements IMinimalDocStorage {
@@ -37,17 +40,29 @@ class DocStorageFake implements IMinimalDocStorage {
 
   // Needs to match the semantics of DocStorage's implementation.
   public async findOrAttachFile(
-    fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined, shouldUpdate: boolean = true
+    fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined
   ): Promise<boolean> {
-    if (fileIdent in this._files && !shouldUpdate) {
+    if (fileIdent in this._files) {
       return false;
     }
+    this._setFileRecord(fileIdent, fileData, storageId);
+    return true;
+  }
+
+  public async attachOrUpdateFile(
+    fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined
+  ): Promise<boolean> {
+    const exists = fileIdent in this._files;
+    this._setFileRecord(fileIdent, fileData, storageId);
+    return !exists;
+  }
+
+  private _setFileRecord(fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined) {
     this._files[fileIdent] = {
       ident: fileIdent,
       data: fileData ?? Buffer.alloc(0),
       storageId: storageId ?? null,
     };
-    return true;
   }
 }
 

diff --git a/test/server/lib/DocStorage.js b/test/server/lib/DocStorage.js
@@ -404,7 +404,7 @@ describe('DocStorage', function() {
       .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), correctFileContents))
 
       // The update parameter should allow the record to be overwritten
-      .then(() => docStorage.findOrAttachFile("hello_world.txt", Buffer.from(replacementFileContents), undefined, true))
+      .then(() => docStorage.attachOrUpdateFile("hello_world.txt", Buffer.from(replacementFileContents), undefined))
       .then(result => assert.isFalse(result))
       .then(() => docStorage.getFileInfo("hello_world.txt"))
       .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), replacementFileContents));