From b2ab4e022e9ff9d1f8b8e54da1e3615bec8a9001 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 4 Apr 2024 20:39:11 +0200 Subject: [PATCH] Handle singleattachment on AttachmentCleanup --- .../src/utilities/rowProcessor/attachments.ts | 60 +++- .../rowProcessor/tests/attachments.spec.ts | 262 ++++++++++-------- 2 files changed, 193 insertions(+), 129 deletions(-) diff --git a/packages/server/src/utilities/rowProcessor/attachments.ts b/packages/server/src/utilities/rowProcessor/attachments.ts index 652851a48b..9204a6640f 100644 --- a/packages/server/src/utilities/rowProcessor/attachments.ts +++ b/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( - (attachment: RowAttachment) => attachment.key - ) || [] - const newKeys = - opts.row[key]?.map((attachment: RowAttachment) => attachment.key) || - [] + + const oldKeys = AttachmentCleanup.extractAttachmentKeys( + schema.type, + opts.oldRow[key] + ) + const newKeys = AttachmentCleanup.extractAttachmentKeys( + schema.type, + opts.row[key] + ) files = files.concat( oldKeys.filter((key: string) => newKeys.indexOf(key) === -1) ) diff --git a/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts b/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts index 1b36a4cb81..3ef8c71afc 100644 --- a/packages/server/src/utilities/rowProcessor/tests/attachments.spec.ts +++ b/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 { - return { - name: "table", - sourceId: DEFAULT_BB_DATASOURCE_ID, - sourceType: TableSourceType.INTERNAL, - type: "table", - schema: { - attach: { - name: "attach", - type: FieldType.ATTACHMENTS, - constraints: {}, - }, +const rowGenerators: [ + string, + FieldType.ATTACHMENT_SINGLE | FieldType.ATTACHMENTS, + (fileKey?: string) => Row +][] = [ + [ + "row with a attachment list column", + FieldType.ATTACHMENTS, + function rowWithAttachments(fileKey: string = FILE_NAME): Row { + return { + attach: [ + { + size: 1, + extension: "jpg", + key: fileKey, + }, + ], + } }, - } -} - -function row(fileKey: string = FILE_NAME): Row { - return { - attach: [ - { - size: 1, - extension: "jpg", - 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(), + ], + [ + "row with a single attachment column", + FieldType.ATTACHMENT_SINGLE, + function rowWithAttachments(fileKey: string = FILE_NAME): Row { + return { + attach: { + size: 1, + extension: "jpg", + key: fileKey, + }, } - ) - 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 = table() - delete originalTable.schema["attach"] - await AttachmentCleanup.tableUpdate( - originalTable, - [{}, { attach: undefined }], - { - oldTable: table(), +describe.each(rowGenerators)( + "attachment cleanup", + (_, attachmentFieldType, rowGenerator) => { + function tableGenerator(): Table { + return { + name: "table", + sourceId: DEFAULT_BB_DATASOURCE_ID, + 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() + }) + } +)