From c9d9069c0e9670cb8eb7347ac2b61c9bf5e9acd1 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: Fri, 24 Mar 2023 13:11:48 +0100 Subject: [PATCH] refactor(core): Stronger typing for workflow settings (no-changelog) (#5754) --- packages/cli/src/CredentialsHelper.ts | 3 +- packages/cli/src/Interfaces.ts | 11 ++-- .../src/UserManagement/PermissionChecker.ts | 2 +- .../cli/src/WorkflowExecuteAdditionalData.ts | 50 +++++++------------ packages/cli/src/WorkflowRunner.ts | 32 +++++------- packages/cli/src/WorkflowRunnerProcess.ts | 9 ++-- packages/cli/src/commands/worker.ts | 11 ++-- .../cli/src/workflows/workflows.services.ts | 41 +++++++-------- packages/workflow/src/Expression.ts | 2 +- packages/workflow/src/Interfaces.ts | 15 +++++- packages/workflow/src/WorkflowDataProxy.ts | 2 +- 11 files changed, 77 insertions(+), 101 deletions(-) diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 643b12f8e1..9a5a230268 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -393,8 +393,7 @@ export class CredentialsHelper extends ICredentialsHelper { } if (expressionResolveValues) { - const timezone = - (expressionResolveValues.workflow.settings.timezone as string) || defaultTimezone; + const timezone = expressionResolveValues.workflow.settings.timezone ?? defaultTimezone; try { decryptedData = expressionResolveValues.workflow.expression.getParameterValue( diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 991b055f9f..02f15ea518 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -23,6 +23,7 @@ import type { ExecutionStatus, IExecutionsSummary, FeatureFlags, + WorkflowSettings, } from 'n8n-workflow'; import type { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; @@ -455,16 +456,12 @@ export interface IVersionNotificationSettings { export interface IN8nUISettings { endpointWebhook: string; endpointWebhookTest: string; - saveDataErrorExecution: 'all' | 'none'; - saveDataSuccessExecution: 'all' | 'none'; + saveDataErrorExecution: WorkflowSettings.SaveDataExecution; + saveDataSuccessExecution: WorkflowSettings.SaveDataExecution; saveManualExecutions: boolean; executionTimeout: number; maxExecutionTimeout: number; - workflowCallerPolicyDefaultOption: - | 'any' - | 'none' - | 'workflowsFromAList' - | 'workflowsFromSameOwner'; + workflowCallerPolicyDefaultOption: WorkflowSettings.CallerPolicy; oauthCallbackUrls: { oauth1: string; oauth2: string; diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index 8949156587..fb2e7b19e1 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -116,7 +116,7 @@ export class PermissionChecker { if (parentWorkflowId === undefined) { throw errorToThrow; } - const allowedCallerIds = (subworkflow.settings.callerIds as string | undefined) + const allowedCallerIds = subworkflow.settings.callerIds ?.split(',') .map((id) => id.trim()) .filter((id) => id !== ''); diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 0e2ad0c7ee..85597074b4 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -136,17 +136,11 @@ export function executeErrorWorkflow( // Run the error workflow // To avoid an infinite loop do not run the error workflow again if the error-workflow itself failed and it is its own error-workflow. - if ( - workflowData.settings?.errorWorkflow && - !( - mode === 'error' && - workflowId && - workflowData.settings.errorWorkflow.toString() === workflowId - ) - ) { + const { errorWorkflow } = workflowData.settings ?? {}; + if (errorWorkflow && !(mode === 'error' && workflowId && errorWorkflow === workflowId)) { Logger.verbose('Start external error workflow', { executionId, - errorWorkflowId: workflowData.settings.errorWorkflow.toString(), + errorWorkflowId: errorWorkflow, workflowId, }); // If a specific error workflow is set run only that one @@ -160,11 +154,7 @@ export function executeErrorWorkflow( } getWorkflowOwner(workflowId) .then((user) => { - void WorkflowHelpers.executeErrorWorkflow( - workflowData.settings!.errorWorkflow as string, - workflowErrorData, - user, - ); + void WorkflowHelpers.executeErrorWorkflow(errorWorkflow, workflowErrorData, user); }) .catch((error: Error) => { ErrorReporter.error(error); @@ -172,7 +162,7 @@ export function executeErrorWorkflow( `Could not execute ErrorWorkflow for execution ID ${this.executionId} because of error querying the workflow owner`, { executionId, - errorWorkflowId: workflowData.settings!.errorWorkflow!.toString(), + errorWorkflowId: errorWorkflow, workflowId, error, workflowErrorData, @@ -421,21 +411,21 @@ export function hookFunctionsPreExecute(parentProcessMode?: string): IWorkflowEx ], nodeExecuteAfter: [ async function ( + this: WorkflowHooks, nodeName: string, data: ITaskData, executionData: IRunExecutionData, ): Promise { - if (this.workflowData.settings !== undefined) { - if (this.workflowData.settings.saveExecutionProgress === false) { + const saveExecutionProgress = config.getEnv('executions.saveExecutionProgress'); + const workflowSettings = this.workflowData.settings; + if (workflowSettings !== undefined) { + if (workflowSettings.saveExecutionProgress === false) { return; } - if ( - this.workflowData.settings.saveExecutionProgress !== true && - !config.getEnv('executions.saveExecutionProgress') - ) { + if (workflowSettings.saveExecutionProgress !== true && !saveExecutionProgress) { return; } - } else if (!config.getEnv('executions.saveExecutionProgress')) { + } else if (!saveExecutionProgress) { return; } @@ -563,13 +553,11 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { } } + const workflowSettings = this.workflowData.settings; let saveManualExecutions = config.getEnv('executions.saveDataManualExecutions'); - if ( - this.workflowData.settings !== undefined && - this.workflowData.settings.saveManualExecutions !== undefined - ) { + if (workflowSettings?.saveManualExecutions !== undefined) { // Apply to workflow override - saveManualExecutions = this.workflowData.settings.saveManualExecutions as boolean; + saveManualExecutions = workflowSettings.saveManualExecutions as boolean; } if (isManualMode && !saveManualExecutions && !fullRunData.waitTill) { @@ -1024,16 +1012,14 @@ async function executeWorkflow( additionalDataIntegrated.executeWorkflow = additionalData.executeWorkflow; let subworkflowTimeout = additionalData.executionTimeoutTimestamp; - if ( - workflowData.settings?.executionTimeout !== undefined && - workflowData.settings.executionTimeout > 0 - ) { + const workflowSettings = workflowData.settings; + if (workflowSettings?.executionTimeout !== undefined && workflowSettings.executionTimeout > 0) { // We might have received a max timeout timestamp from the parent workflow // If we did, then we get the minimum time between the two timeouts // If no timeout was given from the parent, then we use our timeout. subworkflowTimeout = Math.min( additionalData.executionTimeoutTimestamp || Number.MAX_SAFE_INTEGER, - Date.now() + (workflowData.settings.executionTimeout as number) * 1000, + Date.now() + workflowSettings.executionTimeout * 1000, ); } diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 2d67afd65a..16683b57c9 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -21,6 +21,7 @@ import type { IRun, WorkflowExecuteMode, WorkflowHooks, + WorkflowSettings, } from 'n8n-workflow'; import { ErrorReporterProxy as ErrorReporter, @@ -248,11 +249,9 @@ export class WorkflowRunner { // Changes were made by adding the `workflowTimeout` to the `additionalData` // So that the timeout will also work for executions with nested workflows. let executionTimeout: NodeJS.Timeout; - let workflowTimeout = config.getEnv('executions.timeout'); // initialize with default - if (data.workflowData.settings && data.workflowData.settings.executionTimeout) { - workflowTimeout = data.workflowData.settings.executionTimeout as number; // preference on workflow setting - } + const workflowSettings = data.workflowData.settings ?? {}; + let workflowTimeout = workflowSettings.executionTimeout ?? config.getEnv('executions.timeout'); // initialize with default if (workflowTimeout > 0) { workflowTimeout = Math.min(workflowTimeout, config.getEnv('executions.maxTimeout')); } @@ -265,7 +264,7 @@ export class WorkflowRunner { active: data.workflowData.active, nodeTypes, staticData: data.workflowData.staticData, - settings: data.workflowData.settings, + settings: workflowSettings, }); const additionalData = await WorkflowExecuteAdditionalData.getBase( data.userId, @@ -590,16 +589,12 @@ export class WorkflowRunner { try { // Check if this execution data has to be removed from database // based on workflow settings. - let saveDataErrorExecution = config.getEnv('executions.saveDataOnError') as string; - let saveDataSuccessExecution = config.getEnv('executions.saveDataOnSuccess') as string; - if (data.workflowData.settings !== undefined) { - saveDataErrorExecution = - (data.workflowData.settings.saveDataErrorExecution as string) || - saveDataErrorExecution; - saveDataSuccessExecution = - (data.workflowData.settings.saveDataSuccessExecution as string) || - saveDataSuccessExecution; - } + const workflowSettings = data.workflowData.settings ?? {}; + const saveDataErrorExecution = + workflowSettings.saveDataErrorExecution ?? config.getEnv('executions.saveDataOnError'); + const saveDataSuccessExecution = + workflowSettings.saveDataSuccessExecution ?? + config.getEnv('executions.saveDataOnSuccess'); const workflowDidSucceed = !runData.data.resultData.error; if ( @@ -666,10 +661,9 @@ export class WorkflowRunner { // Start timeout for the execution let executionTimeout: NodeJS.Timeout; - let workflowTimeout = config.getEnv('executions.timeout'); // initialize with default - if (data.workflowData.settings && data.workflowData.settings.executionTimeout) { - workflowTimeout = data.workflowData.settings.executionTimeout as number; // preference on workflow setting - } + + const workflowSettings = data.workflowData.settings ?? {}; + let workflowTimeout = workflowSettings.executionTimeout ?? config.getEnv('executions.timeout'); // initialize with default const processTimeoutFunction = (timeout: number) => { this.activeExecutions.stopExecution(executionId, 'timeout'); diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index e3716e41fa..32c0d6c93c 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -130,13 +130,10 @@ class WorkflowRunnerProcess { const license = Container.get(License); await license.init(instanceId); - // Start timeout for the execution - let workflowTimeout = config.getEnv('executions.timeout'); // initialize with default - // eslint-disable-next-line @typescript-eslint/prefer-optional-chain - if (this.data.workflowData.settings && this.data.workflowData.settings.executionTimeout) { - workflowTimeout = this.data.workflowData.settings.executionTimeout as number; // preference on workflow setting - } + const workflowSettings = this.data.workflowData.settings ?? {}; + // Start timeout for the execution + let workflowTimeout = workflowSettings.executionTimeout ?? config.getEnv('executions.timeout'); // initialize with default if (workflowTimeout > 0) { workflowTimeout = Math.min(workflowTimeout, config.getEnv('executions.maxTimeout')); } diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index c701d71186..48d2269d0e 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -127,14 +127,9 @@ export class Worker extends BaseCommand { staticData = workflowData.staticData; } - let workflowTimeout = config.getEnv('executions.timeout'); // initialize with default - if ( - // eslint-disable-next-line @typescript-eslint/prefer-optional-chain - currentExecutionDb.workflowData.settings && - currentExecutionDb.workflowData.settings.executionTimeout - ) { - workflowTimeout = currentExecutionDb.workflowData.settings.executionTimeout as number; // preference on workflow setting - } + const workflowSettings = currentExecutionDb.workflowData.settings ?? {}; + + let workflowTimeout = workflowSettings.executionTimeout ?? config.getEnv('executions.timeout'); // initialize with default let executionTimeoutTimestamp: number | undefined; if (workflowTimeout > 0) { diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index 58a0a56a7b..8563038e4c 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -245,32 +245,27 @@ export class WorkflowsService { await Container.get(ActiveWorkflowRunner).remove(workflowId); } - if (workflow.settings) { - if (workflow.settings.timezone === 'DEFAULT') { - // Do not save the default timezone - delete workflow.settings.timezone; - } - if (workflow.settings.saveDataErrorExecution === 'DEFAULT') { - // Do not save when default got set - delete workflow.settings.saveDataErrorExecution; - } - if (workflow.settings.saveDataSuccessExecution === 'DEFAULT') { - // Do not save when default got set - delete workflow.settings.saveDataSuccessExecution; - } - if (workflow.settings.saveManualExecutions === 'DEFAULT') { - // Do not save when default got set - delete workflow.settings.saveManualExecutions; - } - if ( - parseInt(workflow.settings.executionTimeout as string, 10) === - config.get('executions.timeout') - ) { - // Do not save when default got set - delete workflow.settings.executionTimeout; + const workflowSettings = workflow.settings ?? {}; + + const keysAllowingDefault = [ + 'timezone', + 'saveDataErrorExecution', + 'saveDataSuccessExecution', + 'saveManualExecutions', + 'saveExecutionProgress', + ] as const; + for (const key of keysAllowingDefault) { + // Do not save the default value + if (workflowSettings[key] === 'DEFAULT') { + delete workflowSettings[key]; } } + if (workflowSettings.executionTimeout === config.get('executions.timeout')) { + // Do not save when default got set + delete workflowSettings.executionTimeout; + } + if (workflow.name) { workflow.updatedAt = new Date(); // required due to atomic update await validateEntity(workflow); diff --git a/packages/workflow/src/Expression.ts b/packages/workflow/src/Expression.ts index 485cce8b4c..61dd617250 100644 --- a/packages/workflow/src/Expression.ts +++ b/packages/workflow/src/Expression.ts @@ -66,7 +66,7 @@ export class Expression { if (value instanceof Date) { // We don't want to use JSON.stringify for dates since it disregards workflow timezone result = DateTime.fromJSDate(value, { - zone: this.workflow.settings.timezone?.toString() ?? 'default', + zone: this.workflow.settings?.timezone ?? 'default', }).toISO(); } else { result = JSON.stringify(value); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 8d21e5c6c6..0a598b1b2d 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -1709,8 +1709,21 @@ export interface IWorkflowHooksOptionalParameters { sessionId?: string; } +export namespace WorkflowSettings { + export type CallerPolicy = 'any' | 'none' | 'workflowsFromAList' | 'workflowsFromSameOwner'; + export type SaveDataExecution = 'DEFAULT' | 'all' | 'none'; +} + export interface IWorkflowSettings { - [key: string]: IDataObject | string | number | boolean | undefined; + timezone?: 'DEFAULT' | string; + errorWorkflow?: string; + callerIds?: string; + callerPolicy?: WorkflowSettings.CallerPolicy; + saveDataErrorExecution?: WorkflowSettings.SaveDataExecution; + saveDataSuccessExecution?: WorkflowSettings.SaveDataExecution; + saveManualExecutions?: 'DEFAULT' | boolean; + saveExecutionProgress?: 'DEFAULT' | boolean; + executionTimeout?: number; } export type LogTypes = 'debug' | 'verbose' | 'info' | 'warn' | 'error'; diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index 31b0e8c666..b90b21a134 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -111,7 +111,7 @@ export class WorkflowDataProxy { this.siblingParameters = siblingParameters; this.mode = mode; this.defaultTimezone = defaultTimezone; - this.timezone = (this.workflow.settings.timezone as string) || this.defaultTimezone; + this.timezone = workflow.settings?.timezone ?? defaultTimezone; this.selfData = selfData; this.additionalKeys = additionalKeys; this.executeData = executeData;