Skip to content

Commit

Permalink
fix: Sync partial execution version of FE and BE, also allow enforcin…
Browse files Browse the repository at this point in the history
…g a specific version (#12840)

Co-authored-by: Iván Ovejero <[email protected]>
  • Loading branch information
despairblue and ivov authored Feb 3, 2025
1 parent a65a9e6 commit a155043
Show file tree
Hide file tree
Showing 21 changed files with 193 additions and 62 deletions.
1 change: 1 addition & 0 deletions packages/@n8n/api-types/src/dto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export { CredentialsGetOneRequestQuery } from './credentials/credentials-get-one
export { CredentialsGetManyRequestQuery } from './credentials/credentials-get-many-request.dto';

export { ImportWorkflowFromUrlDto } from './workflows/import-workflow-from-url.dto';
export { ManualRunQueryDto } from './workflows/manual-run-query.dto';

export { CreateOrUpdateTagRequestDto } from './tag/create-or-update-tag-request.dto';
export { RetrieveTagQueryDto } from './tag/retrieve-tag-query.dto';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { ManualRunQueryDto } from '../manual-run-query.dto';

describe('ManualRunQueryDto', () => {
describe('Valid requests', () => {
test.each([
{ name: 'version number 1', partialExecutionVersion: '1' },
{ name: 'version number 2', partialExecutionVersion: '2' },
{ name: 'missing version' },
])('should validate $name', ({ partialExecutionVersion }) => {
const result = ManualRunQueryDto.safeParse({ partialExecutionVersion });

if (!result.success) {
return fail('expected validation to succeed');
}
expect(result.success).toBe(true);
expect(typeof result.data.partialExecutionVersion).toBe('number');
});
});

describe('Invalid requests', () => {
test.each([
{
name: 'invalid version 0',
partialExecutionVersion: '0',
expectedErrorPath: ['partialExecutionVersion'],
},
{
name: 'invalid type (boolean)',
partialExecutionVersion: true,
expectedErrorPath: ['partialExecutionVersion'],
},
{
name: 'invalid type (number)',
partialExecutionVersion: 1,
expectedErrorPath: ['partialExecutionVersion'],
},
])('should fail validation for $name', ({ partialExecutionVersion, expectedErrorPath }) => {
const result = ManualRunQueryDto.safeParse({ partialExecutionVersion });

if (result.success) {
return fail('expected validation to fail');
}

expect(result.error.issues[0].path).toEqual(expectedErrorPath);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { z } from 'zod';
import { Z } from 'zod-class';

export class ManualRunQueryDto extends Z.class({
partialExecutionVersion: z
.enum(['1', '2'])
.default('1')
.transform((version) => Number.parseInt(version) as 1 | 2),
}) {}
4 changes: 4 additions & 0 deletions packages/@n8n/api-types/src/frontend-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,8 @@ export interface FrontendSettings {
};
betaFeatures: FrontendBetaFeatures[];
easyAIWorkflowOnboarded: boolean;
partialExecution: {
version: 1 | 2;
enforce: boolean;
};
}
12 changes: 12 additions & 0 deletions packages/@n8n/config/src/configs/partial-executions.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Config, Env } from '../decorators';

@Config
export class PartialExecutionsConfig {
/** Partial execution logic version to use by default. */
@Env('N8N_PARTIAL_EXECUTION_VERSION_DEFAULT')
version: 1 | 2 = 1;

/** Set this to true to enforce using the default version. Users cannot use the other version then by setting a local storage key. */
@Env('N8N_PARTIAL_EXECUTION_ENFORCE_VERSION')
enforce: boolean = false;
}
4 changes: 4 additions & 0 deletions packages/@n8n/config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { LicenseConfig } from './configs/license.config';
import { LoggingConfig } from './configs/logging.config';
import { MultiMainSetupConfig } from './configs/multi-main-setup.config';
import { NodesConfig } from './configs/nodes.config';
import { PartialExecutionsConfig } from './configs/partial-executions.config';
import { PublicApiConfig } from './configs/public-api.config';
import { TaskRunnersConfig } from './configs/runners.config';
import { ScalingModeConfig } from './configs/scaling-mode.config';
Expand Down Expand Up @@ -134,4 +135,7 @@ export class GlobalConfig {

@Nested
tags: TagsConfig;

@Nested
partialExecutions: PartialExecutionsConfig;
}
4 changes: 4 additions & 0 deletions packages/@n8n/config/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ describe('GlobalConfig', () => {
tags: {
disabled: false,
},
partialExecutions: {
version: 1,
enforce: false,
},
};

it('should use all default values when no env variables are defined', () => {
Expand Down
9 changes: 0 additions & 9 deletions packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,13 +370,4 @@ export const schema = {
env: 'N8N_PROXY_HOPS',
doc: 'Number of reverse-proxies n8n is running behind',
},

featureFlags: {
partialExecutionVersionDefault: {
format: String,
default: '0',
env: 'PARTIAL_EXECUTION_VERSION_DEFAULT',
doc: 'Set this to 1 to enable the new partial execution logic by default.',
},
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export class TestRunnerService {
pinData,
workflowData: { ...workflow, pinData },
userId: metadata.userId,
partialExecutionVersion: '1',
partialExecutionVersion: 2,
};

// Trigger the workflow under test with mocked data
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/manual-execution.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class ManualExecutionService {
// Execute only the nodes between start and destination nodes
const workflowExecute = new WorkflowExecute(additionalData, data.executionMode);

if (data.partialExecutionVersion === '1') {
if (data.partialExecutionVersion === 2) {
return workflowExecute.runPartialWorkflow2(
workflow,
data.runData,
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/services/frontend.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ export class FrontendService {
},
betaFeatures: this.frontendConfig.betaFeatures,
easyAIWorkflowOnboarded: false,
partialExecution: this.globalConfig.partialExecutions,
};
}

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/workflows/workflow-execution.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class WorkflowExecutionService {
}: WorkflowRequest.ManualRunPayload,
user: User,
pushRef?: string,
partialExecutionVersion?: string,
partialExecutionVersion: 1 | 2 = 1,
) {
const pinData = workflowData.pinData;
const pinnedTrigger = this.selectPinnedActivatorStarter(
Expand Down Expand Up @@ -142,7 +142,7 @@ export class WorkflowExecutionService {
startNodes,
workflowData,
userId: user.id,
partialExecutionVersion: partialExecutionVersion ?? '0',
partialExecutionVersion,
dirtyNodeNames,
triggerToStartFrom,
};
Expand Down
7 changes: 1 addition & 6 deletions packages/cli/src/workflows/workflow.request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,7 @@ export declare namespace WorkflowRequest {

type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string }>;

type ManualRun = AuthenticatedRequest<
{ workflowId: string },
{},
ManualRunPayload,
{ partialExecutionVersion?: string }
>;
type ManualRun = AuthenticatedRequest<{ workflowId: string }, {}, ManualRunPayload, {}>;

type Share = AuthenticatedRequest<{ workflowId: string }, {}, { shareWithIds: string[] }>;

Expand Down
13 changes: 7 additions & 6 deletions packages/cli/src/workflows/workflows.controller.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ImportWorkflowFromUrlDto } from '@n8n/api-types';
import { ImportWorkflowFromUrlDto, ManualRunQueryDto } from '@n8n/api-types';
import { GlobalConfig } from '@n8n/config';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { In, type FindOptionsRelations } from '@n8n/typeorm';
Expand All @@ -9,7 +9,6 @@ import { ApplicationError } from 'n8n-workflow';
import { v4 as uuid } from 'uuid';
import { z } from 'zod';

import config from '@/config';
import type { Project } from '@/databases/entities/project';
import { SharedWorkflow } from '@/databases/entities/shared-workflow';
import { WorkflowEntity } from '@/databases/entities/workflow-entity';
Expand Down Expand Up @@ -367,7 +366,11 @@ export class WorkflowsController {

@Post('/:workflowId/run')
@ProjectScope('workflow:execute')
async runManually(req: WorkflowRequest.ManualRun) {
async runManually(
req: WorkflowRequest.ManualRun,
_res: unknown,
@Query query: ManualRunQueryDto,
) {
if (!req.body.workflowData.id) {
throw new ApplicationError('You cannot execute a workflow without an ID', {
level: 'warning',
Expand Down Expand Up @@ -395,9 +398,7 @@ export class WorkflowsController {
req.body,
req.user,
req.headers['push-ref'],
req.query.partialExecutionVersion === '-1'
? config.getEnv('featureFlags.partialExecutionVersionDefault')
: req.query.partialExecutionVersion,
query.partialExecutionVersion,
);
}

Expand Down
4 changes: 4 additions & 0 deletions packages/editor-ui/src/__tests__/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,8 @@ export const defaultSettings: FrontendSettings = {
},
betaFeatures: [],
easyAIWorkflowOnboarded: false,
partialExecution: {
version: 1,
enforce: false,
},
};
31 changes: 11 additions & 20 deletions packages/editor-ui/src/composables/useRunWorkflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ import { useUIStore } from '@/stores/ui.store';
import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
import { useToast } from './useToast';
import { useI18n } from '@/composables/useI18n';
import { useLocalStorage } from '@vueuse/core';
import { ref } from 'vue';
import { captor, mock } from 'vitest-mock-extended';
import { useSettingsStore } from '@/stores/settings.store';

vi.mock('@/stores/workflows.store', () => ({
useWorkflowsStore: vi.fn().mockReturnValue({
Expand All @@ -41,16 +40,6 @@ vi.mock('@/stores/workflows.store', () => ({
}),
}));

vi.mock('@vueuse/core', async () => {
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
const originalModule = await vi.importActual<typeof import('@vueuse/core')>('@vueuse/core');

return {
...originalModule, // Keep all original exports
useLocalStorage: vi.fn().mockReturnValue({ value: undefined }), // Mock useLocalStorage
};
});

vi.mock('@/composables/useTelemetry', () => ({
useTelemetry: vi.fn().mockReturnValue({ track: vi.fn() }),
}));
Expand Down Expand Up @@ -106,6 +95,7 @@ describe('useRunWorkflow({ router })', () => {
let workflowsStore: ReturnType<typeof useWorkflowsStore>;
let router: ReturnType<typeof useRouter>;
let workflowHelpers: ReturnType<typeof useWorkflowHelpers>;
let settingsStore: ReturnType<typeof useSettingsStore>;

beforeAll(() => {
const pinia = createTestingPinia({ stubActions: false });
Expand All @@ -115,6 +105,7 @@ describe('useRunWorkflow({ router })', () => {
rootStore = useRootStore();
uiStore = useUIStore();
workflowsStore = useWorkflowsStore();
settingsStore = useSettingsStore();

router = useRouter();
workflowHelpers = useWorkflowHelpers({ router });
Expand Down Expand Up @@ -322,8 +313,8 @@ describe('useRunWorkflow({ router })', () => {
expect(result).toEqual(mockExecutionResponse);
});

it('should send dirty nodes for partial executions', async () => {
vi.mocked(useLocalStorage).mockReturnValueOnce(ref(1));
it('should send dirty nodes for partial executions v2', async () => {
vi.mocked(settingsStore).partialExecutionVersion = 2;
const composable = useRunWorkflow({ router });
const parentName = 'When clicking';
const executeName = 'Code';
Expand Down Expand Up @@ -404,7 +395,7 @@ describe('useRunWorkflow({ router })', () => {
);
});

it('does not use the original run data if `PartialExecution.version` is set to 0', async () => {
it('does not use the original run data if `partialExecutionVersion` is set to 1', async () => {
// ARRANGE
const mockExecutionResponse = { executionId: '123' };
const mockRunData = { nodeName: [] };
Expand All @@ -413,7 +404,7 @@ describe('useRunWorkflow({ router })', () => {
const workflow = mock<Workflow>({ name: 'Test Workflow' });
workflow.getParentNodes.mockReturnValue([]);

vi.mocked(useLocalStorage).mockReturnValueOnce(ref(0));
vi.mocked(settingsStore).partialExecutionVersion = 1;
vi.mocked(rootStore).pushConnectionActive = true;
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
vi.mocked(workflowsStore).nodesIssuesExist = false;
Expand All @@ -435,7 +426,7 @@ describe('useRunWorkflow({ router })', () => {
});
});

it('retains the original run data if `PartialExecution.version` is set to 1', async () => {
it('retains the original run data if `partialExecutionVersion` is set to 2', async () => {
// ARRANGE
const mockExecutionResponse = { executionId: '123' };
const mockRunData = { nodeName: [] };
Expand All @@ -444,7 +435,7 @@ describe('useRunWorkflow({ router })', () => {
const workflow = mock<Workflow>({ name: 'Test Workflow' });
workflow.getParentNodes.mockReturnValue([]);

vi.mocked(useLocalStorage).mockReturnValueOnce(ref(1));
vi.mocked(settingsStore).partialExecutionVersion = 2;
vi.mocked(rootStore).pushConnectionActive = true;
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
vi.mocked(workflowsStore).nodesIssuesExist = false;
Expand All @@ -464,7 +455,7 @@ describe('useRunWorkflow({ router })', () => {
expect(dataCaptor.value).toMatchObject({ data: { resultData: { runData: mockRunData } } });
});

it("does not send run data if it's not a partial execution even if `PartialExecution.version` is set to 1", async () => {
it("does not send run data if it's not a partial execution even if `partialExecutionVersion` is set to 2", async () => {
// ARRANGE
const mockExecutionResponse = { executionId: '123' };
const mockRunData = { nodeName: [] };
Expand All @@ -473,7 +464,7 @@ describe('useRunWorkflow({ router })', () => {
const workflow = mock<Workflow>({ name: 'Test Workflow' });
workflow.getParentNodes.mockReturnValue([]);

vi.mocked(useLocalStorage).mockReturnValueOnce(ref(1));
vi.mocked(settingsStore).partialExecutionVersion = 2;
vi.mocked(rootStore).pushConnectionActive = true;
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
vi.mocked(workflowsStore).nodesIssuesExist = false;
Expand Down
12 changes: 6 additions & 6 deletions packages/editor-ui/src/composables/useRunWorkflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { isEmpty } from '@/utils/typesUtils';
import { useI18n } from '@/composables/useI18n';
import { get } from 'lodash-es';
import { useExecutionsStore } from '@/stores/executions.store';
import { useLocalStorage } from '@vueuse/core';
import { useSettingsStore } from '@/stores/settings.store';

const getDirtyNodeNames = (
runData: IRunData,
Expand Down Expand Up @@ -260,18 +260,18 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType<typeof u
return undefined;
}

// -1 means the backend chooses the default
// 0 is the old flow
// 1 is the new flow
const partialExecutionVersion = useLocalStorage('PartialExecution.version', -1);
// partial executions must have a destination node
const isPartialExecution = options.destinationNode !== undefined;
const settingsStore = useSettingsStore();
const version = settingsStore.partialExecutionVersion;
const startRunData: IStartRunData = {
workflowData,
// With the new partial execution version the backend decides what run
// data to use and what to ignore.
runData: !isPartialExecution
? // if it's a full execution we don't want to send any run data
undefined
: partialExecutionVersion.value === 1
: version === 2
? // With the new partial execution version the backend decides
//what run data to use and what to ignore.
(runData ?? undefined)
Expand Down
Loading

0 comments on commit a155043

Please sign in to comment.