diff --git a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/validate-value-against-schema.test.ts b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/validate-value-against-schema.test.ts index 1037bedd92d..aea4e67142b 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/validate-value-against-schema.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/validate-value-against-schema.test.ts @@ -464,5 +464,136 @@ describe('validateValueAgainstSchema', () => { ).toThrow("Invalid input for 'count' [item 0]"); }); }); + + describe('schema field lookup', () => { + const nodeType = { + description: { + properties: [ + { + displayName: 'Columns', + name: 'columns', + type: 'resourceMapper', + typeOptions: { + resourceMapper: { + mode: 'add', + }, + }, + }, + ], + }, + } as unknown as INodeType; + + const schemaField = (id: string, type: string, required = false) => ({ + id, + displayName: id, + required, + defaultMatch: false, + display: true, + type, + canBeUsedToMatch: false, + }); + + const makeNode = (schema: unknown[], value: IDataObject) => + ({ + parameters: { + columns: { + mappingMode: 'defineBelow', + value, + matchingColumns: [], + attemptToConvertTypes: false, + schema, + }, + options: {}, + }, + id: '8d6cec63-8db1-440c-8966-4d6311ee69a9', + name: 'add products to DB', + type: 'n8n-nodes-base.postgres', + typeVersion: 2.3, + position: [420, 0], + }) as unknown as INode; + + const parameterName = 'columns.value'; + + test('resolves every value against its own schema entry, whatever its position', () => { + const schema = [ + schemaField('a', 'number'), + schemaField('b', 'string'), + schemaField('c', 'number'), + schemaField('d', 'boolean'), + ]; + const value = { d: 'true', c: '3', b: 2, a: '1' }; + + const result = validateValueAgainstSchema( + makeNode(schema, value), + nodeType, + value, + parameterName, + 0, + 0, + ); + + // `a`, `c` and `d` are cast to their schema type; `b` keeps its type, as + // validateFieldType does not coerce values for string fields. + expect(result).toEqual({ a: 1, b: 2, c: 3, d: true }); + }); + + test('uses the first entry when a field id appears twice, like Array.prototype.find', () => { + const schema = [schemaField('dup', 'number'), schemaField('dup', 'string')]; + const value = { dup: '42' }; + + const result = validateValueAgainstSchema( + makeNode(schema, value), + nodeType, + value, + parameterName, + 0, + 0, + ); + + // The first entry types `dup` as a number, so the string is cast. Had the second + // (string) entry won, the value would have stayed '42'. + expect(result).toEqual({ dup: 42 }); + }); + + test('leaves values that have no schema entry untouched', () => { + const schema = [schemaField('known', 'number')]; + const value = { known: '7', unknown: 'abc' }; + + const result = validateValueAgainstSchema( + makeNode(schema, value), + nodeType, + value, + parameterName, + 0, + 0, + ); + + expect(result).toEqual({ known: 7, unknown: 'abc' }); + }); + + // `isResourceMapperValue` only checks that `schema` is present, so a persisted + // `schema: null` reaches the lookup. Indexing now runs before the loop, where the + // previous `schema.find` only ran inside it — an empty value never touched it. + describe.each([ + ['null', null], + ['undefined', undefined], + ['empty', []], + ])('when the stored schema is %s', (_label, schema) => { + test('resolves no field instead of throwing', () => { + const value = { anything: 'kept' }; + + const result = validateValueAgainstSchema( + makeNode(schema as unknown as unknown[], value), + nodeType, + value, + parameterName, + 0, + 0, + ); + + expect(result).toEqual({ anything: 'kept' }); + }); + }); + }); }); }); diff --git a/packages/core/src/execution-engine/node-execution-context/utils/validate-value-against-schema.ts b/packages/core/src/execution-engine/node-execution-context/utils/validate-value-against-schema.ts index 5f0e2a9bba1..30a35e902d0 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/validate-value-against-schema.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/validate-value-against-schema.ts @@ -6,6 +6,7 @@ import type { INodePropertyCollection, INodePropertyOptions, INodeType, + ResourceMapperField, ResourceMapperTypeOptions, } from 'n8n-workflow'; import { @@ -17,6 +18,31 @@ import { import type { ExtendedValidationResult } from '@/interfaces'; +const schemaIndexCache = new WeakMap>(); + +const EMPTY_SCHEMA_INDEX: ReadonlyMap = new Map(); + +const indexSchemaById = ( + schema: ResourceMapperField[] | undefined, +): ReadonlyMap => { + // `isResourceMapperValue` only checks that `schema` is present, so a persisted + // `schema: null` reaches this. Indexing runs before the loop now, where the previous + // `schema.find` only ran inside it, so bail out here instead of throwing on iteration. + if (!schema?.length) { + return EMPTY_SCHEMA_INDEX; + } + let index = schemaIndexCache.get(schema); + if (!index) { + index = new Map(); + for (const entry of schema) { + // First occurrence wins, matching the `schema.find` this index replaces + if (!index.has(entry.id)) index.set(entry.id, entry); + } + schemaIndexCache.set(schema, index); + } + return index; +}; + const validateResourceMapperValue = ( parameterName: string, paramValues: { [key: string]: unknown }, @@ -35,13 +61,13 @@ const validateResourceMapperValue = ( if (!resourceMapperField || !isResourceMapperValue(resourceMapperField)) { return result; } - const schema = resourceMapperField.schema; + const schemaById = indexSchemaById(resourceMapperField.schema); const paramValueNames = Object.keys(paramValues); for (let i = 0; i < paramValueNames.length; i++) { const key = paramValueNames[i]; const resolvedValue = paramValues[key]; - const schemaEntry = schema.find((s) => s.id === key); + const schemaEntry = schemaById.get(key); if ( !skipRequiredCheck &&