feat(editor): Handle pin data edge cases and unify validation (no-changelog) (#6685)

Github issue / Community forum post (link here to close automatically):
This commit is contained in:
Alex Grozav 2023-11-02 10:43:02 +02:00 committed by GitHub
parent 27f37091c8
commit 721a36637c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 238 additions and 142 deletions

View file

@ -99,6 +99,8 @@ jobs:
runTests: false
install: false
build: pnpm build
env:
VUE_APP_MAX_PINNED_DATA_SIZE: 16384
- name: Cypress install
run: pnpm cypress:install

View file

@ -19,4 +19,9 @@ module.exports = defineConfig({
experimentalInteractiveRunEvents: true,
experimentalSessionAndOrigin: true,
},
env: {
MAX_PINNED_DATA_SIZE: process.env.VUE_APP_MAX_PINNED_DATA_SIZE
? parseInt(process.env.VUE_APP_MAX_PINNED_DATA_SIZE, 10)
: 16 * 1024,
},
});

View file

@ -62,13 +62,49 @@ describe('Data pinning', () => {
workflowPage.actions.saveWorkflowOnButtonClick();
cy.reload();
workflowPage.actions.openNode('Schedule Trigger');
ndv.getters.outputTableHeaders().first().should('include.text', 'test');
ndv.getters.outputTbodyCell(1, 0).should('include.text', 1);
});
it('Should be duplicating pin data when duplicating node', () => {
workflowPage.actions.addInitialNodeToCanvas('Schedule Trigger');
workflowPage.actions.addNodeToCanvas('Edit Fields', true, true);
ndv.getters.container().should('be.visible');
ndv.getters.pinDataButton().should('not.exist');
ndv.getters.editPinnedDataButton().should('be.visible');
ndv.actions.setPinnedData([{ test: 1 }]);
ndv.actions.close();
workflowPage.actions.duplicateNode(workflowPage.getters.canvasNodes().last());
workflowPage.actions.saveWorkflowOnButtonClick();
workflowPage.actions.openNode('Edit Fields1');
ndv.getters.outputTableHeaders().first().should('include.text', 'test');
ndv.getters.outputTbodyCell(1, 0).should('include.text', 1);
});
it('Should show an error when maximum pin data size is exceeded', () => {
workflowPage.actions.addInitialNodeToCanvas('Schedule Trigger');
workflowPage.actions.addNodeToCanvas('Edit Fields', true, true);
ndv.getters.container().should('be.visible');
ndv.getters.pinDataButton().should('not.exist');
ndv.getters.editPinnedDataButton().should('be.visible');
ndv.actions.setPinnedData([
{
test: '1'.repeat(Cypress.env('MAX_PINNED_DATA_SIZE')),
},
]);
workflowPage.getters
.errorToast()
.should('contain', 'Workflow has reached the maximum allowed pinned data size');
});
it('Should be able to reference paired items in a node located before pinned data', () => {
workflowPage.actions.addInitialNodeToCanvas(MANUAL_TRIGGER_NODE_NAME);
workflowPage.actions.addNodeToCanvas(HTTP_REQUEST_NODE_NAME, true, true);

View file

@ -104,7 +104,12 @@ export class NDV extends BasePage {
this.getters.pinnedDataEditor().click();
this.getters
.pinnedDataEditor()
.type(`{selectall}{backspace}${JSON.stringify(data).replace(new RegExp('{', 'g'), '{{}')}`);
.type(
`{selectall}{backspace}${JSON.stringify(data).replace(new RegExp('{', 'g'), '{{}')}`,
{
delay: 0,
},
);
this.actions.savePinnedData();
},

View file

@ -2,6 +2,7 @@ import { META_KEY } from '../constants';
import { BasePage } from './base';
import { getVisibleSelect } from '../utils';
import { NodeCreator } from './features/node-creator';
import Chainable = Cypress.Chainable;
const nodeCreator = new NodeCreator();
export class WorkflowPage extends BasePage {
@ -46,8 +47,8 @@ export class WorkflowPage extends BasePage {
canvasNodePlusEndpointByName: (nodeName: string, index = 0) => {
return cy.get(this.getters.getEndpointSelector('plus', nodeName, index));
},
successToast: () => cy.get('.el-notification .el-notification--success').parent(),
errorToast: () => cy.get('.el-notification .el-notification--error'),
successToast: () => cy.get('.el-notification:has(.el-notification--success)'),
errorToast: () => cy.get('.el-notification:has(.el-notification--error)'),
activatorSwitch: () => cy.getByTestId('workflow-activate-switch'),
workflowMenu: () => cy.getByTestId('workflow-menu'),
firstStepButton: () => cy.getByTestId('canvas-add-button'),
@ -186,6 +187,9 @@ export class WorkflowPage extends BasePage {
openNode: (nodeTypeName: string) => {
this.getters.canvasNodeByName(nodeTypeName).first().dblclick();
},
duplicateNode: (node: Chainable<JQuery<HTMLElement>>) => {
node.find('[data-test-id="duplicate-node-button"]').click({ force: true });
},
openExpressionEditorModal: () => {
cy.contains('Expression').invoke('show').click();
cy.getByTestId('expander').invoke('show').click();

View file

@ -660,24 +660,12 @@ export default defineComponent({
if (shouldPinDataBeforeClosing === MODAL_CONFIRM) {
const { value } = this.outputPanelEditMode;
if (!this.isValidPinDataSize(value)) {
dataPinningEventBus.emit('data-pinning-error', {
errorType: 'data-too-large',
source: 'on-ndv-close-modal',
});
return;
}
if (!this.isValidPinDataJSON(value)) {
dataPinningEventBus.emit('data-pinning-error', {
errorType: 'invalid-json',
source: 'on-ndv-close-modal',
});
return;
}
if (this.activeNode) {
this.workflowsStore.pinData({ node: this.activeNode, data: jsonParse(value) });
try {
this.setPinData(this.activeNode, jsonParse(value), 'on-ndv-close-modal');
} catch (error) {
console.error(error);
}
}
}

View file

@ -34,7 +34,6 @@ import type { INodeUi } from '@/Interface';
import type { INodeTypeDescription } from 'n8n-workflow';
import { workflowRun } from '@/mixins/workflowRun';
import { pinData } from '@/mixins/pinData';
import { dataPinningEventBus } from '@/event-bus';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useNDVStore } from '@/stores/ndv.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
@ -228,9 +227,8 @@ export default defineComponent({
);
shouldUnpinAndExecute = confirmResult === MODAL_CONFIRM;
if (shouldUnpinAndExecute) {
dataPinningEventBus.emit('data-unpinning', { source: 'unpin-and-execute-modal' });
this.workflowsStore.unpinData({ node: this.node });
if (shouldUnpinAndExecute && this.node) {
this.unsetPinData(this.node, 'unpin-and-execute-modal');
}
}

View file

@ -538,9 +538,10 @@ import { externalHooks } from '@/mixins/externalHooks';
import { genericHelpers } from '@/mixins/genericHelpers';
import { nodeHelpers } from '@/mixins/nodeHelpers';
import { pinData } from '@/mixins/pinData';
import type { PinDataSource } from '@/mixins/pinData';
import CodeNodeEditor from '@/components/CodeNodeEditor/CodeNodeEditor.vue';
import { dataPinningEventBus } from '@/event-bus';
import { clearJsonKey, executionDataToJson, stringSizeInBytes, isEmpty } from '@/utils';
import { clearJsonKey, executionDataToJson, isEmpty } from '@/utils';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useNDVStore } from '@/stores/ndv.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
@ -646,9 +647,6 @@ export default defineComponent({
this.init();
if (!this.isPaneTypeInput) {
dataPinningEventBus.on('data-pinning-error', this.onDataPinningError);
dataPinningEventBus.on('data-unpinning', this.onDataUnpinning);
this.showPinDataDiscoveryTooltip(this.jsonData);
}
this.ndvStore.setNDVBranchIndex({
@ -660,8 +658,6 @@ export default defineComponent({
},
beforeUnmount() {
this.hidePinDataDiscoveryTooltip();
dataPinningEventBus.off('data-pinning-error', this.onDataPinningError);
dataPinningEventBus.off('data-unpinning', this.onDataUnpinning);
},
computed: {
...mapStores(useNodeTypesStore, useNDVStore, useWorkflowsStore),
@ -1028,27 +1024,22 @@ export default defineComponent({
this.onExitEditMode({ type: 'cancel' });
},
onClickSaveEdit() {
if (!this.node) {
return;
}
const { value } = this.editMode;
this.clearAllStickyNotifications();
if (!this.isValidPinDataSize(value)) {
this.onDataPinningError({ errorType: 'data-too-large', source: 'save-edit' });
return;
}
if (!this.isValidPinDataJSON(value)) {
this.onDataPinningError({ errorType: 'invalid-json', source: 'save-edit' });
try {
this.setPinData(this.node, clearJsonKey(value) as INodeExecutionData[], 'save-edit');
} catch (error) {
console.error(error);
return;
}
this.ndvStore.setOutputPanelEditModeEnabled(false);
this.workflowsStore.pinData({
node: this.node,
data: clearJsonKey(value) as INodeExecutionData[],
});
this.onDataPinningSuccess({ source: 'save-edit' });
this.onExitEditMode({ type: 'save' });
},
@ -1061,53 +1052,11 @@ export default defineComponent({
type,
});
},
onDataUnpinning({
source,
}: {
source: 'banner-link' | 'pin-icon-click' | 'unpin-and-execute-modal';
}) {
this.$telemetry.track('User unpinned ndv data', {
node_type: this.activeNode?.type,
session_id: this.sessionId,
run_index: this.runIndex,
source,
data_size: stringSizeInBytes(this.pinData),
});
},
onDataPinningSuccess({ source }: { source: 'pin-icon-click' | 'save-edit' }) {
const telemetryPayload = {
pinning_source: source,
node_type: this.activeNode.type,
session_id: this.sessionId,
data_size: stringSizeInBytes(this.pinData),
view: this.displayMode,
run_index: this.runIndex,
};
void this.$externalHooks().run('runData.onDataPinningSuccess', telemetryPayload);
this.$telemetry.track('Ndv data pinning success', telemetryPayload);
},
onDataPinningError({
errorType,
source,
}: {
errorType: 'data-too-large' | 'invalid-json';
source: 'on-ndv-close-modal' | 'pin-icon-click' | 'save-edit';
}) {
this.$telemetry.track('Ndv data pinning failure', {
pinning_source: source,
node_type: this.activeNode.type,
session_id: this.sessionId,
data_size: stringSizeInBytes(this.pinData),
view: this.displayMode,
run_index: this.runIndex,
error_type: errorType,
});
},
async onTogglePinData({
source,
}: {
source: 'banner-link' | 'pin-icon-click' | 'unpin-and-execute-modal';
}) {
async onTogglePinData({ source }: { source: PinDataSource }) {
if (!this.node) {
return;
}
if (source === 'pin-icon-click') {
const telemetryPayload = {
node_type: this.activeNode.type,
@ -1123,20 +1072,17 @@ export default defineComponent({
this.updateNodeParameterIssues(this.node);
if (this.hasPinData) {
this.onDataUnpinning({ source });
this.workflowsStore.unpinData({ node: this.node });
this.unsetPinData(this.node, source);
return;
}
if (!this.isValidPinDataSize(this.rawInputData)) {
this.onDataPinningError({ errorType: 'data-too-large', source: 'pin-icon-click' });
try {
this.setPinData(this.node, this.rawInputData, 'pin-icon-click');
} catch (error) {
console.error(error);
return;
}
this.onDataPinningSuccess({ source: 'pin-icon-click' });
this.workflowsStore.pinData({ node: this.node, data: this.rawInputData });
if (this.maxRunIndex > 0) {
this.showToast({
title: this.$locale.baseText('ndv.pinData.pin.multipleRuns.title', {

View file

@ -1,9 +1,12 @@
import type { NodeCreatorOpenSource } from './Interface';
import { NodeConnectionType } from 'n8n-workflow';
export const MAX_WORKFLOW_SIZE = 16777216; // Workflow size limit in bytes
export const MAX_WORKFLOW_PINNED_DATA_SIZE = 12582912; // Workflow pinned data size limit in bytes
export const MAX_DISPLAY_DATA_SIZE = 204800;
export const MAX_WORKFLOW_SIZE = 1024 * 1024 * 16; // Workflow size limit in bytes
export const MAX_EXPECTED_REQUEST_SIZE = 2048; // Expected maximum workflow request metadata (i.e. headers) size in bytes
export const MAX_PINNED_DATA_SIZE = import.meta.env.VUE_APP_MAX_PINNED_DATA_SIZE
? parseInt(import.meta.env.VUE_APP_MAX_PINNED_DATA_SIZE, 10)
: 1024 * 1024 * 12; // Workflow pinned data size limit in bytes
export const MAX_DISPLAY_DATA_SIZE = 1024 * 200;
export const MAX_DISPLAY_ITEMS_AUTO_ALL = 250;
export const PLACEHOLDER_FILLED_AT_EXECUTION_TIME = '[filled at execution time]';

View file

@ -1,17 +1,27 @@
import { defineComponent } from 'vue';
import type { INodeUi } from '@/Interface';
import type { INodeTypeDescription, IPinData } from 'n8n-workflow';
import type { IPinData, INodeExecutionData } from 'n8n-workflow';
import { stringSizeInBytes } from '@/utils';
import { MAX_WORKFLOW_PINNED_DATA_SIZE, PIN_DATA_NODE_TYPES_DENYLIST } from '@/constants';
import {
MAX_EXPECTED_REQUEST_SIZE,
MAX_PINNED_DATA_SIZE,
MAX_WORKFLOW_SIZE,
PIN_DATA_NODE_TYPES_DENYLIST,
} from '@/constants';
import { mapStores } from 'pinia';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useNDVStore } from '@/stores/ndv.store';
import { useToast } from '@/composables';
import { jsonParse, jsonStringify } from 'n8n-workflow';
export interface IPinDataContext {
node: INodeUi;
nodeType: INodeTypeDescription;
$showError(error: Error, title: string): void;
}
export type PinDataSource =
| 'pin-icon-click'
| 'save-edit'
| 'on-ndv-close-modal'
| 'duplicate-node'
| 'add-nodes';
export type UnpinDataSource = 'unpin-and-execute-modal';
export const pinData = defineComponent({
setup() {
@ -20,7 +30,7 @@ export const pinData = defineComponent({
};
},
computed: {
...mapStores(useWorkflowsStore),
...mapStores(useWorkflowsStore, useNDVStore),
pinData(): IPinData[string] | undefined {
return this.node ? this.workflowsStore.pinDataByNodeName(this.node.name) : undefined;
},
@ -83,13 +93,16 @@ export const pinData = defineComponent({
return false;
}
},
isValidPinDataSize(data: string | object): boolean {
isValidPinDataSize(data: string | object, activeNodeName: string): boolean {
if (typeof data === 'object') data = JSON.stringify(data);
if (
this.workflowsStore.pinDataSize + stringSizeInBytes(data) >
MAX_WORKFLOW_PINNED_DATA_SIZE
) {
const { pinData: currentPinData, ...workflow } = this.workflowsStore.getCurrentWorkflow();
const workflowJson = jsonStringify(workflow, { replaceCircularRefs: true });
const newPinData = { ...currentPinData, [activeNodeName]: data };
const newPinDataSize = this.workflowsStore.getPinDataSize(newPinData);
if (newPinDataSize > MAX_PINNED_DATA_SIZE) {
this.showError(
new Error(this.$locale.baseText('ndv.pinData.error.tooLarge.description')),
this.$locale.baseText('ndv.pinData.error.tooLarge.title'),
@ -98,7 +111,83 @@ export const pinData = defineComponent({
return false;
}
if (
stringSizeInBytes(workflowJson) + newPinDataSize >
MAX_WORKFLOW_SIZE - MAX_EXPECTED_REQUEST_SIZE
) {
this.showError(
new Error(this.$locale.baseText('ndv.pinData.error.tooLargeWorkflow.description')),
this.$locale.baseText('ndv.pinData.error.tooLargeWorkflow.title'),
);
return false;
}
return true;
},
setPinData(node: INodeUi, data: string | INodeExecutionData[], source: PinDataSource): boolean {
if (typeof data === 'string') {
if (!this.isValidPinDataJSON(data)) {
this.onDataPinningError({ errorType: 'invalid-json', source });
throw new Error('Invalid JSON');
}
data = jsonParse(data);
}
if (!this.isValidPinDataSize(data, node.name)) {
this.onDataPinningError({ errorType: 'data-too-large', source });
throw new Error('Data too large');
}
this.onDataPinningSuccess({ source });
this.workflowsStore.pinData({ node, data: data as INodeExecutionData[] });
},
unsetPinData(node: INodeUi, source: UnpinDataSource): void {
this.onDataUnpinning({ source });
this.workflowsStore.unpinData({ node });
},
onDataPinningSuccess({ source }: { source: PinDataSource }) {
const telemetryPayload = {
pinning_source: source,
node_type: this.activeNode?.type,
session_id: this.sessionId,
data_size: stringSizeInBytes(this.pinData),
view: this.displayMode,
run_index: this.runIndex,
};
void this.$externalHooks().run('runData.onDataPinningSuccess', telemetryPayload);
this.$telemetry.track('Ndv data pinning success', telemetryPayload);
},
onDataPinningError({
errorType,
source,
}: {
errorType: 'data-too-large' | 'invalid-json';
source: PinDataSource;
}) {
this.$telemetry.track('Ndv data pinning failure', {
pinning_source: source,
node_type: this.activeNode?.type,
session_id: this.sessionId,
data_size: stringSizeInBytes(this.pinData),
view: this.displayMode,
run_index: this.runIndex,
error_type: errorType,
});
},
onDataUnpinning({
source,
}: {
source: 'banner-link' | 'pin-icon-click' | 'unpin-and-execute-modal';
}) {
this.$telemetry.track('User unpinned ndv data', {
node_type: this.activeNode?.type,
session_id: this.sessionId,
run_index: this.runIndex,
source,
data_size: stringSizeInBytes(this.pinData),
});
},
},
});

View file

@ -795,8 +795,10 @@
"ndv.pinData.beforeClosing.title": "Save output changes before closing?",
"ndv.pinData.beforeClosing.cancel": "Discard",
"ndv.pinData.beforeClosing.confirm": "Save",
"ndv.pinData.error.tooLarge.title": "Output data is too large to pin",
"ndv.pinData.error.tooLarge.description": "You can pin at most 12MB of output per workflow.",
"ndv.pinData.error.tooLarge.title": "Pinned data too big",
"ndv.pinData.error.tooLarge.description": "Workflow has reached the maximum allowed pinned data size",
"ndv.pinData.error.tooLargeWorkflow.title": "Pinned data too big",
"ndv.pinData.error.tooLargeWorkflow.description": "Workflow has reached the maximum allowed size",
"noTagsView.readyToOrganizeYourWorkflows": "Ready to organize your workflows?",
"noTagsView.withWorkflowTagsYouReFree": "With workflow tags, you're free to create the perfect tagging system for your flows",
"node.thisIsATriggerNode": "This is a Trigger node. <a target=\"_blank\" href=\"https://docs.n8n.io/workflows/components/nodes/\">Learn more</a>",

View file

@ -13,6 +13,7 @@ declare global {
PROD: boolean;
NODE_ENV: 'development' | 'production';
VUE_APP_URL_BASE_API: string;
VUE_APP_MAX_PINNED_DATA_SIZE: string;
};
}

View file

@ -61,7 +61,7 @@ import type {
import { deepCopy, NodeHelpers, Workflow } from 'n8n-workflow';
import { findLast } from 'lodash-es';
import { useRootStore } from './n8nRoot.store';
import { useRootStore } from '@/stores/n8nRoot.store';
import {
getActiveWorkflows,
getCurrentExecutions,
@ -71,7 +71,7 @@ import {
getWorkflow,
getWorkflows,
} from '@/api/workflows';
import { useUIStore } from './ui.store';
import { useUIStore } from '@/stores/ui.store';
import { dataPinningEventBus } from '@/event-bus';
import {
isJsonKeyObject,
@ -82,8 +82,8 @@ import {
makeRestApiRequest,
unflattenExecutionData,
} from '@/utils';
import { useNDVStore } from './ndv.store';
import { useNodeTypesStore } from './nodeTypes.store';
import { useNDVStore } from '@/stores/ndv.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import { useUsersStore } from '@/stores/users.store';
import { useSettingsStore } from '@/stores/settings.store';
@ -237,17 +237,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
getPinData(): IPinData | undefined {
return this.workflow.pinData;
},
pinDataSize(): number {
const ndvStore = useNDVStore();
const activeNode = ndvStore.activeNodeName;
return this.workflow.nodes.reduce((acc, node) => {
if (typeof node.pinData !== 'undefined' && node.name !== activeNode) {
acc += stringSizeInBytes(node.pinData);
}
return acc;
}, 0);
},
shouldReplaceInputDataWithPinData(): boolean {
return !this.activeWorkflowExecution || this.activeWorkflowExecution?.mode === 'manual';
},
@ -283,6 +272,11 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
},
},
actions: {
getPinDataSize(pinData: Record<string, string | INodeExecutionData[]> = {}): number {
return Object.values(pinData).reduce<number>((acc, value) => {
return acc + stringSizeInBytes(value);
}, 0);
},
getNodeTypes(): INodeTypes {
const nodeTypes: INodeTypes = {
nodeTypes: {},

View file

@ -241,6 +241,7 @@ import { useUniqueNodeName } from '@/composables/useUniqueNodeName';
import { useI18n } from '@/composables/useI18n';
import { workflowHelpers } from '@/mixins/workflowHelpers';
import { workflowRun } from '@/mixins/workflowRun';
import { pinData } from '@/mixins/pinData';
import NodeDetailsView from '@/components/NodeDetailsView.vue';
import Node from '@/components/Node.vue';
@ -367,6 +368,7 @@ export default defineComponent({
workflowHelpers,
workflowRun,
debounceHelper,
pinData,
],
components: {
NodeDetailsView,
@ -1719,10 +1721,6 @@ export default defineComponent({
});
});
if (workflowData.pinData) {
this.workflowsStore.setWorkflowPinData(workflowData.pinData);
}
const tagsEnabled = this.settingsStore.areTagsEnabled;
if (importTags && tagsEnabled && Array.isArray(workflowData.tags)) {
const allTags = await this.tagsStore.fetchAll();
@ -3234,12 +3232,13 @@ export default defineComponent({
await this.addNodes([newNodeData], [], true);
const pinData = this.workflowsStore.pinDataByNodeName(nodeName);
if (pinData) {
this.workflowsStore.pinData({
node: newNodeData,
data: pinData,
});
const pinDataForNode = this.workflowsStore.pinDataByNodeName(nodeName);
if (pinDataForNode?.length) {
try {
this.setPinData(newNodeData, pinDataForNode, 'duplicate-node');
} catch (error) {
console.error(error);
}
}
this.uiStore.stateIsDirty = true;
@ -3963,6 +3962,29 @@ export default defineComponent({
tempWorkflow.renameNode(oldName, nodeNameTable[oldName]);
}
if (data.pinData) {
let pinDataSuccess = true;
for (const nodeName of Object.keys(data.pinData)) {
// Pin data limit reached
if (!pinDataSuccess) {
this.showError(
new Error(this.$locale.baseText('ndv.pinData.error.tooLarge.description')),
this.$locale.baseText('ndv.pinData.error.tooLarge.title'),
);
continue;
}
const node = tempWorkflow.nodes[nodeNameTable[nodeName]];
try {
this.setPinData(node, data.pinData![nodeName], 'add-nodes');
pinDataSuccess = true;
} catch (error) {
pinDataSuccess = false;
console.error(error);
}
}
}
// Add the nodes with the changed node names, expressions and connections
this.historyStore.startRecordingUndo();
await this.addNodes(

View file

@ -13,6 +13,7 @@ function runTests(options) {
process.env.N8N_USER_FOLDER = userFolder;
process.env.E2E_TESTS = 'true';
process.env.NODE_OPTIONS = '--dns-result-order=ipv4first';
process.env.VUE_APP_MAX_PINNED_DATA_SIZE = `${16 * 1024}`;
if (options.customEnv) {
Object.keys(options.customEnv).forEach((key) => {