From 78243edd18d13826075550eb2c70047546fb8cfd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= <ivov.src@gmail.com>
Date: Tue, 24 Oct 2023 10:37:02 +0200
Subject: [PATCH] refactor(core): Make pruning via lifecycle configuration in
 S3 mode mandatory (#7482)

Since we do not store which executions produced binary data, for pruning
on S3 we need to query for binary data items for each execution in order
to delete them. To minimize requests to S3, allow the user to skip
pruning requests when setting TTL at bucket level.
---
 .../repositories/execution.repository.ts          |  7 ++-----
 .../core/src/BinaryData/BinaryData.service.ts     |  2 +-
 .../core/src/BinaryData/ObjectStore.manager.ts    | 13 -------------
 packages/core/src/BinaryData/types.ts             |  5 ++++-
 packages/core/test/ObjectStore.manager.test.ts    | 15 ---------------
 5 files changed, 7 insertions(+), 35 deletions(-)

diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts
index 76a4475a0..ca9694250 100644
--- a/packages/cli/src/databases/repositories/execution.repository.ts
+++ b/packages/cli/src/databases/repositories/execution.repository.ts
@@ -301,10 +301,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
 	 * Permanently remove a single execution and its binary data.
 	 */
 	async hardDelete(ids: { workflowId: string; executionId: string }) {
-		return Promise.all([
-			this.binaryDataService.deleteMany([ids]),
-			this.delete({ id: ids.executionId }),
-		]);
+		return Promise.all([this.delete(ids.executionId), this.binaryDataService.deleteMany([ids])]);
 	}
 
 	async updateExistingExecution(executionId: string, execution: Partial<IExecutionResponse>) {
@@ -565,7 +562,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
 			return;
 		}
 
-		await this.binaryDataService.deleteMany(workflowIdsAndExecutionIds);
+		await this.binaryDataService.deleteMany(workflowIdsAndExecutionIds); // only in FS mode
 
 		this.logger.debug(
 			`Hard-deleting ${executionIds.length} executions from database (pruning cycle)`,
diff --git a/packages/core/src/BinaryData/BinaryData.service.ts b/packages/core/src/BinaryData/BinaryData.service.ts
index 393e42b4d..b96e87d91 100644
--- a/packages/core/src/BinaryData/BinaryData.service.ts
+++ b/packages/core/src/BinaryData/BinaryData.service.ts
@@ -151,7 +151,7 @@ export class BinaryDataService {
 
 		if (!manager) return;
 
-		await manager.deleteMany(ids);
+		if (manager.deleteMany) await manager.deleteMany(ids);
 	}
 
 	@LogCatch((error) =>
diff --git a/packages/core/src/BinaryData/ObjectStore.manager.ts b/packages/core/src/BinaryData/ObjectStore.manager.ts
index 9a6040b1b..37a0f944d 100644
--- a/packages/core/src/BinaryData/ObjectStore.manager.ts
+++ b/packages/core/src/BinaryData/ObjectStore.manager.ts
@@ -83,19 +83,6 @@ export class ObjectStoreManager implements BinaryData.Manager {
 		return { fileId: targetFileId, fileSize: sourceFile.length };
 	}
 
-	async deleteMany(ids: BinaryData.IdsForDeletion) {
-		const prefixes = ids.map(
-			({ workflowId, executionId }) =>
-				`workflows/${workflowId}/executions/${executionId}/binary_data/`,
-		);
-
-		await Promise.all(
-			prefixes.map(async (prefix) => {
-				await this.objectStoreService.deleteMany(prefix);
-			}),
-		);
-	}
-
 	async rename(oldFileId: string, newFileId: string) {
 		const oldFile = await this.objectStoreService.get(oldFileId, { mode: 'buffer' });
 		const oldFileMetadata = await this.objectStoreService.getMetadata(oldFileId);
diff --git a/packages/core/src/BinaryData/types.ts b/packages/core/src/BinaryData/types.ts
index 2067d90c2..ef39197b3 100644
--- a/packages/core/src/BinaryData/types.ts
+++ b/packages/core/src/BinaryData/types.ts
@@ -55,7 +55,10 @@ export namespace BinaryData {
 		getAsStream(fileId: string, chunkSize?: number): Promise<Readable>;
 		getMetadata(fileId: string): Promise<Metadata>;
 
-		deleteMany(ids: IdsForDeletion): Promise<void>;
+		/**
+		 * Present for `FileSystem`, absent for `ObjectStore` (delegated to S3 lifecycle config)
+		 */
+		deleteMany?(ids: IdsForDeletion): Promise<void>;
 
 		copyByFileId(workflowId: string, executionId: string, sourceFileId: string): Promise<string>;
 		copyByFilePath(
diff --git a/packages/core/test/ObjectStore.manager.test.ts b/packages/core/test/ObjectStore.manager.test.ts
index 79fa51910..dc91e3322 100644
--- a/packages/core/test/ObjectStore.manager.test.ts
+++ b/packages/core/test/ObjectStore.manager.test.ts
@@ -116,21 +116,6 @@ describe('copyByFilePath()', () => {
 	});
 });
 
-describe('deleteMany()', () => {
-	it('should delete many files by prefix', async () => {
-		const ids = [
-			{ workflowId, executionId },
-			{ workflowId: otherWorkflowId, executionId: otherExecutionId },
-		];
-
-		const promise = objectStoreManager.deleteMany(ids);
-
-		await expect(promise).resolves.not.toThrow();
-
-		expect(objectStoreService.deleteMany).toHaveBeenCalledTimes(2);
-	});
-});
-
 describe('rename()', () => {
 	it('should rename a file', async () => {
 		const promise = objectStoreManager.rename(fileId, otherFileId);