mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
perf(core): O(1) schema lookup in resource-mapper value validation (#34899)
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.18.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.18.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Co-authored-by: yoo-minho <13810291+yoo-minho@users.noreply.github.com>
This commit is contained in:
parent
c0a8f02f0f
commit
10a7422757
|
|
@ -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' });
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<ResourceMapperField[], Map<string, ResourceMapperField>>();
|
||||
|
||||
const EMPTY_SCHEMA_INDEX: ReadonlyMap<string, ResourceMapperField> = new Map();
|
||||
|
||||
const indexSchemaById = (
|
||||
schema: ResourceMapperField[] | undefined,
|
||||
): ReadonlyMap<string, ResourceMapperField> => {
|
||||
// `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<string, ResourceMapperField>();
|
||||
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 &&
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user