From 29f1d11ab96a3dfb5498cfaff901b434bd6aa3be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 31 Aug 2023 19:49:37 +0200 Subject: [PATCH 01/38] refactor(core): Simplify executions and binary data pruning --- .../handlers/executions/executions.handler.ts | 4 - .../handlers/executions/executions.service.ts | 4 +- packages/cli/src/Server.ts | 1 - .../cli/src/WorkflowExecuteAdditionalData.ts | 88 +----------- packages/cli/src/WorkflowRunner.ts | 2 +- packages/cli/src/config/schema.ts | 12 -- .../repositories/execution.repository.ts | 127 ++++++++++++++---- .../cli/src/executions/executions.service.ts | 12 +- .../core/src/BinaryDataManager/FileSystem.ts | 56 +------- packages/core/src/BinaryDataManager/index.ts | 16 --- packages/core/src/Interfaces.ts | 3 - .../core/test/NodeExecuteFunctions.test.ts | 2 - packages/nodes-base/test/nodes/Helpers.ts | 1 - 13 files changed, 117 insertions(+), 211 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts index 03b70556e0970..9a5c419b7deef 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts @@ -1,7 +1,5 @@ import type express from 'express'; -import { BinaryDataManager } from 'n8n-core'; - import { getExecutions, getExecutionInWorkflows, @@ -37,8 +35,6 @@ export = { return res.status(404).json({ message: 'Not Found' }); } - await BinaryDataManager.getInstance().deleteBinaryDataByExecutionIds([execution.id!]); - await deleteExecution(execution); execution.id = id; diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts index 6061df800ca97..6ab0fdfc8519e 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts @@ -110,6 +110,6 @@ export async function getExecutionInWorkflows( }); } -export async function deleteExecution(execution: IExecutionBase): Promise { - return Container.get(ExecutionRepository).deleteExecution(execution.id as string); +export async function deleteExecution(execution: IExecutionBase): Promise { + return Container.get(ExecutionRepository).softDeleteExecution(execution.id as string); } diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 7500ef17c0bd7..09a464589999a 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -393,7 +393,6 @@ export class Server extends AbstractServer { ), executions_data_prune: config.getEnv('executions.pruneData'), executions_data_max_age: config.getEnv('executions.pruneDataMaxAge'), - executions_data_prune_timeout: config.getEnv('executions.pruneDataTimeout'), }, deploymentType: config.getEnv('deployment.type'), binaryDataMode: binaryDataConfig.mode, diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 5760cce3d1a6d..903daba4775c6 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -9,7 +9,7 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ -import { BinaryDataManager, UserSettings, WorkflowExecute } from 'n8n-core'; +import { UserSettings, WorkflowExecute } from 'n8n-core'; import type { IDataObject, @@ -38,9 +38,6 @@ import { import pick from 'lodash/pick'; import { Container } from 'typedi'; -import type { FindOptionsWhere } from 'typeorm'; -import { LessThanOrEqual, In } from 'typeorm'; -import { DateUtils } from 'typeorm/util/DateUtils'; import config from '@/config'; import * as Db from '@/Db'; import { ActiveExecutions } from '@/ActiveExecutions'; @@ -48,7 +45,6 @@ import { CredentialsHelper } from '@/CredentialsHelper'; import { ExternalHooks } from '@/ExternalHooks'; import type { IExecutionDb, - IExecutionFlattedDb, IPushDataExecutionFinished, IWorkflowExecuteProcess, IWorkflowExecutionDataProcess, @@ -181,77 +177,6 @@ export function executeErrorWorkflow( } } -/** - * Prunes Saved Execution which are older than configured. - * Throttled to be executed just once in configured timeframe. - * TODO: Consider moving this whole function to the repository or at least the queries - */ -let throttling = false; -async function pruneExecutionData(this: WorkflowHooks): Promise { - if (!throttling) { - Logger.verbose('Pruning execution data from database'); - - throttling = true; - const timeout = config.getEnv('executions.pruneDataTimeout'); // in seconds - const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h - const maxCount = config.getEnv('executions.pruneDataMaxCount'); - const date = new Date(); // today - date.setHours(date.getHours() - maxAge); - - // date reformatting needed - see https://github.com/typeorm/typeorm/issues/2286 - - const utcDate = DateUtils.mixedDateToUtcDatetimeString(date); - - const toPrune: Array> = [ - { stoppedAt: LessThanOrEqual(utcDate) }, - ]; - - if (maxCount > 0) { - const executions = await Db.collections.Execution.find({ - select: ['id'], - skip: maxCount, - take: 1, - order: { id: 'DESC' }, - }); - - if (executions[0]) { - toPrune.push({ id: LessThanOrEqual(executions[0].id) }); - } - } - - try { - setTimeout(() => { - throttling = false; - }, timeout * 1000); - let executionIds: Array; - do { - executionIds = ( - await Db.collections.Execution.find({ - select: ['id'], - where: toPrune, - take: 100, - }) - ).map(({ id }) => id); - await Db.collections.Execution.delete({ id: In(executionIds) }); - // Mark binary data for deletion for all executions - await BinaryDataManager.getInstance().markDataForDeletionByExecutionIds(executionIds); - } while (executionIds.length > 0); - } catch (error) { - ErrorReporter.error(error); - throttling = false; - Logger.error( - `Failed pruning execution data from database for execution ID ${this.executionId} (hookFunctionsSave)`, - { - ...error, - executionId: this.executionId, - sessionId: this.sessionId, - workflowId: this.workflowData.id, - }, - ); - } - } -} - export async function saveExecutionMetadata( executionId: string, executionMetadata: Record, @@ -535,11 +460,6 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { workflowId: this.workflowData.id, }); - // Prune old execution data - if (config.getEnv('executions.pruneData')) { - await pruneExecutionData.call(this); - } - const isManualMode = [this.mode, parentProcessMode].includes('manual'); try { @@ -567,8 +487,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { } if (isManualMode && !saveManualExecutions && !fullRunData.waitTill) { - // Data is always saved, so we remove from database - await Container.get(ExecutionRepository).deleteExecution(this.executionId, true); + await Container.get(ExecutionRepository).softDeleteExecution(this.executionId); return; } @@ -606,8 +525,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { this.executionId, this.retryOf, ); - // Data is always saved, so we remove from database - await Container.get(ExecutionRepository).deleteExecution(this.executionId); + await Container.get(ExecutionRepository).softDeleteExecution(this.executionId); return; } diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index b7e4b4eaacb48..5103e4dafe27e 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -601,7 +601,7 @@ export class WorkflowRunner { (workflowDidSucceed && saveDataSuccessExecution === 'none') || (!workflowDidSucceed && saveDataErrorExecution === 'none') ) { - await Container.get(ExecutionRepository).deleteExecution(executionId); + await Container.get(ExecutionRepository).softDeleteExecution(executionId); } // eslint-disable-next-line id-denylist } catch (err) { diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 157e9c2a95723..586fba82b973e 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -316,12 +316,6 @@ export const schema = { default: 336, env: 'EXECUTIONS_DATA_MAX_AGE', }, - pruneDataTimeout: { - doc: 'Timeout (seconds) after execution data has been pruned', - format: Number, - default: 3600, - env: 'EXECUTIONS_DATA_PRUNE_TIMEOUT', - }, // Additional pruning option to delete executions if total count exceeds the configured max. // Deletes the oldest entries first @@ -907,12 +901,6 @@ export const schema = { env: 'N8N_BINARY_DATA_STORAGE_PATH', doc: 'Path for binary data storage in "filesystem" mode', }, - binaryDataTTL: { - format: Number, - default: 60, - env: 'N8N_BINARY_DATA_TTL', - doc: 'TTL for binary data of unsaved executions in minutes', - }, }, deployment: { diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 07215d4701908..ce4fb0d1bae5b 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -1,29 +1,33 @@ import { Service } from 'typedi'; import { DataSource, In, LessThanOrEqual, MoreThanOrEqual, Repository } from 'typeorm'; +import { DateUtils } from 'typeorm/util/DateUtils'; import type { FindManyOptions, FindOneOptions, FindOptionsWhere, SelectQueryBuilder, } from 'typeorm'; -import { ExecutionEntity } from '../entities/ExecutionEntity'; import { parse, stringify } from 'flatted'; +import { LoggerProxy as Logger } from 'n8n-workflow'; +import type { IExecutionsSummary, IRunExecutionData } from 'n8n-workflow'; +import { BinaryDataManager } from 'n8n-core'; import type { IExecutionBase, IExecutionDb, IExecutionFlattedDb, IExecutionResponse, } from '@/Interfaces'; -import { LoggerProxy } from 'n8n-workflow'; -import type { IExecutionsSummary, IRunExecutionData } from 'n8n-workflow'; -import { ExecutionDataRepository } from './executionData.repository'; -import type { ExecutionData } from '../entities/ExecutionData'; + +import config from '@/config'; import type { IGetExecutionsQueryFilter } from '@/executions/executions.service'; import { isAdvancedExecutionFiltersEnabled } from '@/executions/executionHelpers'; +import type { ExecutionData } from '../entities/ExecutionData'; +import { ExecutionEntity } from '../entities/ExecutionEntity'; import { ExecutionMetadata } from '../entities/ExecutionMetadata'; -import { DateUtils } from 'typeorm/util/DateUtils'; -import { BinaryDataManager } from 'n8n-core'; -import config from '@/config'; +import { ExecutionDataRepository } from './executionData.repository'; +import { inTest } from '@/constants'; + +const PRUNING_BATCH_SIZE = 100; function parseFiltersToQueryBuilder( qb: SelectQueryBuilder, @@ -71,6 +75,14 @@ export class ExecutionRepository extends Repository { private readonly executionDataRepository: ExecutionDataRepository, ) { super(ExecutionEntity, dataSource.manager); + + if (!inTest) { + if (config.getEnv('executions.pruneData')) { + setInterval(async () => this.pruneOlderExecutions(), 60 * 60 * 1000); // Every hour + } + + setInterval(async () => this.deleteSoftDeletedExecutions(), 15 * 60 * 1000); // Every 15 minutes + } } async findMultipleExecutions( @@ -238,16 +250,6 @@ export class ExecutionRepository extends Repository { } } - async deleteExecution(executionId: string, deferBinaryDataDeletion = false) { - const binaryDataManager = BinaryDataManager.getInstance(); - if (deferBinaryDataDeletion) { - await binaryDataManager.markDataForDeletionByExecutionId(executionId); - } else { - await binaryDataManager.deleteBinaryDataByExecutionIds([executionId]); - } - return this.delete({ id: executionId }); - } - async countExecutions( filters: IGetExecutionsQueryFilter | undefined, accessibleWorkflowIds: string[], @@ -287,7 +289,7 @@ export class ExecutionRepository extends Repository { } } catch (error) { if (error instanceof Error) { - LoggerProxy.warn(`Failed to get executions count from Postgres: ${error.message}`, { + Logger.warn(`Failed to get executions count from Postgres: ${error.message}`, { error, }); } @@ -357,7 +359,15 @@ export class ExecutionRepository extends Repository { }); } - async deleteExecutions( + async softDeleteExecution(executionId: string) { + await this.update({ id: executionId }, { deletedAt: Date.now() }); + } + + async softDeleteExecutions(executionIds: string[]) { + await this.update({ id: In(executionIds) }, { deletedAt: Date.now() }); + } + + async deleteExecutionsByFilter( filters: IGetExecutionsQueryFilter | undefined, accessibleWorkflowIds: string[], deleteConditions: { @@ -389,7 +399,7 @@ export class ExecutionRepository extends Repository { if (!executions.length) { if (deleteConditions.ids) { - LoggerProxy.error('Failed to delete an execution due to insufficient permissions', { + Logger.error('Failed to delete an execution due to insufficient permissions', { executionIds: deleteConditions.ids, }); } @@ -397,13 +407,78 @@ export class ExecutionRepository extends Repository { } const executionIds = executions.map(({ id }) => id); - const binaryDataManager = BinaryDataManager.getInstance(); - await binaryDataManager.deleteBinaryDataByExecutionIds(executionIds); - do { // Delete in batches to avoid "SQLITE_ERROR: Expression tree is too large (maximum depth 1000)" error - const batch = executionIds.splice(0, 500); - await this.delete(batch); + const batch = executionIds.splice(0, PRUNING_BATCH_SIZE); + await this.softDeleteExecutions(batch); } while (executionIds.length > 0); } + + private async pruneOlderExecutions() { + Logger.verbose('Pruning execution data from database'); + + const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h + const maxCount = config.getEnv('executions.pruneDataMaxCount'); + + // Find ids of all executions that were stopped longer that pruneDataMaxAge ago + const date = new Date(); + date.setHours(date.getHours() - maxAge); + + const toPrune: Array> = [ + // date reformatting needed - see https://github.com/typeorm/typeorm/issues/2286 + { stoppedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)) }, + ]; + + if (maxCount > 0) { + const executions = await this.find({ + select: ['id'], + skip: maxCount, + take: 1, + order: { id: 'DESC' }, + }); + + if (executions[0]) { + toPrune.push({ id: LessThanOrEqual(executions[0].id) }); + } + } + + const executionIds = ( + await this.find({ + select: ['id'], + where: toPrune, + take: PRUNING_BATCH_SIZE, + }) + ).map(({ id }) => id); + await this.softDeleteExecutions(executionIds); + + if (executionIds.length === PRUNING_BATCH_SIZE) { + setTimeout(async () => this.pruneOlderExecutions(), 1000); + } + } + + private async deleteSoftDeletedExecutions() { + // Find ids of all executions that were deleted over an hour ago + const date = new Date(); + date.setHours(date.getHours() - 1); + + const executionIds = ( + await this.find({ + select: ['id'], + where: { + deletedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)), + }, + take: PRUNING_BATCH_SIZE, + }) + ).map(({ id }) => id); + + const binaryDataManager = BinaryDataManager.getInstance(); + await binaryDataManager.deleteBinaryDataByExecutionIds(executionIds); + + // Actually delete these executions + await this.delete({ id: In(executionIds) }); + + if (executionIds.length === PRUNING_BATCH_SIZE) { + setTimeout(async () => this.deleteSoftDeletedExecutions(), 1000); + } + } } diff --git a/packages/cli/src/executions/executions.service.ts b/packages/cli/src/executions/executions.service.ts index aab68b449d4f5..d13776c5d20d8 100644 --- a/packages/cli/src/executions/executions.service.ts +++ b/packages/cli/src/executions/executions.service.ts @@ -351,9 +351,13 @@ export class ExecutionsService { } } - return Container.get(ExecutionRepository).deleteExecutions(requestFilters, sharedWorkflowIds, { - deleteBefore, - ids, - }); + return Container.get(ExecutionRepository).deleteExecutionsByFilter( + requestFilters, + sharedWorkflowIds, + { + deleteBefore, + ids, + }, + ); } } diff --git a/packages/core/src/BinaryDataManager/FileSystem.ts b/packages/core/src/BinaryDataManager/FileSystem.ts index cf9f1d94de220..6ae70271211b9 100644 --- a/packages/core/src/BinaryDataManager/FileSystem.ts +++ b/packages/core/src/BinaryDataManager/FileSystem.ts @@ -1,4 +1,3 @@ -import glob from 'fast-glob'; import { createReadStream } from 'fs'; import fs from 'fs/promises'; import path from 'path'; @@ -10,32 +9,19 @@ import { jsonParse } from 'n8n-workflow'; import type { IBinaryDataConfig, IBinaryDataManager } from '../Interfaces'; import { FileNotFoundError } from '../errors'; -const PREFIX_METAFILE = 'binarymeta'; - const executionExtractionRegexp = /^(\w+)(?:[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12})$/; export class BinaryDataFileSystem implements IBinaryDataManager { private storagePath: string; - private binaryDataTTL: number; - constructor(config: IBinaryDataConfig) { this.storagePath = config.localStoragePath; - this.binaryDataTTL = config.binaryDataTTL; } - async init(startPurger = false): Promise { - if (startPurger) { - setInterval(async () => { - await this.deleteMarkedFiles(); - }, this.binaryDataTTL * 30000); - } - + async init(): Promise { await this.assertFolder(this.storagePath); await this.assertFolder(this.getBinaryDataMetaPath()); - - await this.deleteMarkedFiles(); } async getFileSize(identifier: string): Promise { @@ -81,47 +67,8 @@ export class BinaryDataFileSystem implements IBinaryDataManager { return this.resolveStoragePath(`${identifier}.metadata`); } - async markDataForDeletionByExecutionId(executionId: string): Promise { - const tt = new Date(new Date().getTime() + this.binaryDataTTL * 60000); - return fs.writeFile( - this.resolveStoragePath('meta', `${PREFIX_METAFILE}_${executionId}_${tt.valueOf()}`), - '', - ); - } - - async deleteMarkedFiles(): Promise { - return this.deleteMarkedFilesByMeta(this.getBinaryDataMetaPath(), PREFIX_METAFILE); - } - - private async deleteMarkedFilesByMeta(metaPath: string, filePrefix: string): Promise { - const currentTimeValue = new Date().valueOf(); - const metaFileNames = await glob(`${filePrefix}_*`, { cwd: metaPath }); - - const executionIds = metaFileNames - .map((f) => f.split('_') as [string, string, string]) - .filter(([prefix, , ts]) => { - if (prefix !== filePrefix) return false; - const execTimestamp = parseInt(ts, 10); - return execTimestamp < currentTimeValue; - }) - .map((e) => e[1]); - - const filesToDelete = []; - const deletedIds = await this.deleteBinaryDataByExecutionIds(executionIds); - for (const executionId of deletedIds) { - filesToDelete.push( - ...(await glob(`${filePrefix}_${executionId}_`, { - absolute: true, - cwd: metaPath, - })), - ); - } - await Promise.all(filesToDelete.map(async (file) => fs.rm(file))); - } - async duplicateBinaryDataByIdentifier(binaryDataId: string, prefix: string): Promise { const newBinaryDataId = this.generateFileName(prefix); - await fs.copyFile( this.resolveStoragePath(binaryDataId), this.resolveStoragePath(newBinaryDataId), @@ -130,6 +77,7 @@ export class BinaryDataFileSystem implements IBinaryDataManager { } async deleteBinaryDataByExecutionIds(executionIds: string[]): Promise { + // TODO: switch over to new folder structure, and delete folders instead const set = new Set(executionIds); const fileNames = await fs.readdir(this.storagePath); const deletedIds = []; diff --git a/packages/core/src/BinaryDataManager/index.ts b/packages/core/src/BinaryDataManager/index.ts index 7bfbf614c997b..eca32f184da26 100644 --- a/packages/core/src/BinaryDataManager/index.ts +++ b/packages/core/src/BinaryDataManager/index.ts @@ -156,22 +156,6 @@ export class BinaryDataManager { throw new Error('Storage mode used to store binary data not available'); } - async markDataForDeletionByExecutionId(executionId: string): Promise { - if (this.managers[this.binaryDataMode]) { - await this.managers[this.binaryDataMode].markDataForDeletionByExecutionId(executionId); - } - } - - async markDataForDeletionByExecutionIds(executionIds: string[]): Promise { - if (this.managers[this.binaryDataMode]) { - await Promise.all( - executionIds.map(async (id) => - this.managers[this.binaryDataMode].markDataForDeletionByExecutionId(id), - ), - ); - } - } - async deleteBinaryDataByExecutionIds(executionIds: string[]): Promise { if (this.managers[this.binaryDataMode]) { await this.managers[this.binaryDataMode].deleteBinaryDataByExecutionIds(executionIds); diff --git a/packages/core/src/Interfaces.ts b/packages/core/src/Interfaces.ts index d01b655bf1e8a..f1f3f4ff7a5bd 100644 --- a/packages/core/src/Interfaces.ts +++ b/packages/core/src/Interfaces.ts @@ -38,7 +38,6 @@ export interface IBinaryDataConfig { mode: 'default' | 'filesystem'; availableModes: string; localStoragePath: string; - binaryDataTTL: number; } export interface IBinaryDataManager { @@ -51,8 +50,6 @@ export interface IBinaryDataManager { retrieveBinaryDataByIdentifier(identifier: string): Promise; getBinaryPath(identifier: string): string; getBinaryStream(identifier: string, chunkSize?: number): Readable; - markDataForDeletionByExecutionId(executionId: string): Promise; - deleteMarkedFiles(): Promise; deleteBinaryDataByIdentifier(identifier: string): Promise; duplicateBinaryDataByIdentifier(binaryDataId: string, prefix: string): Promise; deleteBinaryDataByExecutionIds(executionIds: string[]): Promise; diff --git a/packages/core/test/NodeExecuteFunctions.test.ts b/packages/core/test/NodeExecuteFunctions.test.ts index d981c1d246d98..ec2a32e7f0e54 100644 --- a/packages/core/test/NodeExecuteFunctions.test.ts +++ b/packages/core/test/NodeExecuteFunctions.test.ts @@ -34,7 +34,6 @@ describe('NodeExecuteFunctions', () => { mode: 'default', availableModes: 'default', localStoragePath: temporaryDir, - binaryDataTTL: 1, }); // Set our binary data buffer @@ -84,7 +83,6 @@ describe('NodeExecuteFunctions', () => { mode: 'filesystem', availableModes: 'filesystem', localStoragePath: temporaryDir, - binaryDataTTL: 1, }); // Set our binary data buffer diff --git a/packages/nodes-base/test/nodes/Helpers.ts b/packages/nodes-base/test/nodes/Helpers.ts index ecc87dbb556d9..d14ef99577011 100644 --- a/packages/nodes-base/test/nodes/Helpers.ts +++ b/packages/nodes-base/test/nodes/Helpers.ts @@ -222,7 +222,6 @@ export async function initBinaryDataManager(mode: 'default' | 'filesystem' = 'de mode, availableModes: mode, localStoragePath: temporaryDir, - binaryDataTTL: 1, }); return temporaryDir; } From 05e3fefd0cc19552f01dbc8a4f51a069c5203ffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 11:19:53 +0200 Subject: [PATCH 02/38] Remove test code from service --- .../databases/repositories/execution.repository.ts | 11 ++++------- .../cli/test/integration/shared/utils/testServer.ts | 1 + 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 620027075b458..ddb483fd89f76 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -25,7 +25,6 @@ import type { ExecutionData } from '../entities/ExecutionData'; import { ExecutionEntity } from '../entities/ExecutionEntity'; import { ExecutionMetadata } from '../entities/ExecutionMetadata'; import { ExecutionDataRepository } from './executionData.repository'; -import { inTest } from '@/constants'; const PRUNING_BATCH_SIZE = 100; @@ -76,13 +75,11 @@ export class ExecutionRepository extends Repository { ) { super(ExecutionEntity, dataSource.manager); - if (!inTest) { - if (config.getEnv('executions.pruneData')) { - setInterval(async () => this.pruneOlderExecutions(), 60 * 60 * 1000); // Every hour - } - - setInterval(async () => this.deleteSoftDeletedExecutions(), 15 * 60 * 1000); // Every 15 minutes + if (config.getEnv('executions.pruneData')) { + setInterval(async () => this.pruneOlderExecutions(), 60 * 60 * 1000); // Every hour } + + setInterval(async () => this.deleteSoftDeletedExecutions(), 15 * 60 * 1000); // Every 15 minutes } async findMultipleExecutions( diff --git a/packages/cli/test/integration/shared/utils/testServer.ts b/packages/cli/test/integration/shared/utils/testServer.ts index b00273eb66958..54e1af53babb4 100644 --- a/packages/cli/test/integration/shared/utils/testServer.ts +++ b/packages/cli/test/integration/shared/utils/testServer.ts @@ -152,6 +152,7 @@ export const setupTestServer = ({ config.set('userManagement.jwtSecret', 'My JWT secret'); config.set('userManagement.isInstanceOwnerSetUp', true); + config.set('executions.pruneData', false); if (enabledFeatures) { Container.get(License).isFeatureEnabled = (feature) => enabledFeatures.includes(feature); From 9dabbe86a8247edc9864bced3c972062c9c3d7c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 11:24:40 +0200 Subject: [PATCH 03/38] Use time constants --- packages/cli/src/constants.ts | 6 ++++++ .../cli/src/databases/repositories/execution.repository.ts | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 8d9bbb9e14f2b..ec66cbdf32062 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -94,3 +94,9 @@ export const CREDENTIAL_BLANKING_VALUE = '__n8n_BLANK_VALUE_e5362baf-c777-4d57-a export const UM_FIX_INSTRUCTION = 'Please fix the database by running ./packages/cli/bin/n8n user-management:reset'; + +export const TIME = { + SECOND: 1000, + MINUTE: 60 * 1000, + HOUR: 60 * 60 * 1000, +}; diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index ddb483fd89f76..981eacad3f1b7 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -25,6 +25,7 @@ import type { ExecutionData } from '../entities/ExecutionData'; import { ExecutionEntity } from '../entities/ExecutionEntity'; import { ExecutionMetadata } from '../entities/ExecutionMetadata'; import { ExecutionDataRepository } from './executionData.repository'; +import { TIME } from '@/constants'; const PRUNING_BATCH_SIZE = 100; @@ -76,10 +77,10 @@ export class ExecutionRepository extends Repository { super(ExecutionEntity, dataSource.manager); if (config.getEnv('executions.pruneData')) { - setInterval(async () => this.pruneOlderExecutions(), 60 * 60 * 1000); // Every hour + setInterval(async () => this.pruneOlderExecutions(), TIME.HOUR); } - setInterval(async () => this.deleteSoftDeletedExecutions(), 15 * 60 * 1000); // Every 15 minutes + setInterval(async () => this.deleteSoftDeletedExecutions(), 15 * TIME.MINUTE); } async findMultipleExecutions( From 3f7de8e044c8ac9c682c45acb6fd8b25a3dcb069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 11:26:38 +0200 Subject: [PATCH 04/38] Improve naming --- .../cli/src/databases/repositories/execution.repository.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 981eacad3f1b7..399999cf6bd4b 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -77,7 +77,7 @@ export class ExecutionRepository extends Repository { super(ExecutionEntity, dataSource.manager); if (config.getEnv('executions.pruneData')) { - setInterval(async () => this.pruneOlderExecutions(), TIME.HOUR); + setInterval(async () => this.pruneBySoftDeleting(), TIME.HOUR); } setInterval(async () => this.deleteSoftDeletedExecutions(), 15 * TIME.MINUTE); @@ -422,7 +422,7 @@ export class ExecutionRepository extends Repository { } while (executionIds.length > 0); } - private async pruneOlderExecutions() { + private async pruneBySoftDeleting() { Logger.verbose('Pruning execution data from database'); const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h @@ -460,7 +460,7 @@ export class ExecutionRepository extends Repository { await this.softDeleteExecutions(executionIds); if (executionIds.length === PRUNING_BATCH_SIZE) { - setTimeout(async () => this.pruneOlderExecutions(), 1000); + setTimeout(async () => this.pruneBySoftDeleting(), 1000); } } From 5583814938af16a84cd3395a3a3b29224b8dde03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 11:51:11 +0200 Subject: [PATCH 05/38] Use native typeorm soft deletion --- .../v1/handlers/executions/executions.handler.ts | 12 ++++-------- .../v1/handlers/executions/executions.service.ts | 6 +----- .../cli/src/WorkflowExecuteAdditionalData.ts | 5 ++--- packages/cli/src/WorkflowRunner.ts | 2 +- .../src/databases/entities/ExecutionEntity.ts | 3 ++- .../repositories/execution.repository.ts | 16 ++++------------ 6 files changed, 14 insertions(+), 30 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts index 9a5c419b7deef..90ed886bfa62a 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts @@ -1,11 +1,6 @@ import type express from 'express'; -import { - getExecutions, - getExecutionInWorkflows, - deleteExecution, - getExecutionsCount, -} from './executions.service'; +import { getExecutions, getExecutionInWorkflows, getExecutionsCount } from './executions.service'; import { ActiveExecutions } from '@/ActiveExecutions'; import { authorize, validCursor } from '../../shared/middlewares/global.middleware'; import type { ExecutionRequest } from '../../../types'; @@ -13,6 +8,7 @@ import { getSharedWorkflowIds } from '../workflows/workflows.service'; import { encodeNextCursor } from '../../shared/services/pagination.service'; import { Container } from 'typedi'; import { InternalHooks } from '@/InternalHooks'; +import { ExecutionRepository } from '@/databases/repositories'; export = { deleteExecution: [ @@ -31,11 +27,11 @@ export = { // look for the execution on the workflow the user owns const execution = await getExecutionInWorkflows(id, sharedWorkflowsIds, false); - if (!execution) { + if (!execution?.id) { return res.status(404).json({ message: 'Not Found' }); } - await deleteExecution(execution); + await Container.get(ExecutionRepository).softDelete(execution.id); execution.id = id; diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts index 6ab0fdfc8519e..de08c9095a601 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts @@ -1,4 +1,4 @@ -import type { DeleteResult, FindOptionsWhere } from 'typeorm'; +import type { FindOptionsWhere } from 'typeorm'; import { In, Not, Raw, LessThan } from 'typeorm'; import { Container } from 'typedi'; import type { ExecutionStatus } from 'n8n-workflow'; @@ -109,7 +109,3 @@ export async function getExecutionInWorkflows( unflattenData: true, }); } - -export async function deleteExecution(execution: IExecutionBase): Promise { - return Container.get(ExecutionRepository).softDeleteExecution(execution.id as string); -} diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 59aac1192d002..d19bae6e25009 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -39,7 +39,6 @@ import { import pick from 'lodash/pick'; import { Container } from 'typedi'; import config from '@/config'; -import * as Db from '@/Db'; import { ActiveExecutions } from '@/ActiveExecutions'; import { CredentialsHelper } from '@/CredentialsHelper'; import { ExternalHooks } from '@/ExternalHooks'; @@ -471,7 +470,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { } if (isManualMode && !saveManualExecutions && !fullRunData.waitTill) { - await Container.get(ExecutionRepository).softDeleteExecution(this.executionId); + await Container.get(ExecutionRepository).softDelete(this.executionId); return; } @@ -509,7 +508,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { this.executionId, this.retryOf, ); - await Container.get(ExecutionRepository).softDeleteExecution(this.executionId); + await Container.get(ExecutionRepository).softDelete(this.executionId); return; } diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 5103e4dafe27e..2f730af402480 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -601,7 +601,7 @@ export class WorkflowRunner { (workflowDidSucceed && saveDataSuccessExecution === 'none') || (!workflowDidSucceed && saveDataErrorExecution === 'none') ) { - await Container.get(ExecutionRepository).softDeleteExecution(executionId); + await Container.get(ExecutionRepository).softDelete(executionId); } // eslint-disable-next-line id-denylist } catch (err) { diff --git a/packages/cli/src/databases/entities/ExecutionEntity.ts b/packages/cli/src/databases/entities/ExecutionEntity.ts index f71bf0ba57988..d73267ee52003 100644 --- a/packages/cli/src/databases/entities/ExecutionEntity.ts +++ b/packages/cli/src/databases/entities/ExecutionEntity.ts @@ -9,6 +9,7 @@ import { OneToOne, PrimaryColumn, Relation, + DeleteDateColumn, } from 'typeorm'; import { datetimeColumnType } from './AbstractEntity'; import { idStringifier } from '../utils/transformers'; @@ -49,7 +50,7 @@ export class ExecutionEntity { @Column({ type: datetimeColumnType, nullable: true }) stoppedAt: Date; - @Column(datetimeColumnType) + @DeleteDateColumn({ type: datetimeColumnType, nullable: true }) deletedAt: Date; @Column({ nullable: true }) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 399999cf6bd4b..bc29057b828b6 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -367,14 +367,6 @@ export class ExecutionRepository extends Repository { }); } - async softDeleteExecution(executionId: string) { - await this.update({ id: executionId }, { deletedAt: Date.now() }); - } - - async softDeleteExecutions(executionIds: string[]) { - await this.update({ id: In(executionIds) }, { deletedAt: Date.now() }); - } - async deleteExecutionsByFilter( filters: IGetExecutionsQueryFilter | undefined, accessibleWorkflowIds: string[], @@ -418,12 +410,12 @@ export class ExecutionRepository extends Repository { do { // Delete in batches to avoid "SQLITE_ERROR: Expression tree is too large (maximum depth 1000)" error const batch = executionIds.splice(0, PRUNING_BATCH_SIZE); - await this.softDeleteExecutions(batch); + await this.softDelete(batch); } while (executionIds.length > 0); } private async pruneBySoftDeleting() { - Logger.verbose('Pruning execution data from database'); + Logger.verbose('Pruning (soft-deleting) execution data from database'); const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h const maxCount = config.getEnv('executions.pruneDataMaxCount'); @@ -457,10 +449,10 @@ export class ExecutionRepository extends Repository { take: PRUNING_BATCH_SIZE, }) ).map(({ id }) => id); - await this.softDeleteExecutions(executionIds); + await this.softDelete(executionIds); if (executionIds.length === PRUNING_BATCH_SIZE) { - setTimeout(async () => this.pruneBySoftDeleting(), 1000); + setTimeout(async () => this.pruneBySoftDeleting(), TIME.SECOND); } } From f50108a0cbc7a6d3b8c3479acf57434568ac282e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 12:00:26 +0200 Subject: [PATCH 06/38] Improve naming --- .../src/databases/repositories/execution.repository.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index bc29057b828b6..518adc8dbe6a4 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -80,7 +80,7 @@ export class ExecutionRepository extends Repository { setInterval(async () => this.pruneBySoftDeleting(), TIME.HOUR); } - setInterval(async () => this.deleteSoftDeletedExecutions(), 15 * TIME.MINUTE); + setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); } async findMultipleExecutions( @@ -456,7 +456,10 @@ export class ExecutionRepository extends Repository { } } - private async deleteSoftDeletedExecutions() { + /** + * Permanently delete all soft-deleted executions and their binary data, in batches. + */ + private async hardDelete() { // Find ids of all executions that were deleted over an hour ago const date = new Date(); date.setHours(date.getHours() - 1); @@ -478,7 +481,7 @@ export class ExecutionRepository extends Repository { await this.delete({ id: In(executionIds) }); if (executionIds.length === PRUNING_BATCH_SIZE) { - setTimeout(async () => this.deleteSoftDeletedExecutions(), 1000); + setTimeout(async () => this.hardDelete(), 1000); } } } From 85ac53a4316011a98f0c3f036ae77b8d2ca90e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 12:01:49 +0200 Subject: [PATCH 07/38] Make batch size class field --- .../databases/repositories/execution.repository.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 518adc8dbe6a4..d1c0feef41345 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -27,8 +27,6 @@ import { ExecutionMetadata } from '../entities/ExecutionMetadata'; import { ExecutionDataRepository } from './executionData.repository'; import { TIME } from '@/constants'; -const PRUNING_BATCH_SIZE = 100; - function parseFiltersToQueryBuilder( qb: SelectQueryBuilder, filters?: IGetExecutionsQueryFilter, @@ -70,6 +68,8 @@ function parseFiltersToQueryBuilder( @Service() export class ExecutionRepository extends Repository { + private deletionBatchSize = 100; + constructor( dataSource: DataSource, private readonly executionDataRepository: ExecutionDataRepository, @@ -409,7 +409,7 @@ export class ExecutionRepository extends Repository { const executionIds = executions.map(({ id }) => id); do { // Delete in batches to avoid "SQLITE_ERROR: Expression tree is too large (maximum depth 1000)" error - const batch = executionIds.splice(0, PRUNING_BATCH_SIZE); + const batch = executionIds.splice(0, this.deletionBatchSize); await this.softDelete(batch); } while (executionIds.length > 0); } @@ -446,12 +446,12 @@ export class ExecutionRepository extends Repository { await this.find({ select: ['id'], where: toPrune, - take: PRUNING_BATCH_SIZE, + take: this.deletionBatchSize, }) ).map(({ id }) => id); await this.softDelete(executionIds); - if (executionIds.length === PRUNING_BATCH_SIZE) { + if (executionIds.length === this.deletionBatchSize) { setTimeout(async () => this.pruneBySoftDeleting(), TIME.SECOND); } } @@ -470,7 +470,7 @@ export class ExecutionRepository extends Repository { where: { deletedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)), }, - take: PRUNING_BATCH_SIZE, + take: this.deletionBatchSize, }) ).map(({ id }) => id); @@ -480,7 +480,7 @@ export class ExecutionRepository extends Repository { // Actually delete these executions await this.delete({ id: In(executionIds) }); - if (executionIds.length === PRUNING_BATCH_SIZE) { + if (executionIds.length === this.deletionBatchSize) { setTimeout(async () => this.hardDelete(), 1000); } } From 1d87e9ef9e06e128a747aaee67b313a3d0d2513c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 12:04:49 +0200 Subject: [PATCH 08/38] Cleanup --- packages/core/src/BinaryDataManager/FileSystem.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/BinaryDataManager/FileSystem.ts b/packages/core/src/BinaryDataManager/FileSystem.ts index 6ae70271211b9..07eb66c815461 100644 --- a/packages/core/src/BinaryDataManager/FileSystem.ts +++ b/packages/core/src/BinaryDataManager/FileSystem.ts @@ -19,7 +19,7 @@ export class BinaryDataFileSystem implements IBinaryDataManager { this.storagePath = config.localStoragePath; } - async init(): Promise { + async init() { await this.assertFolder(this.storagePath); await this.assertFolder(this.getBinaryDataMetaPath()); } @@ -77,7 +77,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager { } async deleteBinaryDataByExecutionIds(executionIds: string[]): Promise { - // TODO: switch over to new folder structure, and delete folders instead const set = new Set(executionIds); const fileNames = await fs.readdir(this.storagePath); const deletedIds = []; From dd16e458fa0a32946a177635d421fda6aa4fb41a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 13:04:06 +0200 Subject: [PATCH 09/38] Remove unused method --- packages/core/src/BinaryDataManager/FileSystem.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/core/src/BinaryDataManager/FileSystem.ts b/packages/core/src/BinaryDataManager/FileSystem.ts index 07eb66c815461..c5501add5a180 100644 --- a/packages/core/src/BinaryDataManager/FileSystem.ts +++ b/packages/core/src/BinaryDataManager/FileSystem.ts @@ -21,7 +21,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager { async init() { await this.assertFolder(this.storagePath); - await this.assertFolder(this.getBinaryDataMetaPath()); } async getFileSize(identifier: string): Promise { @@ -107,10 +106,6 @@ export class BinaryDataFileSystem implements IBinaryDataManager { return [prefix, uuid()].join(''); } - private getBinaryDataMetaPath() { - return path.join(this.storagePath, 'meta'); - } - private async deleteFromLocalStorage(identifier: string) { return fs.rm(this.getBinaryPath(identifier)); } From 619bff6d95f32b256872cffcae119ec33bfefe3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 13:05:45 +0200 Subject: [PATCH 10/38] Cleanup --- packages/core/src/BinaryDataManager/FileSystem.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/BinaryDataManager/FileSystem.ts b/packages/core/src/BinaryDataManager/FileSystem.ts index c5501add5a180..5c4bdd32a9a2c 100644 --- a/packages/core/src/BinaryDataManager/FileSystem.ts +++ b/packages/core/src/BinaryDataManager/FileSystem.ts @@ -68,6 +68,7 @@ export class BinaryDataFileSystem implements IBinaryDataManager { async duplicateBinaryDataByIdentifier(binaryDataId: string, prefix: string): Promise { const newBinaryDataId = this.generateFileName(prefix); + await fs.copyFile( this.resolveStoragePath(binaryDataId), this.resolveStoragePath(newBinaryDataId), From 8cace1144e84aaf83eb044918807481f3eedb8f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 13:21:38 +0200 Subject: [PATCH 11/38] Filter out soft-deleted executions --- .../src/databases/repositories/execution.repository.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index d1c0feef41345..4d76bae08aeb4 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -1,5 +1,5 @@ import { Service } from 'typedi'; -import { DataSource, In, LessThanOrEqual, MoreThanOrEqual, Repository } from 'typeorm'; +import { DataSource, In, IsNull, LessThanOrEqual, MoreThanOrEqual, Repository } from 'typeorm'; import { DateUtils } from 'typeorm/util/DateUtils'; import type { FindManyOptions, @@ -118,6 +118,10 @@ export class ExecutionRepository extends Repository { (queryParams.relations as string[]).push('executionData'); } + if (queryParams.where && !Array.isArray(queryParams.where)) { + queryParams.where.deletedAt = IsNull(); + } + const executions = await this.find(queryParams); if (options?.includeData && options?.unflattenData) { @@ -182,6 +186,7 @@ export class ExecutionRepository extends Repository { where: { id, ...options?.where, + deletedAt: IsNull(), }, }; if (options?.includeData) { @@ -340,7 +345,8 @@ export class ExecutionRepository extends Repository { .limit(limit) // eslint-disable-next-line @typescript-eslint/naming-convention .orderBy({ 'execution.id': 'DESC' }) - .andWhere('execution.workflowId IN (:...accessibleWorkflowIds)', { accessibleWorkflowIds }); + .andWhere('execution.workflowId IN (:...accessibleWorkflowIds)', { accessibleWorkflowIds }) + .andWhere('execution.deletedAt IS NULL'); if (excludedExecutionIds.length > 0) { query.andWhere('execution.id NOT IN (:...excludedExecutionIds)', { excludedExecutionIds }); From c4ffe6d0060fcd20f668715c8dbc354ac6dc3961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 5 Sep 2023 17:12:56 +0200 Subject: [PATCH 12/38] Add tests --- .../repositories/execution.repository.ts | 29 +++++-- .../integration/executions.controller.test.ts | 39 ++++++++++ .../integration/publicApi/executions.test.ts | 2 + .../cli/test/integration/shared/testDb.ts | 4 + packages/cli/test/integration/shared/types.ts | 3 +- .../integration/shared/utils/testServer.ts | 11 ++- .../repositories/execution.repository.test.ts | 78 +++++++++++++++++++ 7 files changed, 157 insertions(+), 9 deletions(-) create mode 100644 packages/cli/test/integration/executions.controller.test.ts create mode 100644 packages/cli/test/unit/repositories/execution.repository.test.ts diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 4d76bae08aeb4..1ea6b707fa9e7 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -83,6 +83,14 @@ export class ExecutionRepository extends Repository { setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); } + setDeletionBatchSize(size: number) { + this.deletionBatchSize = size; + } + + getDeletionBatchSize() { + return this.deletionBatchSize; + } + async findMultipleExecutions( queryParams: FindManyOptions, options?: { @@ -420,19 +428,26 @@ export class ExecutionRepository extends Repository { } while (executionIds.length > 0); } - private async pruneBySoftDeleting() { - Logger.verbose('Pruning (soft-deleting) execution data from database'); - - const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h - const maxCount = config.getEnv('executions.pruneDataMaxCount'); + /** + * Return the timestamp up to which executions should be pruned. + */ + pruningLimit() { + const maxAge = config.getEnv('executions.pruneDataMaxAge'); // hours - // Find ids of all executions that were stopped longer that pruneDataMaxAge ago const date = new Date(); date.setHours(date.getHours() - maxAge); + return date; + } + + async pruneBySoftDeleting() { + Logger.verbose('Pruning (soft-deleting) execution data from database'); + + const maxCount = config.getEnv('executions.pruneDataMaxCount'); + const toPrune: Array> = [ // date reformatting needed - see https://github.com/typeorm/typeorm/issues/2286 - { stoppedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)) }, + { stoppedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(this.pruningLimit())) }, ]; if (maxCount > 0) { diff --git a/packages/cli/test/integration/executions.controller.test.ts b/packages/cli/test/integration/executions.controller.test.ts new file mode 100644 index 0000000000000..fa6a70499cc3f --- /dev/null +++ b/packages/cli/test/integration/executions.controller.test.ts @@ -0,0 +1,39 @@ +import * as testDb from './shared/testDb'; +import { setupTestServer } from './shared/utils'; +import type { User } from '@/databases/entities/User'; + +const testServer = setupTestServer({ endpointGroups: ['executions'] }); + +let owner: User; + +const saveExecution = async ({ belongingTo }: { belongingTo: User }) => { + const workflow = await testDb.createWorkflow({}, belongingTo); + return testDb.createSuccessfulExecution(workflow); +}; + +beforeEach(async () => { + await testDb.truncate(['Execution', 'Workflow', 'SharedWorkflow']); + owner = await testDb.createOwner(); +}); + +describe('POST /executions/delete', () => { + test('should hard-delete an execution', async () => { + await saveExecution({ belongingTo: owner }); + + const response = await testServer.authAgentFor(owner).get('/executions').expect(200); + + expect(response.body.data.count).toBe(1); + + const [execution] = response.body.data.results; + + await testServer + .authAgentFor(owner) + .post('/executions/delete') + .send({ ids: [execution.id] }) + .expect(200); + + const executions = await testDb.getAllExecutions(); + + expect(executions).toHaveLength(0); + }); +}); diff --git a/packages/cli/test/integration/publicApi/executions.test.ts b/packages/cli/test/integration/publicApi/executions.test.ts index 2f5216a8b9198..d42f8ca62ad35 100644 --- a/packages/cli/test/integration/publicApi/executions.test.ts +++ b/packages/cli/test/integration/publicApi/executions.test.ts @@ -168,6 +168,8 @@ describe('DELETE /executions/:id', () => { expect(stoppedAt).not.toBeNull(); expect(workflowId).toBe(execution.workflowId); expect(waitTill).toBeNull(); + + await authOwnerAgent.get(`/executions/${execution.id}`).expect(404); }); }); diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 3186c3f64bbe2..27b91c9c8122b 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -528,6 +528,10 @@ export async function getAllWorkflows() { return Db.collections.Workflow.find(); } +export async function getAllExecutions() { + return Db.collections.Execution.find(); +} + // ---------------------------------- // workflow sharing // ---------------------------------- diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts index 3fd972a6bd1cd..f552c7e1f7bc9 100644 --- a/packages/cli/test/integration/shared/types.ts +++ b/packages/cli/test/integration/shared/types.ts @@ -28,7 +28,8 @@ export type EndpointGroup = | 'tags' | 'externalSecrets' | 'mfa' - | 'metrics'; + | 'metrics' + | 'executions'; export interface SetupProps { applyAuth?: boolean; diff --git a/packages/cli/test/integration/shared/utils/testServer.ts b/packages/cli/test/integration/shared/utils/testServer.ts index b89e6ab7bddd0..b33c3afb04387 100644 --- a/packages/cli/test/integration/shared/utils/testServer.ts +++ b/packages/cli/test/integration/shared/utils/testServer.ts @@ -64,6 +64,7 @@ import { import { JwtService } from '@/services/jwt.service'; import { RoleService } from '@/services/role.service'; import { UserService } from '@/services/user.service'; +import { executionsController } from '@/executions/executions.controller'; /** * Plugin to prefix a path segment into a request URL pathname. @@ -93,7 +94,14 @@ const classifyEndpointGroups = (endpointGroups: EndpointGroup[]) => { const routerEndpoints: EndpointGroup[] = []; const functionEndpoints: EndpointGroup[] = []; - const ROUTER_GROUP = ['credentials', 'workflows', 'publicApi', 'license', 'variables']; + const ROUTER_GROUP = [ + 'credentials', + 'workflows', + 'publicApi', + 'license', + 'variables', + 'executions', + ]; endpointGroups.forEach((group) => (ROUTER_GROUP.includes(group) ? routerEndpoints : functionEndpoints).push(group), @@ -176,6 +184,7 @@ export const setupTestServer = ({ workflows: { controller: workflowsController, path: 'workflows' }, license: { controller: licenseController, path: 'license' }, variables: { controller: variablesController, path: 'variables' }, + executions: { controller: executionsController, path: 'executions' }, }; if (enablePublicAPI) { diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts new file mode 100644 index 0000000000000..d00c0610ee68e --- /dev/null +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -0,0 +1,78 @@ +import { Container } from 'typedi'; +import { DataSource, EntityManager } from 'typeorm'; +import { mock } from 'jest-mock-extended'; +import { mockInstance } from '../../integration/shared/utils/'; +import { ExecutionRepository } from '@/databases/repositories'; +import config from '@/config'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; +import { TIME } from '@/constants'; + +const { objectContaining } = expect; + +describe('ExecutionRepository', () => { + const entityManager = mockInstance(EntityManager); + const dataSource = mockInstance(DataSource, { manager: entityManager }); + dataSource.getMetadata.mockReturnValue(mock()); + Object.assign(entityManager, { connection: dataSource }); + + const executionRepository = Container.get(ExecutionRepository); + + beforeAll(() => { + Container.set(ExecutionRepository, executionRepository); + LoggerProxy.init(getLogger()); + }); + + beforeEach(() => { + config.load(config.default); + + jest.clearAllMocks(); + }); + + describe('pruneBySoftDeleting()', () => { + test('should soft-delete executions based on batch size', async () => { + config.set('executions.pruneDataMaxCount', 0); // disable path + + executionRepository.setDeletionBatchSize(5); + + const find = jest.spyOn(ExecutionRepository.prototype, 'find'); + entityManager.find.mockResolvedValueOnce([]); + + await executionRepository.pruneBySoftDeleting(); + + expect(find.mock.calls[0][0]).toEqual( + objectContaining({ take: executionRepository.getDeletionBatchSize() }), + ); + }); + + test('should limit pruning based on EXECUTIONS_DATA_PRUNE_MAX_COUNT', async () => { + const maxCount = 1; + + config.set('executions.pruneDataMaxCount', maxCount); + + const find = jest.spyOn(ExecutionRepository.prototype, 'find'); + entityManager.find.mockResolvedValue([]); + + await executionRepository.pruneBySoftDeleting(); + + expect(find.mock.calls[0][0]).toEqual(objectContaining({ skip: maxCount })); + }); + }); + + describe('pruningLimit()', () => { + test('should limit pruning based on EXECUTIONS_DATA_MAX_AGE', async () => { + config.set('executions.pruneDataMaxCount', 0); // disable path + + const maxAge = 5; // hours + + config.set('executions.pruneDataMaxAge', maxAge); + + const now = Date.now(); + const limit = executionRepository.pruningLimit(); + + const difference = now - limit.valueOf(); + + expect(difference / TIME.HOUR).toBe(maxAge); + }); + }); +}); From 738eb1b6eb940fa2ad83d65d5608bb9de88b8cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 6 Sep 2023 11:53:05 +0200 Subject: [PATCH 13/38] Improve test --- .../repositories/execution.repository.ts | 29 +++++-------------- .../repositories/execution.repository.test.ts | 24 +++++++++------ 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 1ea6b707fa9e7..75773a2e97fd6 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -68,7 +68,7 @@ function parseFiltersToQueryBuilder( @Service() export class ExecutionRepository extends Repository { - private deletionBatchSize = 100; + deletionBatchSize = 100; constructor( dataSource: DataSource, @@ -83,14 +83,6 @@ export class ExecutionRepository extends Repository { setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); } - setDeletionBatchSize(size: number) { - this.deletionBatchSize = size; - } - - getDeletionBatchSize() { - return this.deletionBatchSize; - } - async findMultipleExecutions( queryParams: FindManyOptions, options?: { @@ -428,26 +420,19 @@ export class ExecutionRepository extends Repository { } while (executionIds.length > 0); } - /** - * Return the timestamp up to which executions should be pruned. - */ - pruningLimit() { - const maxAge = config.getEnv('executions.pruneDataMaxAge'); // hours - - const date = new Date(); - date.setHours(date.getHours() - maxAge); - - return date; - } - async pruneBySoftDeleting() { Logger.verbose('Pruning (soft-deleting) execution data from database'); + const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h const maxCount = config.getEnv('executions.pruneDataMaxCount'); + // Find ids of all executions that were stopped longer that pruneDataMaxAge ago + const date = new Date(); + date.setHours(date.getHours() - maxAge); + const toPrune: Array> = [ // date reformatting needed - see https://github.com/typeorm/typeorm/issues/2286 - { stoppedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(this.pruningLimit())) }, + { stoppedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)) }, ]; if (maxCount > 0) { diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index d00c0610ee68e..a1307de31e80b 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -7,6 +7,9 @@ import config from '@/config'; import { LoggerProxy } from 'n8n-workflow'; import { getLogger } from '@/Logger'; import { TIME } from '@/constants'; +import { DateUtils } from 'typeorm/util/DateUtils'; + +jest.mock('typeorm/util/DateUtils'); const { objectContaining } = expect; @@ -33,7 +36,7 @@ describe('ExecutionRepository', () => { test('should soft-delete executions based on batch size', async () => { config.set('executions.pruneDataMaxCount', 0); // disable path - executionRepository.setDeletionBatchSize(5); + executionRepository.deletionBatchSize = 5; const find = jest.spyOn(ExecutionRepository.prototype, 'find'); entityManager.find.mockResolvedValueOnce([]); @@ -41,7 +44,7 @@ describe('ExecutionRepository', () => { await executionRepository.pruneBySoftDeleting(); expect(find.mock.calls[0][0]).toEqual( - objectContaining({ take: executionRepository.getDeletionBatchSize() }), + objectContaining({ take: executionRepository.deletionBatchSize }), ); }); @@ -57,22 +60,25 @@ describe('ExecutionRepository', () => { expect(find.mock.calls[0][0]).toEqual(objectContaining({ skip: maxCount })); }); - }); - describe('pruningLimit()', () => { test('should limit pruning based on EXECUTIONS_DATA_MAX_AGE', async () => { + const maxAge = 5; // hours + config.set('executions.pruneDataMaxCount', 0); // disable path + config.set('executions.pruneDataMaxAge', 5); - const maxAge = 5; // hours + entityManager.find.mockResolvedValue([]); - config.set('executions.pruneDataMaxAge', maxAge); + const dateFormat = jest.spyOn(DateUtils, 'mixedDateToUtcDatetimeString'); const now = Date.now(); - const limit = executionRepository.pruningLimit(); - const difference = now - limit.valueOf(); + await executionRepository.pruneBySoftDeleting(); + + const argDate = dateFormat.mock.calls[0][0]; + const difference = now - argDate.valueOf(); - expect(difference / TIME.HOUR).toBe(maxAge); + expect(Math.round(difference / TIME.HOUR)).toBe(maxAge); }); }); }); From 259fe75b8608754e3c41337dca022dbddbb1dd50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 13 Sep 2023 15:20:02 +0200 Subject: [PATCH 14/38] Soft-delete in single pass --- .../repositories/execution.repository.ts | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 75773a2e97fd6..0f54e54c8bf34 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -1,5 +1,13 @@ import { Service } from 'typedi'; -import { DataSource, In, IsNull, LessThanOrEqual, MoreThanOrEqual, Repository } from 'typeorm'; +import { + Brackets, + DataSource, + In, + IsNull, + LessThanOrEqual, + MoreThanOrEqual, + Repository, +} from 'typeorm'; import { DateUtils } from 'typeorm/util/DateUtils'; import type { FindManyOptions, @@ -77,7 +85,7 @@ export class ExecutionRepository extends Repository { super(ExecutionEntity, dataSource.manager); if (config.getEnv('executions.pruneData')) { - setInterval(async () => this.pruneBySoftDeleting(), TIME.HOUR); + setInterval(async () => this.pruneBySoftDeleting(), 10 * TIME.SECOND); } setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); @@ -430,7 +438,7 @@ export class ExecutionRepository extends Repository { const date = new Date(); date.setHours(date.getHours() - maxAge); - const toPrune: Array> = [ + const toPrune: Array> = [ // date reformatting needed - see https://github.com/typeorm/typeorm/issues/2286 { stoppedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)) }, ]; @@ -448,18 +456,19 @@ export class ExecutionRepository extends Repository { } } - const executionIds = ( - await this.find({ - select: ['id'], - where: toPrune, - take: this.deletionBatchSize, - }) - ).map(({ id }) => id); - await this.softDelete(executionIds); - - if (executionIds.length === this.deletionBatchSize) { - setTimeout(async () => this.pruneBySoftDeleting(), TIME.SECOND); - } + const [timeBasedWhere, countBasedWhere] = toPrune; + + await this.createQueryBuilder() + .update(ExecutionEntity) + .set({ deletedAt: new Date() }) + .where( + new Brackets((qb) => + countBasedWhere + ? qb.where(timeBasedWhere).orWhere(countBasedWhere) + : qb.where(timeBasedWhere), + ), + ) + .execute(); } /** From f3f5b271b8d655f611db2352186d8fa20858a341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 13 Sep 2023 15:23:58 +0200 Subject: [PATCH 15/38] Restore value --- packages/cli/src/databases/repositories/execution.repository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 0f54e54c8bf34..1670dab32360d 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -85,7 +85,7 @@ export class ExecutionRepository extends Repository { super(ExecutionEntity, dataSource.manager); if (config.getEnv('executions.pruneData')) { - setInterval(async () => this.pruneBySoftDeleting(), 10 * TIME.SECOND); + setInterval(async () => this.pruneBySoftDeleting(), 10 * TIME.HOUR); } setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); From 65d17cba684ac36d4a1882424b2f6615fe4923af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 13 Sep 2023 15:24:44 +0200 Subject: [PATCH 16/38] Restore from debug value --- packages/cli/src/databases/repositories/execution.repository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 1670dab32360d..6cd7561026b3a 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -85,7 +85,7 @@ export class ExecutionRepository extends Repository { super(ExecutionEntity, dataSource.manager); if (config.getEnv('executions.pruneData')) { - setInterval(async () => this.pruneBySoftDeleting(), 10 * TIME.HOUR); + setInterval(async () => this.pruneBySoftDeleting(), 1 * TIME.HOUR); } setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); From 96ae7b464d6f50ecf0255dca2e982a49685b6117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 13 Sep 2023 15:30:21 +0200 Subject: [PATCH 17/38] Add clarifying comments --- .../cli/test/unit/repositories/execution.repository.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index a1307de31e80b..d6a77e678e898 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -34,7 +34,7 @@ describe('ExecutionRepository', () => { describe('pruneBySoftDeleting()', () => { test('should soft-delete executions based on batch size', async () => { - config.set('executions.pruneDataMaxCount', 0); // disable path + config.set('executions.pruneDataMaxCount', 0); // disable prune-by-count path executionRepository.deletionBatchSize = 5; @@ -64,7 +64,7 @@ describe('ExecutionRepository', () => { test('should limit pruning based on EXECUTIONS_DATA_MAX_AGE', async () => { const maxAge = 5; // hours - config.set('executions.pruneDataMaxCount', 0); // disable path + config.set('executions.pruneDataMaxCount', 0); // disable prune-by-count path config.set('executions.pruneDataMaxAge', 5); entityManager.find.mockResolvedValue([]); From 21040b5ac86cd49e0bf14e0c68b1e444a7f64749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 13 Sep 2023 16:40:44 +0200 Subject: [PATCH 18/38] Update tests --- .../repositories/execution.repository.test.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index d6a77e678e898..ac15979504a6f 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -13,6 +13,14 @@ jest.mock('typeorm/util/DateUtils'); const { objectContaining } = expect; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const qb: any = { + update: jest.fn().mockReturnThis(), + set: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + execute: jest.fn().mockReturnThis(), +}; + describe('ExecutionRepository', () => { const entityManager = mockInstance(EntityManager); const dataSource = mockInstance(DataSource, { manager: entityManager }); @@ -33,21 +41,6 @@ describe('ExecutionRepository', () => { }); describe('pruneBySoftDeleting()', () => { - test('should soft-delete executions based on batch size', async () => { - config.set('executions.pruneDataMaxCount', 0); // disable prune-by-count path - - executionRepository.deletionBatchSize = 5; - - const find = jest.spyOn(ExecutionRepository.prototype, 'find'); - entityManager.find.mockResolvedValueOnce([]); - - await executionRepository.pruneBySoftDeleting(); - - expect(find.mock.calls[0][0]).toEqual( - objectContaining({ take: executionRepository.deletionBatchSize }), - ); - }); - test('should limit pruning based on EXECUTIONS_DATA_PRUNE_MAX_COUNT', async () => { const maxCount = 1; @@ -56,6 +49,8 @@ describe('ExecutionRepository', () => { const find = jest.spyOn(ExecutionRepository.prototype, 'find'); entityManager.find.mockResolvedValue([]); + jest.spyOn(ExecutionRepository.prototype, 'createQueryBuilder').mockReturnValueOnce(qb); + await executionRepository.pruneBySoftDeleting(); expect(find.mock.calls[0][0]).toEqual(objectContaining({ skip: maxCount })); @@ -69,6 +64,8 @@ describe('ExecutionRepository', () => { entityManager.find.mockResolvedValue([]); + jest.spyOn(ExecutionRepository.prototype, 'createQueryBuilder').mockReturnValueOnce(qb); + const dateFormat = jest.spyOn(DateUtils, 'mixedDateToUtcDatetimeString'); const now = Date.now(); From 9a25301ee96535d1eed8b5223bc5f0af56e7e3dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 13 Sep 2023 20:12:03 +0200 Subject: [PATCH 19/38] Add clarifying comment --- packages/cli/src/constants.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index ec66cbdf32062..c94af1f5c64b1 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -95,6 +95,9 @@ export const CREDENTIAL_BLANKING_VALUE = '__n8n_BLANK_VALUE_e5362baf-c777-4d57-a export const UM_FIX_INSTRUCTION = 'Please fix the database by running ./packages/cli/bin/n8n user-management:reset'; +/** + * Units of time in milliseconds + */ export const TIME = { SECOND: 1000, MINUTE: 60 * 1000, From 0e075c257d26f5f34fb722fdd2f5120a96f09023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 14 Sep 2023 10:51:50 +0200 Subject: [PATCH 20/38] Speed up pruning if high volume --- .../repositories/execution.repository.ts | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 6cd7561026b3a..6998d9292cff7 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -78,6 +78,8 @@ function parseFiltersToQueryBuilder( export class ExecutionRepository extends Repository { deletionBatchSize = 100; + hardDeletionInterval: NodeJS.Timer | null = null; + constructor( dataSource: DataSource, private readonly executionDataRepository: ExecutionDataRepository, @@ -88,7 +90,15 @@ export class ExecutionRepository extends Repository { setInterval(async () => this.pruneBySoftDeleting(), 1 * TIME.HOUR); } - setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); + this.setHardDeletionInterval(); + } + + setHardDeletionInterval() { + this.hardDeletionInterval = setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); + } + + clearHardDeletionInterval() { + if (this.hardDeletionInterval) clearInterval(this.hardDeletionInterval); } async findMultipleExecutions( @@ -495,8 +505,22 @@ export class ExecutionRepository extends Repository { // Actually delete these executions await this.delete({ id: In(executionIds) }); + /** + * If the volume of executions to prune is as high as the batch size, there is a risk + * that the pruning process is unable to catch up to the creation of new executions, + * with high concurrency possibly leading to errors from duplicate deletions. + * + * Therefore, in this high-volume case we speed up the hard deletion cycle, until + * the number of executions to prune is low enough to fit in a single batch. + */ if (executionIds.length === this.deletionBatchSize) { - setTimeout(async () => this.hardDelete(), 1000); + this.clearHardDeletionInterval(); + + setTimeout(async () => this.hardDelete(), 1 * TIME.SECOND); + } else { + if (this.hardDeletionInterval) return; + + this.setHardDeletionInterval(); } } } From 2071c7fb677b8e4818356d55d095e056704f8573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 14 Sep 2023 11:08:04 +0200 Subject: [PATCH 21/38] Remove call from hook --- packages/cli/src/WorkflowExecuteAdditionalData.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 193acce3a53aa..6ed7d4bb0d38f 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -599,11 +599,6 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { workflowId: this.workflowData.id, }); try { - // Prune old execution data - if (config.getEnv('executions.pruneData')) { - await pruneExecutionData.call(this); - } - if (isWorkflowIdValid(this.workflowData.id) && newStaticData) { // Workflow is saved so update in database try { From 67e163fb240f505de42248bcb8924fa69fd9d09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 15 Sep 2023 10:46:33 +0200 Subject: [PATCH 22/38] Make execution ID non-nullable --- packages/cli/src/Interfaces.ts | 2 +- .../PublicApi/v1/handlers/executions/executions.handler.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index d7718efa90b81..40d6a354c3dda 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -171,7 +171,7 @@ export type ICredentialsDecryptedResponse = ICredentialsDecryptedDb; export type SaveExecutionDataType = 'all' | 'none'; export interface IExecutionBase { - id?: string; + id: string; mode: WorkflowExecuteMode; startedAt: Date; stoppedAt?: Date; // empty value means execution is still running diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts index 90ed886bfa62a..a6b42c96ee38d 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts @@ -27,7 +27,7 @@ export = { // look for the execution on the workflow the user owns const execution = await getExecutionInWorkflows(id, sharedWorkflowsIds, false); - if (!execution?.id) { + if (!execution) { return res.status(404).json({ message: 'Not Found' }); } @@ -103,7 +103,7 @@ export = { const executions = await getExecutions(filters); - const newLastId = !executions.length ? '0' : (executions.slice(-1)[0].id as string); + const newLastId = !executions.length ? '0' : executions.slice(-1)[0].id; filters.lastId = newLastId; From 12636dd7406ed1ecc11202e97d10828153708c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 15 Sep 2023 10:52:16 +0200 Subject: [PATCH 23/38] Readability improvements --- .../repositories/execution.repository.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 6998d9292cff7..a892e370cb096 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -86,19 +86,25 @@ export class ExecutionRepository extends Repository { ) { super(ExecutionEntity, dataSource.manager); - if (config.getEnv('executions.pruneData')) { - setInterval(async () => this.pruneBySoftDeleting(), 1 * TIME.HOUR); - } + if (config.getEnv('executions.pruneData')) this.setPruningInterval(); this.setHardDeletionInterval(); } + setPruningInterval() { + setInterval(async () => this.pruneBySoftDeleting(), 1 * TIME.HOUR); + } + setHardDeletionInterval() { + if (this.hardDeletionInterval) return; + this.hardDeletionInterval = setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); } clearHardDeletionInterval() { - if (this.hardDeletionInterval) clearInterval(this.hardDeletionInterval); + if (!this.hardDeletionInterval) return; + + clearInterval(this.hardDeletionInterval); } async findMultipleExecutions( @@ -518,8 +524,6 @@ export class ExecutionRepository extends Repository { setTimeout(async () => this.hardDelete(), 1 * TIME.SECOND); } else { - if (this.hardDeletionInterval) return; - this.setHardDeletionInterval(); } } From e5c8c7296ffce6df822cdb665e3a13475b0ceedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 15 Sep 2023 11:01:28 +0200 Subject: [PATCH 24/38] Adjust types, followup to 67e163f --- packages/cli/src/ActiveExecutions.ts | 3 ++- packages/cli/src/GenericHelpers.ts | 4 ++-- packages/cli/src/Interfaces.ts | 5 +++++ packages/cli/src/WorkflowExecuteAdditionalData.ts | 3 ++- packages/cli/src/WorkflowHelpers.ts | 8 ++++++-- .../src/databases/repositories/execution.repository.ts | 3 ++- .../executionLifecycleHooks/shared/sharedHookFunctions.ts | 6 +++--- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/ActiveExecutions.ts b/packages/cli/src/ActiveExecutions.ts index 26feeee589b28..f794335204773 100644 --- a/packages/cli/src/ActiveExecutions.ts +++ b/packages/cli/src/ActiveExecutions.ts @@ -12,6 +12,7 @@ import { createDeferredPromise, LoggerProxy } from 'n8n-workflow'; import type { ChildProcess } from 'child_process'; import type PCancelable from 'p-cancelable'; import type { + ExecutionPayload, IExecutingWorkflowData, IExecutionDb, IExecutionsCurrentSummary, @@ -38,7 +39,7 @@ export class ActiveExecutions { if (executionId === undefined) { // Is a new execution so save in DB - const fullExecutionData: IExecutionDb = { + const fullExecutionData: ExecutionPayload = { data: executionData.executionData!, mode: executionData.executionMode, finished: false, diff --git a/packages/cli/src/GenericHelpers.ts b/packages/cli/src/GenericHelpers.ts index 0034ebea90074..ef9e82a9fdbd6 100644 --- a/packages/cli/src/GenericHelpers.ts +++ b/packages/cli/src/GenericHelpers.ts @@ -11,7 +11,7 @@ import { Container } from 'typedi'; import { Like } from 'typeorm'; import config from '@/config'; import * as Db from '@/Db'; -import type { ICredentialsDb, IExecutionDb, IWorkflowDb } from '@/Interfaces'; +import type { ExecutionPayload, ICredentialsDb, IWorkflowDb } from '@/Interfaces'; import * as ResponseHelper from '@/ResponseHelper'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; @@ -178,7 +178,7 @@ export async function createErrorExecution( }, }; - const fullExecutionData: IExecutionDb = { + const fullExecutionData: ExecutionPayload = { data: executionData, mode, finished: false, diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 40d6a354c3dda..3f373e9d9f750 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -189,6 +189,11 @@ export interface IExecutionDb extends IExecutionBase { workflowData?: IWorkflowBase; } +/** + * Payload for creating or updating an execution. + */ +export type ExecutionPayload = Omit; + export interface IExecutionPushResponse { executionId?: string; waitingForWebhook?: boolean; diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 6ed7d4bb0d38f..9c2d309733dfd 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -47,6 +47,7 @@ import type { IWorkflowExecuteProcess, IWorkflowExecutionDataProcess, IWorkflowErrorData, + ExecutionPayload, } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; import { Push } from '@/push'; @@ -885,7 +886,7 @@ async function executeWorkflow( // Therefore, database might not contain finished errors. // Force an update to db as there should be no harm doing this - const fullExecutionData: IExecutionDb = { + const fullExecutionData: ExecutionPayload = { data: fullRunData.data, mode: fullRunData.mode, finished: fullRunData.finished ? fullRunData.finished : false, diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index e1a6e7057f09c..093f713e4ef3b 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -23,7 +23,11 @@ import { } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; import * as Db from '@/Db'; -import type { IExecutionDb, IWorkflowErrorData, IWorkflowExecutionDataProcess } from '@/Interfaces'; +import type { + ExecutionPayload, + IWorkflowErrorData, + IWorkflowExecutionDataProcess, +} from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; // eslint-disable-next-line import/no-cycle import { WorkflowRunner } from '@/WorkflowRunner'; @@ -186,7 +190,7 @@ export async function executeErrorWorkflow( initialNode, ); - const fullExecutionData: IExecutionDb = { + const fullExecutionData: ExecutionPayload = { data: fakeExecution.data, mode: fakeExecution.mode, finished: false, diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index a892e370cb096..6706ceaefba0d 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -20,6 +20,7 @@ import { LoggerProxy as Logger } from 'n8n-workflow'; import type { IExecutionsSummary, IRunExecutionData } from 'n8n-workflow'; import { BinaryDataManager } from 'n8n-core'; import type { + ExecutionPayload, IExecutionBase, IExecutionDb, IExecutionFlattedDb, @@ -242,7 +243,7 @@ export class ExecutionRepository extends Repository { return rest; } - async createNewExecution(execution: IExecutionDb) { + async createNewExecution(execution: ExecutionPayload) { const { data, workflowData, ...rest } = execution; const newExecution = await this.save(rest); diff --git a/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts b/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts index cf68a1930bed2..113bb2980a9af 100644 --- a/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts +++ b/packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts @@ -1,5 +1,5 @@ import type { ExecutionStatus, IRun, IWorkflowBase } from 'n8n-workflow'; -import type { IExecutionDb } from '@/Interfaces'; +import type { ExecutionPayload, IExecutionDb } from '@/Interfaces'; import pick from 'lodash/pick'; import { isWorkflowIdValid } from '@/utils'; import { LoggerProxy } from 'n8n-workflow'; @@ -24,7 +24,7 @@ export function prepareExecutionDataForDbUpdate(parameters: { workflowData: IWorkflowBase; workflowStatusFinal: ExecutionStatus; retryOf?: string; -}): IExecutionDb { +}) { const { runData, workflowData, workflowStatusFinal, retryOf } = parameters; // Although it is treated as IWorkflowBase here, it's being instantiated elsewhere with properties that may be sensitive // As a result, we should create an IWorkflowBase object with only the data we want to save in it. @@ -41,7 +41,7 @@ export function prepareExecutionDataForDbUpdate(parameters: { 'pinData', ]); - const fullExecutionData: IExecutionDb = { + const fullExecutionData: ExecutionPayload = { data: runData.data, mode: runData.mode, finished: runData.finished ? runData.finished : false, From b7062e59120307f1ed051201715e80f7f754a182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 15 Sep 2023 11:25:58 +0200 Subject: [PATCH 25/38] Fix lint --- packages/cli/src/WorkflowExecuteAdditionalData.ts | 1 - packages/cli/src/databases/repositories/execution.repository.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 9c2d309733dfd..08bfd71031c8e 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -42,7 +42,6 @@ import { ActiveExecutions } from '@/ActiveExecutions'; import { CredentialsHelper } from '@/CredentialsHelper'; import { ExternalHooks } from '@/ExternalHooks'; import type { - IExecutionDb, IPushDataExecutionFinished, IWorkflowExecuteProcess, IWorkflowExecutionDataProcess, diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 6706ceaefba0d..e1d4c3840e2bb 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -22,7 +22,6 @@ import { BinaryDataManager } from 'n8n-core'; import type { ExecutionPayload, IExecutionBase, - IExecutionDb, IExecutionFlattedDb, IExecutionResponse, } from '@/Interfaces'; From abe6d9df1d298dd9d602fb6198b824e87193beaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 18 Sep 2023 15:11:57 +0200 Subject: [PATCH 26/38] Clear timers on shutdown --- packages/cli/src/commands/start.ts | 3 +++ .../databases/repositories/execution.repository.ts | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index b0a9b0b73988b..0cebdb633ec8e 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -29,6 +29,7 @@ import { eventBus } from '@/eventbus'; import { BaseCommand } from './BaseCommand'; import { InternalHooks } from '@/InternalHooks'; import { License } from '@/License'; +import { ExecutionRepository } from '@/databases/repositories/execution.repository'; // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-var-requires const open = require('open'); @@ -103,6 +104,8 @@ export class Start extends BaseCommand { // Note: While this saves a new license cert to DB, the previous entitlements are still kept in memory so that the shutdown process can complete await Container.get(License).shutdown(); + Container.get(ExecutionRepository).clearTimers(); + await Container.get(InternalHooks).onN8nStop(); const skipWebhookDeregistration = config.getEnv( diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index e1d4c3840e2bb..7b2e436416902 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -78,7 +78,9 @@ function parseFiltersToQueryBuilder( export class ExecutionRepository extends Repository { deletionBatchSize = 100; - hardDeletionInterval: NodeJS.Timer | null = null; + private pruningInterval: NodeJS.Timer | null = null; + + private hardDeletionInterval: NodeJS.Timer | null = null; constructor( dataSource: DataSource, @@ -91,8 +93,13 @@ export class ExecutionRepository extends Repository { this.setHardDeletionInterval(); } + clearTimers() { + if (this.hardDeletionInterval) clearInterval(this.hardDeletionInterval); + if (this.pruningInterval) clearInterval(this.pruningInterval); + } + setPruningInterval() { - setInterval(async () => this.pruneBySoftDeleting(), 1 * TIME.HOUR); + this.pruningInterval = setInterval(async () => this.pruneBySoftDeleting(), 1 * TIME.HOUR); } setHardDeletionInterval() { From 2682b3d596dcbfa3aca8fd697f1f5af589217aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 18 Sep 2023 16:20:03 +0200 Subject: [PATCH 27/38] Set timers only on main instance --- packages/cli/src/commands/BaseCommand.ts | 2 ++ packages/cli/src/config/schema.ts | 6 ++++++ .../cli/src/databases/repositories/execution.repository.ts | 3 +++ 3 files changed, 11 insertions(+) diff --git a/packages/cli/src/commands/BaseCommand.ts b/packages/cli/src/commands/BaseCommand.ts index 9b2658ac7f7ff..6aa1e12176999 100644 --- a/packages/cli/src/commands/BaseCommand.ts +++ b/packages/cli/src/commands/BaseCommand.ts @@ -114,6 +114,8 @@ export abstract class BaseCommand extends Command { } async initLicense(instanceType: N8nInstanceType = 'main'): Promise { + config.set('generic.instanceType', instanceType); + const license = Container.get(License); await license.init(this.instanceId, instanceType); diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 05ed7000ee186..1704be42ba8b2 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -432,6 +432,12 @@ export const schema = { default: 'America/New_York', env: 'GENERIC_TIMEZONE', }, + + instanceType: { + doc: 'Type of n8n instance', + format: ['main', 'webhook', 'worker'] as const, + default: 'main', + }, }, // How n8n can be reached (Editor & REST-API) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 7b2e436416902..11177325c4835 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -87,7 +87,10 @@ export class ExecutionRepository extends Repository { private readonly executionDataRepository: ExecutionDataRepository, ) { super(ExecutionEntity, dataSource.manager); + if (config.get('generic.instanceType') === 'main') this.setTimers(); + } + setTimers() { if (config.getEnv('executions.pruneData')) this.setPruningInterval(); this.setHardDeletionInterval(); From 1c788fe9e7620fd98dcc832a665daa195acf2780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 18 Sep 2023 16:21:57 +0200 Subject: [PATCH 28/38] Also for clearing timers --- .../cli/src/databases/repositories/execution.repository.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 11177325c4835..9b3dd338370e2 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -87,6 +87,7 @@ export class ExecutionRepository extends Repository { private readonly executionDataRepository: ExecutionDataRepository, ) { super(ExecutionEntity, dataSource.manager); + if (config.get('generic.instanceType') === 'main') this.setTimers(); } @@ -97,6 +98,8 @@ export class ExecutionRepository extends Repository { } clearTimers() { + if (config.get('generic.instanceType') !== 'main') return; + if (this.hardDeletionInterval) clearInterval(this.hardDeletionInterval); if (this.pruningInterval) clearInterval(this.pruningInterval); } From 88235544adec7f55718d2dba72244e218d10af09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 10:12:25 +0200 Subject: [PATCH 29/38] Add logging, refactor for readability --- .../repositories/execution.repository.ts | 59 ++++++++++++------- .../repositories/execution.repository.test.ts | 4 +- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 9b3dd338370e2..53d66d5ef1a4a 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -76,11 +76,23 @@ function parseFiltersToQueryBuilder( @Service() export class ExecutionRepository extends Repository { + private logger = Logger; + deletionBatchSize = 100; - private pruningInterval: NodeJS.Timer | null = null; + private intervals: Record = { + softDeletion: undefined, + hardDeletion: undefined, + }; + + private rates: Record = { + softDeletion: 1 * TIME.HOUR, + hardDeletion: 15 * TIME.MINUTE, + }; + + private isMainInstance = config.get('generic.instanceType') === 'main'; - private hardDeletionInterval: NodeJS.Timer | null = null; + private isPruningEnabled = config.getEnv('executions.pruneData'); constructor( dataSource: DataSource, @@ -88,36 +100,35 @@ export class ExecutionRepository extends Repository { ) { super(ExecutionEntity, dataSource.manager); - if (config.get('generic.instanceType') === 'main') this.setTimers(); - } + if (!this.isMainInstance) return; - setTimers() { - if (config.getEnv('executions.pruneData')) this.setPruningInterval(); + if (this.isPruningEnabled) this.setSoftDeletionInterval(); this.setHardDeletionInterval(); } clearTimers() { - if (config.get('generic.instanceType') !== 'main') return; + if (!this.isMainInstance) return; - if (this.hardDeletionInterval) clearInterval(this.hardDeletionInterval); - if (this.pruningInterval) clearInterval(this.pruningInterval); - } + this.logger.info('Clearing soft-deletion and hard-deletion intervals for executions'); - setPruningInterval() { - this.pruningInterval = setInterval(async () => this.pruneBySoftDeleting(), 1 * TIME.HOUR); + clearInterval(this.intervals.softDeletion); + clearInterval(this.intervals.hardDeletion); } - setHardDeletionInterval() { - if (this.hardDeletionInterval) return; + setSoftDeletionInterval() { + this.logger.info('Setting soft-deletion interval (pruning) for executions'); - this.hardDeletionInterval = setInterval(async () => this.hardDelete(), 15 * TIME.MINUTE); + this.intervals.softDeletion = setInterval(async () => this.prune(), this.rates.hardDeletion); } - clearHardDeletionInterval() { - if (!this.hardDeletionInterval) return; + setHardDeletionInterval() { + this.logger.info('Setting hard-deletion interval for executions'); - clearInterval(this.hardDeletionInterval); + this.intervals.hardDeletion = setInterval( + async () => this.hardDelete(), + this.rates.hardDeletion, + ); } async findMultipleExecutions( @@ -457,8 +468,8 @@ export class ExecutionRepository extends Repository { } while (executionIds.length > 0); } - async pruneBySoftDeleting() { - Logger.verbose('Pruning (soft-deleting) execution data from database'); + async prune() { + Logger.verbose('Soft-deleting (pruning) execution data from database'); const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h const maxCount = config.getEnv('executions.pruneDataMaxCount'); @@ -521,6 +532,10 @@ export class ExecutionRepository extends Repository { const binaryDataManager = BinaryDataManager.getInstance(); await binaryDataManager.deleteBinaryDataByExecutionIds(executionIds); + this.logger.info(`Hard-deleting ${executionIds.length} executions from database`, { + executionIds, + }); + // Actually delete these executions await this.delete({ id: In(executionIds) }); @@ -533,10 +548,12 @@ export class ExecutionRepository extends Repository { * the number of executions to prune is low enough to fit in a single batch. */ if (executionIds.length === this.deletionBatchSize) { - this.clearHardDeletionInterval(); + clearInterval(this.intervals.hardDeletion); setTimeout(async () => this.hardDelete(), 1 * TIME.SECOND); } else { + if (this.intervals.hardDeletion) return; + this.setHardDeletionInterval(); } } diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index ac15979504a6f..500e1ae0b6a33 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -51,7 +51,7 @@ describe('ExecutionRepository', () => { jest.spyOn(ExecutionRepository.prototype, 'createQueryBuilder').mockReturnValueOnce(qb); - await executionRepository.pruneBySoftDeleting(); + await executionRepository.prune(); expect(find.mock.calls[0][0]).toEqual(objectContaining({ skip: maxCount })); }); @@ -70,7 +70,7 @@ describe('ExecutionRepository', () => { const now = Date.now(); - await executionRepository.pruneBySoftDeleting(); + await executionRepository.prune(); const argDate = dateFormat.mock.calls[0][0]; const difference = now - argDate.valueOf(); From a5def408265a59a800a772fb17e25b887cfcf2a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 10:18:00 +0200 Subject: [PATCH 30/38] Ensure hard-deletion select includes soft-deleted rows --- .../cli/src/databases/repositories/execution.repository.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 53d66d5ef1a4a..0e1814fbe65d8 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -526,6 +526,12 @@ export class ExecutionRepository extends Repository { deletedAt: LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(date)), }, take: this.deletionBatchSize, + + /** + * @important This ensures soft-deleted executions are included, + * else `@DeleteDateColumn()` at `deletedAt` will exclude them. + */ + withDeleted: true, }) ).map(({ id }) => id); From 77955e4ea0fe74e9ac9edef82b2f07ddd5a6a3e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 10:18:51 +0200 Subject: [PATCH 31/38] Switch `info` to `debug` --- .../src/databases/repositories/execution.repository.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 0e1814fbe65d8..4e2def452fdb5 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -110,20 +110,20 @@ export class ExecutionRepository extends Repository { clearTimers() { if (!this.isMainInstance) return; - this.logger.info('Clearing soft-deletion and hard-deletion intervals for executions'); + this.logger.debug('Clearing soft-deletion and hard-deletion intervals for executions'); clearInterval(this.intervals.softDeletion); clearInterval(this.intervals.hardDeletion); } setSoftDeletionInterval() { - this.logger.info('Setting soft-deletion interval (pruning) for executions'); + this.logger.debug('Setting soft-deletion interval (pruning) for executions'); this.intervals.softDeletion = setInterval(async () => this.prune(), this.rates.hardDeletion); } setHardDeletionInterval() { - this.logger.info('Setting hard-deletion interval for executions'); + this.logger.debug('Setting hard-deletion interval for executions'); this.intervals.hardDeletion = setInterval( async () => this.hardDelete(), @@ -538,7 +538,7 @@ export class ExecutionRepository extends Repository { const binaryDataManager = BinaryDataManager.getInstance(); await binaryDataManager.deleteBinaryDataByExecutionIds(executionIds); - this.logger.info(`Hard-deleting ${executionIds.length} executions from database`, { + this.logger.debug(`Hard-deleting ${executionIds.length} executions from database`, { executionIds, }); From b930e3e12d986a497708774ad3764b44e27ff92f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 10:30:30 +0200 Subject: [PATCH 32/38] Fix tests --- packages/cli/test/integration/executions.controller.test.ts | 6 +++++- packages/cli/test/integration/publicApi/executions.test.ts | 4 ++++ .../cli/test/unit/repositories/execution.repository.test.ts | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/cli/test/integration/executions.controller.test.ts b/packages/cli/test/integration/executions.controller.test.ts index fa6a70499cc3f..bc2f783674a1c 100644 --- a/packages/cli/test/integration/executions.controller.test.ts +++ b/packages/cli/test/integration/executions.controller.test.ts @@ -1,8 +1,12 @@ import * as testDb from './shared/testDb'; import { setupTestServer } from './shared/utils'; import type { User } from '@/databases/entities/User'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; -const testServer = setupTestServer({ endpointGroups: ['executions'] }); +LoggerProxy.init(getLogger()); + +let testServer = setupTestServer({ endpointGroups: ['executions'] }); let owner: User; diff --git a/packages/cli/test/integration/publicApi/executions.test.ts b/packages/cli/test/integration/publicApi/executions.test.ts index d42f8ca62ad35..9b208548a5b05 100644 --- a/packages/cli/test/integration/publicApi/executions.test.ts +++ b/packages/cli/test/integration/publicApi/executions.test.ts @@ -5,6 +5,8 @@ import type { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import { randomApiKey } from '../shared/random'; import * as utils from '../shared/utils/'; import * as testDb from '../shared/testDb'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; let owner: User; let user1: User; @@ -14,6 +16,8 @@ let authUser1Agent: SuperAgentTest; let authUser2Agent: SuperAgentTest; let workflowRunner: ActiveWorkflowRunner; +LoggerProxy.init(getLogger()); + const testServer = utils.setupTestServer({ endpointGroups: ['publicApi'] }); beforeAll(async () => { diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index 500e1ae0b6a33..7d2d0350dbe93 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -11,6 +11,8 @@ import { DateUtils } from 'typeorm/util/DateUtils'; jest.mock('typeorm/util/DateUtils'); +LoggerProxy.init(getLogger()); + const { objectContaining } = expect; // eslint-disable-next-line @typescript-eslint/no-explicit-any From e91f3de30a60e7c463af5c002cb1411e8d6e02f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 10:32:25 +0200 Subject: [PATCH 33/38] Remove redundant checks for `deletedAt` being `NULL` The column `deletedAt` is marked `@DeleteDateColumn()` so all reads from the repository automatically add a `WHERE` clause checking that the column `IS NULL`. --- .../src/databases/repositories/execution.repository.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 4e2def452fdb5..d9a0839f33b08 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -166,10 +166,6 @@ export class ExecutionRepository extends Repository { (queryParams.relations as string[]).push('executionData'); } - if (queryParams.where && !Array.isArray(queryParams.where)) { - queryParams.where.deletedAt = IsNull(); - } - const executions = await this.find(queryParams); if (options?.includeData && options?.unflattenData) { @@ -234,7 +230,6 @@ export class ExecutionRepository extends Repository { where: { id, ...options?.where, - deletedAt: IsNull(), }, }; if (options?.includeData) { @@ -393,9 +388,7 @@ export class ExecutionRepository extends Repository { .limit(limit) // eslint-disable-next-line @typescript-eslint/naming-convention .orderBy({ 'execution.id': 'DESC' }) - .andWhere('execution.workflowId IN (:...accessibleWorkflowIds)', { accessibleWorkflowIds }) - .andWhere('execution.deletedAt IS NULL'); - + .andWhere('execution.workflowId IN (:...accessibleWorkflowIds)', { accessibleWorkflowIds }); if (excludedExecutionIds.length > 0) { query.andWhere('execution.id NOT IN (:...excludedExecutionIds)', { excludedExecutionIds }); } From 2c3704d0e3aec8fe16ec5775fbe3decd16e48a3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 10:43:41 +0200 Subject: [PATCH 34/38] Fix lint --- .../src/databases/repositories/execution.repository.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index d9a0839f33b08..651e6d7bfc6d7 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -1,13 +1,5 @@ import { Service } from 'typedi'; -import { - Brackets, - DataSource, - In, - IsNull, - LessThanOrEqual, - MoreThanOrEqual, - Repository, -} from 'typeorm'; +import { Brackets, DataSource, In, LessThanOrEqual, MoreThanOrEqual, Repository } from 'typeorm'; import { DateUtils } from 'typeorm/util/DateUtils'; import type { FindManyOptions, From 215f58e7ba1bc3c2312746d1ededf412c1cb6c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 11:23:02 +0200 Subject: [PATCH 35/38] Fix last test --- packages/cli/test/integration/publicApi/workflows.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/test/integration/publicApi/workflows.test.ts b/packages/cli/test/integration/publicApi/workflows.test.ts index 910bfbb3970c8..980986cf96d44 100644 --- a/packages/cli/test/integration/publicApi/workflows.test.ts +++ b/packages/cli/test/integration/publicApi/workflows.test.ts @@ -10,6 +10,8 @@ import * as utils from '../shared/utils/'; import * as testDb from '../shared/testDb'; import type { INode } from 'n8n-workflow'; import { STARTING_NODES } from '@/constants'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; let workflowOwnerRole: Role; let owner: User; @@ -18,6 +20,8 @@ let authOwnerAgent: SuperAgentTest; let authMemberAgent: SuperAgentTest; let workflowRunner: ActiveWorkflowRunner; +LoggerProxy.init(getLogger()); + const testServer = utils.setupTestServer({ endpointGroups: ['publicApi'] }); beforeAll(async () => { From c33d164913ba3c231b6489de55b20b8f725a4e94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 11:54:50 +0200 Subject: [PATCH 36/38] More missing loggers in tests --- .../cli/test/integration/workflows.controller.ee.test.ts | 5 +++++ packages/cli/test/integration/workflows.controller.test.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts index 6c63bbc8ae9ff..4833c36c1e2e6 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -13,6 +13,9 @@ import type { SaveCredentialFunction } from './shared/types'; import { makeWorkflow } from './shared/utils/'; import { randomCredentialPayload } from './shared/random'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + let owner: User; let member: User; let anotherMember: User; @@ -21,6 +24,8 @@ let authMemberAgent: SuperAgentTest; let authAnotherMemberAgent: SuperAgentTest; let saveCredential: SaveCredentialFunction; +LoggerProxy.init(getLogger()); + const sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); const testServer = utils.setupTestServer({ endpointGroups: ['workflows'], diff --git a/packages/cli/test/integration/workflows.controller.test.ts b/packages/cli/test/integration/workflows.controller.test.ts index fb56237fd7a6b..db4298b933503 100644 --- a/packages/cli/test/integration/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows.controller.test.ts @@ -12,9 +12,14 @@ import { RoleService } from '@/services/role.service'; import Container from 'typedi'; import type { ListQuery } from '@/requests'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + let owner: User; let authOwnerAgent: SuperAgentTest; +LoggerProxy.init(getLogger()); + jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(false); const testServer = utils.setupTestServer({ endpointGroups: ['workflows'] }); From 69764f7b9f1d0f913c0cca95b840b227cf65c67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 12:39:59 +0200 Subject: [PATCH 37/38] Add logger to even more test --- packages/cli/test/integration/audit/credentials.risk.test.ts | 5 +++++ packages/cli/test/integration/credentials.controller.test.ts | 5 +++++ packages/cli/test/integration/credentials.ee.test.ts | 5 +++++ packages/cli/test/integration/credentials.test.ts | 4 ++++ packages/cli/test/integration/publicApi/credentials.test.ts | 5 +++++ 5 files changed, 24 insertions(+) diff --git a/packages/cli/test/integration/audit/credentials.risk.test.ts b/packages/cli/test/integration/audit/credentials.risk.test.ts index 10d1d9ecbd042..49a399841870a 100644 --- a/packages/cli/test/integration/audit/credentials.risk.test.ts +++ b/packages/cli/test/integration/audit/credentials.risk.test.ts @@ -7,6 +7,11 @@ import { getRiskSection } from './utils'; import * as testDb from '../shared/testDb'; import { generateNanoId } from '@db/utils/generators'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + beforeAll(async () => { await testDb.init(); }); diff --git a/packages/cli/test/integration/credentials.controller.test.ts b/packages/cli/test/integration/credentials.controller.test.ts index 1b55c71d46c74..cab87149b12f3 100644 --- a/packages/cli/test/integration/credentials.controller.test.ts +++ b/packages/cli/test/integration/credentials.controller.test.ts @@ -12,6 +12,11 @@ const testServer = utils.setupTestServer({ endpointGroups: ['credentials'] }); let owner: User; let member: User; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + beforeEach(async () => { await testDb.truncate(['SharedCredentials', 'Credentials']); diff --git a/packages/cli/test/integration/credentials.ee.test.ts b/packages/cli/test/integration/credentials.ee.test.ts index a1dc4f7be00b5..e9fa4f088f268 100644 --- a/packages/cli/test/integration/credentials.ee.test.ts +++ b/packages/cli/test/integration/credentials.ee.test.ts @@ -14,6 +14,11 @@ import * as testDb from './shared/testDb'; import type { SaveCredentialFunction } from './shared/types'; import * as utils from './shared/utils/'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + const sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); const testServer = utils.setupTestServer({ endpointGroups: ['credentials'] }); diff --git a/packages/cli/test/integration/credentials.test.ts b/packages/cli/test/integration/credentials.test.ts index 526d3bb28c77b..72c0aea730c2e 100644 --- a/packages/cli/test/integration/credentials.test.ts +++ b/packages/cli/test/integration/credentials.test.ts @@ -12,6 +12,10 @@ import { randomCredentialPayload, randomName, randomString } from './shared/rand import * as testDb from './shared/testDb'; import type { SaveCredentialFunction } from './shared/types'; import * as utils from './shared/utils/'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); // mock that credentialsSharing is not enabled jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(false); diff --git a/packages/cli/test/integration/publicApi/credentials.test.ts b/packages/cli/test/integration/publicApi/credentials.test.ts index bc5c414142a17..fc576a9f7eed7 100644 --- a/packages/cli/test/integration/publicApi/credentials.test.ts +++ b/packages/cli/test/integration/publicApi/credentials.test.ts @@ -10,6 +10,11 @@ import * as utils from '../shared/utils/'; import type { CredentialPayload, SaveCredentialFunction } from '../shared/types'; import * as testDb from '../shared/testDb'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + let globalMemberRole: Role; let credentialOwnerRole: Role; let owner: User; From 7c3e58db2f8cb80983d885fc6096207089287941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 19 Sep 2023 13:28:53 +0200 Subject: [PATCH 38/38] Refactor logging for tests --- packages/cli/test/integration/audit/database.risk.test.ts | 5 +++++ packages/cli/test/integration/audit/filesystem.risk.test.ts | 5 +++++ packages/cli/test/integration/audit/instance.risk.test.ts | 5 +++++ packages/cli/test/integration/audit/nodes.risk.test.ts | 5 +++++ packages/cli/test/integration/commands/import.cmd.test.ts | 5 +++++ .../cli/test/integration/credentials.controller.test.ts | 5 ----- packages/cli/test/integration/credentials.ee.test.ts | 5 ----- packages/cli/test/integration/credentials.test.ts | 4 ---- packages/cli/test/integration/executions.controller.test.ts | 4 ---- packages/cli/test/integration/ldap/ldap.api.test.ts | 5 +++++ packages/cli/test/integration/publicApi/credentials.test.ts | 5 ----- packages/cli/test/integration/publicApi/workflows.test.ts | 4 ---- packages/cli/test/integration/shared/utils/testServer.ts | 6 +++--- .../cli/test/integration/workflows.controller.ee.test.ts | 5 ----- packages/cli/test/integration/workflows.controller.test.ts | 5 ----- packages/cli/test/unit/PermissionChecker.test.ts | 5 +++++ 16 files changed, 38 insertions(+), 40 deletions(-) diff --git a/packages/cli/test/integration/audit/database.risk.test.ts b/packages/cli/test/integration/audit/database.risk.test.ts index a4068f5d77704..b2e485663feb4 100644 --- a/packages/cli/test/integration/audit/database.risk.test.ts +++ b/packages/cli/test/integration/audit/database.risk.test.ts @@ -10,6 +10,11 @@ import { getRiskSection, saveManualTriggerWorkflow } from './utils'; import * as testDb from '../shared/testDb'; import { generateNanoId } from '@db/utils/generators'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + beforeAll(async () => { await testDb.init(); }); diff --git a/packages/cli/test/integration/audit/filesystem.risk.test.ts b/packages/cli/test/integration/audit/filesystem.risk.test.ts index d8f3e711e7b55..33d0ef8fa5782 100644 --- a/packages/cli/test/integration/audit/filesystem.risk.test.ts +++ b/packages/cli/test/integration/audit/filesystem.risk.test.ts @@ -5,6 +5,11 @@ import { FILESYSTEM_INTERACTION_NODE_TYPES, FILESYSTEM_REPORT } from '@/audit/co import { getRiskSection, saveManualTriggerWorkflow } from './utils'; import * as testDb from '../shared/testDb'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + beforeAll(async () => { await testDb.init(); }); diff --git a/packages/cli/test/integration/audit/instance.risk.test.ts b/packages/cli/test/integration/audit/instance.risk.test.ts index aa186c4d7cfc3..349f5de38eb46 100644 --- a/packages/cli/test/integration/audit/instance.risk.test.ts +++ b/packages/cli/test/integration/audit/instance.risk.test.ts @@ -14,6 +14,11 @@ import { toReportTitle } from '@/audit/utils'; import config from '@/config'; import { generateNanoId } from '@db/utils/generators'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + beforeAll(async () => { await testDb.init(); diff --git a/packages/cli/test/integration/audit/nodes.risk.test.ts b/packages/cli/test/integration/audit/nodes.risk.test.ts index fb966d23697fb..dbe414a951930 100644 --- a/packages/cli/test/integration/audit/nodes.risk.test.ts +++ b/packages/cli/test/integration/audit/nodes.risk.test.ts @@ -11,6 +11,11 @@ import { NodeTypes } from '@/NodeTypes'; import { CommunityPackageService } from '@/services/communityPackage.service'; import Container from 'typedi'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + const nodesAndCredentials = mockInstance(LoadNodesAndCredentials); nodesAndCredentials.getCustomDirectories.mockReturnValue([]); mockInstance(NodeTypes); diff --git a/packages/cli/test/integration/commands/import.cmd.test.ts b/packages/cli/test/integration/commands/import.cmd.test.ts index 750ed86b440f7..a191267f622be 100644 --- a/packages/cli/test/integration/commands/import.cmd.test.ts +++ b/packages/cli/test/integration/commands/import.cmd.test.ts @@ -4,6 +4,11 @@ import { InternalHooks } from '@/InternalHooks'; import { ImportWorkflowsCommand } from '@/commands/import/workflow'; import * as Config from '@oclif/config'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + beforeAll(async () => { mockInstance(InternalHooks); await testDb.init(); diff --git a/packages/cli/test/integration/credentials.controller.test.ts b/packages/cli/test/integration/credentials.controller.test.ts index cab87149b12f3..1b55c71d46c74 100644 --- a/packages/cli/test/integration/credentials.controller.test.ts +++ b/packages/cli/test/integration/credentials.controller.test.ts @@ -12,11 +12,6 @@ const testServer = utils.setupTestServer({ endpointGroups: ['credentials'] }); let owner: User; let member: User; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; - -LoggerProxy.init(getLogger()); - beforeEach(async () => { await testDb.truncate(['SharedCredentials', 'Credentials']); diff --git a/packages/cli/test/integration/credentials.ee.test.ts b/packages/cli/test/integration/credentials.ee.test.ts index e9fa4f088f268..a1dc4f7be00b5 100644 --- a/packages/cli/test/integration/credentials.ee.test.ts +++ b/packages/cli/test/integration/credentials.ee.test.ts @@ -14,11 +14,6 @@ import * as testDb from './shared/testDb'; import type { SaveCredentialFunction } from './shared/types'; import * as utils from './shared/utils/'; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; - -LoggerProxy.init(getLogger()); - const sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); const testServer = utils.setupTestServer({ endpointGroups: ['credentials'] }); diff --git a/packages/cli/test/integration/credentials.test.ts b/packages/cli/test/integration/credentials.test.ts index 72c0aea730c2e..526d3bb28c77b 100644 --- a/packages/cli/test/integration/credentials.test.ts +++ b/packages/cli/test/integration/credentials.test.ts @@ -12,10 +12,6 @@ import { randomCredentialPayload, randomName, randomString } from './shared/rand import * as testDb from './shared/testDb'; import type { SaveCredentialFunction } from './shared/types'; import * as utils from './shared/utils/'; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; - -LoggerProxy.init(getLogger()); // mock that credentialsSharing is not enabled jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(false); diff --git a/packages/cli/test/integration/executions.controller.test.ts b/packages/cli/test/integration/executions.controller.test.ts index bc2f783674a1c..2af9ec5b8597d 100644 --- a/packages/cli/test/integration/executions.controller.test.ts +++ b/packages/cli/test/integration/executions.controller.test.ts @@ -1,10 +1,6 @@ import * as testDb from './shared/testDb'; import { setupTestServer } from './shared/utils'; import type { User } from '@/databases/entities/User'; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; - -LoggerProxy.init(getLogger()); let testServer = setupTestServer({ endpointGroups: ['executions'] }); diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index 8b00566d1187d..1a3e23cd8ccf8 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -17,6 +17,9 @@ import { randomEmail, randomName, uniqueId } from './../shared/random'; import * as testDb from './../shared/testDb'; import * as utils from '../shared/utils/'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + jest.mock('@/telemetry'); jest.mock('@/UserManagement/email/NodeMailer'); @@ -24,6 +27,8 @@ let globalMemberRole: Role; let owner: User; let authOwnerAgent: SuperAgentTest; +LoggerProxy.init(getLogger()); + const defaultLdapConfig = { ...LDAP_DEFAULT_CONFIGURATION, loginEnabled: true, diff --git a/packages/cli/test/integration/publicApi/credentials.test.ts b/packages/cli/test/integration/publicApi/credentials.test.ts index fc576a9f7eed7..bc5c414142a17 100644 --- a/packages/cli/test/integration/publicApi/credentials.test.ts +++ b/packages/cli/test/integration/publicApi/credentials.test.ts @@ -10,11 +10,6 @@ import * as utils from '../shared/utils/'; import type { CredentialPayload, SaveCredentialFunction } from '../shared/types'; import * as testDb from '../shared/testDb'; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; - -LoggerProxy.init(getLogger()); - let globalMemberRole: Role; let credentialOwnerRole: Role; let owner: User; diff --git a/packages/cli/test/integration/publicApi/workflows.test.ts b/packages/cli/test/integration/publicApi/workflows.test.ts index 980986cf96d44..910bfbb3970c8 100644 --- a/packages/cli/test/integration/publicApi/workflows.test.ts +++ b/packages/cli/test/integration/publicApi/workflows.test.ts @@ -10,8 +10,6 @@ import * as utils from '../shared/utils/'; import * as testDb from '../shared/testDb'; import type { INode } from 'n8n-workflow'; import { STARTING_NODES } from '@/constants'; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; let workflowOwnerRole: Role; let owner: User; @@ -20,8 +18,6 @@ let authOwnerAgent: SuperAgentTest; let authMemberAgent: SuperAgentTest; let workflowRunner: ActiveWorkflowRunner; -LoggerProxy.init(getLogger()); - const testServer = utils.setupTestServer({ endpointGroups: ['publicApi'] }); beforeAll(async () => { diff --git a/packages/cli/test/integration/shared/utils/testServer.ts b/packages/cli/test/integration/shared/utils/testServer.ts index b33c3afb04387..21e381ff830ee 100644 --- a/packages/cli/test/integration/shared/utils/testServer.ts +++ b/packages/cli/test/integration/shared/utils/testServer.ts @@ -141,6 +141,9 @@ export const setupTestServer = ({ app.use(rawBodyReader); app.use(cookieParser()); + const logger = getLogger(); + LoggerProxy.init(logger); + const testServer: TestServer = { app, httpServer: app.listen(0), @@ -152,9 +155,6 @@ export const setupTestServer = ({ beforeAll(async () => { await testDb.init(); - const logger = getLogger(); - LoggerProxy.init(logger); - // Mock all telemetry. mockInstance(InternalHooks); mockInstance(PostHogClient); diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts index 4833c36c1e2e6..6c63bbc8ae9ff 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -13,9 +13,6 @@ import type { SaveCredentialFunction } from './shared/types'; import { makeWorkflow } from './shared/utils/'; import { randomCredentialPayload } from './shared/random'; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; - let owner: User; let member: User; let anotherMember: User; @@ -24,8 +21,6 @@ let authMemberAgent: SuperAgentTest; let authAnotherMemberAgent: SuperAgentTest; let saveCredential: SaveCredentialFunction; -LoggerProxy.init(getLogger()); - const sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); const testServer = utils.setupTestServer({ endpointGroups: ['workflows'], diff --git a/packages/cli/test/integration/workflows.controller.test.ts b/packages/cli/test/integration/workflows.controller.test.ts index db4298b933503..fb56237fd7a6b 100644 --- a/packages/cli/test/integration/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows.controller.test.ts @@ -12,14 +12,9 @@ import { RoleService } from '@/services/role.service'; import Container from 'typedi'; import type { ListQuery } from '@/requests'; -import { LoggerProxy } from 'n8n-workflow'; -import { getLogger } from '@/Logger'; - let owner: User; let authOwnerAgent: SuperAgentTest; -LoggerProxy.init(getLogger()); - jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(false); const testServer = utils.setupTestServer({ endpointGroups: ['workflows'] }); diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts index ab1443c0c7ee3..139cf0571003b 100644 --- a/packages/cli/test/unit/PermissionChecker.test.ts +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -25,6 +25,11 @@ import type { SaveCredentialFunction } from '../integration/shared/types'; import { mockInstance } from '../integration/shared/utils/'; import { OwnershipService } from '@/services/ownership.service'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; + +LoggerProxy.init(getLogger()); + let mockNodeTypes: INodeTypes; let credentialOwnerRole: Role; let workflowOwnerRole: Role;