fix: migrate resourceLocator validation to schema-driven approach (#304, #306)

- Replace hardcoded ['list', 'id', 'url'] modes with schema-based validation
- Read allowed modes from prop.typeOptions.resourceLocator.modes
- Support both object and array mode definition formats
- Add Google Sheets range/columns flexibility for v4+ nodes
- Implement Set node JSON structure validation
- Update tests to verify schema-based validation

Fixes #304 (Google Sheets "name" mode false positive)
Fixes #306 (Set node validation gaps)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
czlonkowski
2025-10-11 18:10:47 +02:00
parent dc62fd66cb
commit 43dea68f0b
4 changed files with 106 additions and 16 deletions

View File

@@ -268,16 +268,36 @@ export class ConfigValidator {
type: 'invalid_type',
property: `${key}.mode`,
message: `resourceLocator '${key}.mode' must be a string, got ${typeof value.mode}`,
fix: `Set mode to "list" or "id"`
});
} else if (!['list', 'id', 'url'].includes(value.mode)) {
errors.push({
type: 'invalid_value',
property: `${key}.mode`,
message: `resourceLocator '${key}.mode' must be 'list', 'id', or 'url', got '${value.mode}'`,
fix: `Change mode to "list", "id", or "url"`
fix: `Set mode to a valid string value`
});
} else if (prop.typeOptions?.resourceLocator?.modes) {
// Schema-based validation: Check if mode exists in the modes definition
// Modes can be defined in different ways:
// 1. Object with mode keys: { list: {...}, id: {...}, url: {...}, name: {...} }
// 2. Array of mode objects: [{name: 'list', ...}, {name: 'id', ...}]
const modes = prop.typeOptions.resourceLocator.modes;
let allowedModes: string[] = [];
if (typeof modes === 'object' && !Array.isArray(modes)) {
// Extract keys from modes object
allowedModes = Object.keys(modes);
} else if (Array.isArray(modes)) {
// Extract name property from array of mode objects
allowedModes = modes.map(m => typeof m === 'string' ? m : m.name).filter(Boolean);
}
// Only validate if we found allowed modes
if (allowedModes.length > 0 && !allowedModes.includes(value.mode)) {
errors.push({
type: 'invalid_value',
property: `${key}.mode`,
message: `resourceLocator '${key}.mode' must be one of [${allowedModes.join(', ')}], got '${value.mode}'`,
fix: `Change mode to one of: ${allowedModes.join(', ')}`
});
}
}
// If no typeOptions.resourceLocator.modes defined, skip mode validation
// This prevents false positives for nodes with dynamic/runtime-determined modes
if (value.value === undefined) {
errors.push({

View File

@@ -318,7 +318,11 @@ export class EnhancedConfigValidator extends ConfigValidator {
case 'nodes-base.mysql':
NodeSpecificValidators.validateMySQL(context);
break;
case 'nodes-base.set':
NodeSpecificValidators.validateSet(context);
break;
case 'nodes-base.switch':
this.validateSwitchNodeStructure(config, result);
break;

View File

@@ -269,13 +269,15 @@ export class NodeSpecificValidators {
private static validateGoogleSheetsAppend(context: NodeValidationContext): void {
const { config, errors, warnings, autofix } = context;
if (!config.range) {
// In Google Sheets v4+, range is only required if NOT using the columns resourceMapper
// The columns parameter is a resourceMapper introduced in v4 that handles range automatically
if (!config.range && !config.columns) {
errors.push({
type: 'missing_required',
property: 'range',
message: 'Range is required for append operation',
fix: 'Specify range like "Sheet1!A:B" or "Sheet1!A1:B10"'
message: 'Range or columns mapping is required for append operation',
fix: 'Specify range like "Sheet1!A:B" OR use columns with mappingMode'
});
}
@@ -1556,4 +1558,59 @@ export class NodeSpecificValidators {
});
}
}
/**
* Validate Set node configuration
*/
static validateSet(context: NodeValidationContext): void {
const { config, errors, warnings } = context;
// Validate jsonOutput when present (used in JSON mode or when directly setting JSON)
if (config.jsonOutput !== undefined && config.jsonOutput !== null && config.jsonOutput !== '') {
try {
const parsed = JSON.parse(config.jsonOutput);
// Set node with JSON input expects an OBJECT {}, not an ARRAY []
// This is a common mistake that n8n UI catches but our validator should too
if (Array.isArray(parsed)) {
errors.push({
type: 'invalid_value',
property: 'jsonOutput',
message: 'Set node expects a JSON object {}, not an array []',
fix: 'Either wrap array items as object properties: {"items": [...]}, OR use a different approach for multiple items'
});
}
// Warn about empty objects
if (typeof parsed === 'object' && !Array.isArray(parsed) && Object.keys(parsed).length === 0) {
warnings.push({
type: 'inefficient',
property: 'jsonOutput',
message: 'jsonOutput is an empty object - this node will output no data',
suggestion: 'Add properties to the object or remove this node if not needed'
});
}
} catch (e) {
errors.push({
type: 'syntax_error',
property: 'jsonOutput',
message: `Invalid JSON in jsonOutput: ${e instanceof Error ? e.message : 'Syntax error'}`,
fix: 'Ensure jsonOutput contains valid JSON syntax'
});
}
}
// Validate mode-specific requirements
if (config.mode === 'manual') {
// In manual mode, at least one field should be defined
const hasFields = config.values && Object.keys(config.values).length > 0;
if (!hasFields && !config.jsonOutput) {
warnings.push({
type: 'missing_common',
message: 'Set node has no fields configured - will output empty items',
suggestion: 'Add fields in the Values section or use JSON mode'
});
}
}
}
}

View File

@@ -678,7 +678,7 @@ describe('ConfigValidator - Basic Validation', () => {
expect(result.errors[0].fix).toContain('{ mode: "id", value: "gpt-4o-mini" }');
});
it('should reject invalid mode values', () => {
it('should reject invalid mode values when schema defines allowed modes', () => {
const nodeType = '@n8n/n8n-nodes-langchain.lmChatOpenAi';
const config = {
model: {
@@ -690,7 +690,16 @@ describe('ConfigValidator - Basic Validation', () => {
{
name: 'model',
type: 'resourceLocator',
required: true
required: true,
typeOptions: {
resourceLocator: {
modes: {
list: { displayName: 'List' },
id: { displayName: 'ID' },
url: { displayName: 'URL' }
}
}
}
}
];
@@ -700,7 +709,7 @@ describe('ConfigValidator - Basic Validation', () => {
expect(result.errors.some(e =>
e.property === 'model.mode' &&
e.type === 'invalid_value' &&
e.message.includes("must be 'list', 'id', or 'url'")
e.message.includes('must be one of [list, id, url]')
)).toBe(true);
});