Handle singleattachment on AttachmentCleanup

packages/server/src/utilities/rowProcessor/attachments.ts

@@ -25,6 +25,27 @@ export class AttachmentCleanup {
     }
   }
 
+  private static extractAttachmentKeys(
+    type: FieldType,
+    rowData: any
+  ): string[] {
+    if (
+      type !== FieldType.ATTACHMENTS &&
+      type !== FieldType.ATTACHMENT_SINGLE
+    ) {
+      return []
+    }
+
+    if (!rowData) {
+      return []
+    }
+
+    if (type === FieldType.ATTACHMENTS) {
+      return rowData.map((attachment: any) => attachment.key)
+    }
+    return [rowData.key]
+  }
+
   private static async tableChange(
     table: Table,
     rows: Row[],
@@ -34,16 +55,20 @@ export class AttachmentCleanup {
       let files: string[] = []
       const tableSchema = opts.oldTable?.schema || table.schema
       for (let [key, schema] of Object.entries(tableSchema)) {
-        if (schema.type !== FieldType.ATTACHMENTS) {
+        if (
+          schema.type !== FieldType.ATTACHMENTS &&
+          schema.type !== FieldType.ATTACHMENT_SINGLE
+        ) {
           continue
         }
+
         const columnRemoved = opts.oldTable && !table.schema[key]
         const renaming = opts.rename?.old === key
         // old table had this column, new table doesn't - delete it
         if ((columnRemoved && !renaming) || opts.deleting) {
           rows.forEach(row => {
             files = files.concat(
-              (row[key] || []).map((attachment: any) => attachment.key)
+              AttachmentCleanup.extractAttachmentKeys(schema.type, row[key])
             )
           })
         }
@@ -68,15 +93,15 @@ export class AttachmentCleanup {
     return AttachmentCleanup.coreCleanup(() => {
       let files: string[] = []
       for (let [key, schema] of Object.entries(table.schema)) {
-        if (schema.type !== FieldType.ATTACHMENTS) {
+        if (
+          schema.type !== FieldType.ATTACHMENTS &&
+          schema.type !== FieldType.ATTACHMENT_SINGLE
+        ) {
           continue
         }
         rows.forEach(row => {
-          if (!Array.isArray(row[key])) {
-            return
-          }
           files = files.concat(
-            row[key].map((attachment: any) => attachment.key)
+            AttachmentCleanup.extractAttachmentKeys(schema.type, row[key])
           )
         })
       }
@@ -88,16 +113,21 @@ export class AttachmentCleanup {
     return AttachmentCleanup.coreCleanup(() => {
       let files: string[] = []
       for (let [key, schema] of Object.entries(table.schema)) {
-        if (schema.type !== FieldType.ATTACHMENTS) {
+        if (
+          schema.type !== FieldType.ATTACHMENTS &&
+          schema.type !== FieldType.ATTACHMENT_SINGLE
+        ) {
           continue
         }
-        const oldKeys =
+
-          opts.oldRow[key]?.map(
+        const oldKeys = AttachmentCleanup.extractAttachmentKeys(
-            (attachment: RowAttachment) => attachment.key
+          schema.type,
-          ) || []
+          opts.oldRow[key]
-        const newKeys =
+        )
-          opts.row[key]?.map((attachment: RowAttachment) => attachment.key) ||
+        const newKeys = AttachmentCleanup.extractAttachmentKeys(
-          []
+          schema.type,
+          opts.row[key]
+        )
         files = files.concat(
           oldKeys.filter((key: string) => newKeys.indexOf(key) === -1)
         )

packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts

@@ -25,121 +25,155 @@ const mockedDeleteFiles = objectStore.deleteFiles as jest.MockedFunction<
   typeof objectStore.deleteFiles
 >
 
-function table(): Table {
+const rowGenerators: [
-  return {
+  string,
-    name: "table",
+  FieldType.ATTACHMENT_SINGLE | FieldType.ATTACHMENTS,
-    sourceId: DEFAULT_BB_DATASOURCE_ID,
+  (fileKey?: string) => Row
-    sourceType: TableSourceType.INTERNAL,
+][] = [
-    type: "table",
+  [
-    schema: {
+    "row with a attachment list column",
-      attach: {
+    FieldType.ATTACHMENTS,
-        name: "attach",
+    function rowWithAttachments(fileKey: string = FILE_NAME): Row {
-        type: FieldType.ATTACHMENTS,
+      return {
-        constraints: {},
+        attach: [
-      },
+          {
+            size: 1,
+            extension: "jpg",
+            key: fileKey,
+          },
+        ],
+      }
     },
-  }
+  ],
-}
+  [
-
+    "row with a single attachment column",
-function row(fileKey: string = FILE_NAME): Row {
+    FieldType.ATTACHMENT_SINGLE,
-  return {
+    function rowWithAttachments(fileKey: string = FILE_NAME): Row {
-    attach: [
+      return {
-      {
+        attach: {
-        size: 1,
+          size: 1,
-        extension: "jpg",
+          extension: "jpg",
-        key: fileKey,
+          key: fileKey,
-      },
+        },
-    ],
-  }
-}
-
-describe("attachment cleanup", () => {
-  beforeEach(() => {
-    mockedDeleteFiles.mockClear()
-  })
-
-  it("should be able to cleanup a table update", async () => {
-    const originalTable = table()
-    delete originalTable.schema["attach"]
-    await AttachmentCleanup.tableUpdate(originalTable, [row()], {
-      oldTable: table(),
-    })
-    expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
-  })
-
-  it("should be able to cleanup a table deletion", async () => {
-    await AttachmentCleanup.tableDelete(table(), [row()])
-    expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
-  })
-
-  it("should handle table column renaming", async () => {
-    const updatedTable = table()
-    updatedTable.schema.attach2 = updatedTable.schema.attach
-    delete updatedTable.schema.attach
-    await AttachmentCleanup.tableUpdate(updatedTable, [row()], {
-      oldTable: table(),
-      rename: { old: "attach", updated: "attach2" },
-    })
-    expect(mockedDeleteFiles).not.toHaveBeenCalled()
-  })
-
-  it("shouldn't cleanup if no table changes", async () => {
-    await AttachmentCleanup.tableUpdate(table(), [row()], { oldTable: table() })
-    expect(mockedDeleteFiles).not.toHaveBeenCalled()
-  })
-
-  it("should handle row updates", async () => {
-    const updatedRow = row()
-    delete updatedRow.attach
-    await AttachmentCleanup.rowUpdate(table(), {
-      row: updatedRow,
-      oldRow: row(),
-    })
-    expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
-  })
-
-  it("should handle row deletion", async () => {
-    await AttachmentCleanup.rowDelete(table(), [row()])
-    expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
-  })
-
-  it("should handle row deletion and not throw when attachments are undefined", async () => {
-    await AttachmentCleanup.rowDelete(table(), [
-      {
-        attach: undefined,
-      },
-    ])
-  })
-
-  it("shouldn't cleanup attachments if row not updated", async () => {
-    await AttachmentCleanup.rowUpdate(table(), { row: row(), oldRow: row() })
-    expect(mockedDeleteFiles).not.toHaveBeenCalled()
-  })
-
-  it("should be able to cleanup a column and not throw when attachments are undefined", async () => {
-    const originalTable = table()
-    delete originalTable.schema["attach"]
-    await AttachmentCleanup.tableUpdate(
-      originalTable,
-      [row("file 1"), { attach: undefined }, row("file 2")],
-      {
-        oldTable: table(),
       }
-    )
+    },
-    expect(mockedDeleteFiles).toHaveBeenCalledTimes(1)
+  ],
-    expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, ["file 1", "file 2"])
+]
-  })
 
-  it("should be able to cleanup a column and not throw when ALL attachments are undefined", async () => {
+describe.each(rowGenerators)(
-    const originalTable = table()
+  "attachment cleanup",
-    delete originalTable.schema["attach"]
+  (_, attachmentFieldType, rowGenerator) => {
-    await AttachmentCleanup.tableUpdate(
+    function tableGenerator(): Table {
-      originalTable,
+      return {
-      [{}, { attach: undefined }],
+        name: "table",
-      {
+        sourceId: DEFAULT_BB_DATASOURCE_ID,
-        oldTable: table(),
+        sourceType: TableSourceType.INTERNAL,
+        type: "table",
+        schema: {
+          attach: {
+            name: "attach",
+            type: attachmentFieldType,
+            constraints: {},
+          },
+        },
       }
-    )
+    }
-    expect(mockedDeleteFiles).not.toHaveBeenCalled()
+
-  })
+    beforeEach(() => {
-})
+      mockedDeleteFiles.mockClear()
+    })
+
+    it("should be able to cleanup a table update", async () => {
+      const originalTable = tableGenerator()
+      delete originalTable.schema["attach"]
+      await AttachmentCleanup.tableUpdate(originalTable, [rowGenerator()], {
+        oldTable: tableGenerator(),
+      })
+      expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
+    })
+
+    it("should be able to cleanup a table deletion", async () => {
+      await AttachmentCleanup.tableDelete(tableGenerator(), [rowGenerator()])
+      expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
+    })
+
+    it("should handle table column renaming", async () => {
+      const updatedTable = tableGenerator()
+      updatedTable.schema.attach2 = updatedTable.schema.attach
+      delete updatedTable.schema.attach
+      await AttachmentCleanup.tableUpdate(updatedTable, [rowGenerator()], {
+        oldTable: tableGenerator(),
+        rename: { old: "attach", updated: "attach2" },
+      })
+      expect(mockedDeleteFiles).not.toHaveBeenCalled()
+    })
+
+    it("shouldn't cleanup if no table changes", async () => {
+      await AttachmentCleanup.tableUpdate(tableGenerator(), [rowGenerator()], {
+        oldTable: tableGenerator(),
+      })
+      expect(mockedDeleteFiles).not.toHaveBeenCalled()
+    })
+
+    it("should handle row updates", async () => {
+      const updatedRow = rowGenerator()
+      delete updatedRow.attach
+      await AttachmentCleanup.rowUpdate(tableGenerator(), {
+        row: updatedRow,
+        oldRow: rowGenerator(),
+      })
+      expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
+    })
+
+    it("should handle row deletion", async () => {
+      await AttachmentCleanup.rowDelete(tableGenerator(), [rowGenerator()])
+      expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [FILE_NAME])
+    })
+
+    it("should handle row deletion and not throw when attachments are undefined", async () => {
+      await AttachmentCleanup.rowDelete(tableGenerator(), [
+        {
+          multipleAttachments: undefined,
+        },
+      ])
+    })
+
+    it("shouldn't cleanup attachments if row not updated", async () => {
+      await AttachmentCleanup.rowUpdate(tableGenerator(), {
+        row: rowGenerator(),
+        oldRow: rowGenerator(),
+      })
+      expect(mockedDeleteFiles).not.toHaveBeenCalled()
+    })
+
+    it("should be able to cleanup a column and not throw when attachments are undefined", async () => {
+      const originalTable = tableGenerator()
+      delete originalTable.schema["attach"]
+      await AttachmentCleanup.tableUpdate(
+        originalTable,
+        [rowGenerator("file 1"), { attach: undefined }, rowGenerator("file 2")],
+        {
+          oldTable: tableGenerator(),
+        }
+      )
+      expect(mockedDeleteFiles).toHaveBeenCalledTimes(1)
+      expect(mockedDeleteFiles).toHaveBeenCalledWith(BUCKET, [
+        "file 1",
+        "file 2",
+      ])
+    })
+
+    it("should be able to cleanup a column and not throw when ALL attachments are undefined", async () => {
+      const originalTable = tableGenerator()
+      delete originalTable.schema["attach"]
+      await AttachmentCleanup.tableUpdate(
+        originalTable,
+        [{}, { attach: undefined }],
+        {
+          oldTable: tableGenerator(),
+        }
+      )
+      expect(mockedDeleteFiles).not.toHaveBeenCalled()
+    })
+  }
+)