diff --git a/data/nodes.db b/data/nodes.db index cb821a5..69be8a7 100644 Binary files a/data/nodes.db and b/data/nodes.db differ diff --git a/src/services/enhanced-config-validator.ts b/src/services/enhanced-config-validator.ts index 71ecf68..0e66629 100644 --- a/src/services/enhanced-config-validator.ts +++ b/src/services/enhanced-config-validator.ts @@ -7,6 +7,7 @@ import { ConfigValidator, ValidationResult, ValidationError, ValidationWarning } from './config-validator'; import { NodeSpecificValidators, NodeValidationContext } from './node-specific-validators'; +import { FixedCollectionValidator } from '../utils/fixed-collection-validator'; export type ValidationMode = 'full' | 'operation' | 'minimal'; export type ValidationProfile = 'strict' | 'runtime' | 'ai-friendly' | 'minimal'; @@ -256,6 +257,9 @@ export class EnhancedConfigValidator extends ConfigValidator { case 'nodes-base.filter': this.validateFilterNodeStructure(config, result); break; + + // Additional nodes handled by FixedCollectionValidator + // No need for specific validators as the generic utility handles them } // Update autofix if changes were made @@ -499,109 +503,44 @@ export class EnhancedConfigValidator extends ConfigValidator { config: Record, result: EnhancedValidationResult ): void { - // Normalize node type (handle both 'n8n-nodes-base.x' and 'nodes-base.x' formats) - const normalizedNodeType = nodeType.replace('n8n-nodes-base.', 'nodes-base.'); + // Use the generic FixedCollectionValidator + const validationResult = FixedCollectionValidator.validate(nodeType, config); - // Define nodes and their problematic patterns - const problematicNodes = { - 'nodes-base.switch': { - property: 'rules', - expectedStructure: 'rules.values array', - invalidPatterns: ['rules.conditions', 'rules.conditions.values'] - }, - 'nodes-base.if': { - property: 'conditions', - expectedStructure: 'conditions array/object', - invalidPatterns: ['conditions.values'] - }, - 'nodes-base.filter': { - property: 'conditions', - expectedStructure: 'conditions array/object', - invalidPatterns: ['conditions.values'] - } - }; - - const nodeConfig = problematicNodes[normalizedNodeType as keyof typeof problematicNodes]; - if (!nodeConfig) return; - - const propertyValue = config[nodeConfig.property]; - if (!propertyValue || typeof propertyValue !== 'object') return; - - // Check for incorrect nesting patterns - for (const pattern of nodeConfig.invalidPatterns) { - const parts = pattern.split('.'); - let current = config; - let isInvalid = true; - - for (const part of parts) { - if (!current || typeof current !== 'object' || !current[part]) { - isInvalid = false; - break; - } - current = current[part]; - } - - if (isInvalid) { + if (!validationResult.isValid) { + // Add errors to the result + for (const error of validationResult.errors) { result.errors.push({ type: 'invalid_value', - property: nodeConfig.property, - message: `Invalid structure for ${normalizedNodeType} node: found nested "${pattern}" but expected "${nodeConfig.expectedStructure}". This causes "propertyValues[itemName] is not iterable" error in n8n.`, - fix: this.generateFixedCollectionFix(normalizedNodeType, pattern, nodeConfig.expectedStructure) + property: error.pattern.split('.')[0], // Get the root property + message: error.message, + fix: error.fix }); - - // Provide auto-fix suggestion - if (!result.autofix) result.autofix = {}; - result.autofix[nodeConfig.property] = this.generateFixedCollectionAutofix(normalizedNodeType, config[nodeConfig.property]); + } + + // Apply autofix if available + if (validationResult.autofix) { + // For nodes like If/Filter where the entire config might be replaced, + // we need to handle it specially + if (typeof validationResult.autofix === 'object' && !Array.isArray(validationResult.autofix)) { + result.autofix = { + ...result.autofix, + ...validationResult.autofix + }; + } else { + // If the autofix is an array (like for If/Filter nodes), wrap it properly + const firstError = validationResult.errors[0]; + if (firstError) { + const rootProperty = firstError.pattern.split('.')[0]; + result.autofix = { + ...result.autofix, + [rootProperty]: validationResult.autofix + }; + } + } } } } - /** - * Generate fix message for fixedCollection errors - */ - private static generateFixedCollectionFix(nodeType: string, invalidPattern: string, expectedStructure: string): string { - switch (nodeType) { - case 'nodes-base.switch': - return 'Use: { "rules": { "values": [{ "conditions": {...}, "outputKey": "output1" }] } }'; - case 'nodes-base.if': - case 'nodes-base.filter': - return 'Use: { "conditions": {...} } or { "conditions": [...] } directly, not nested under "values"'; - default: - return `Use ${expectedStructure} instead of ${invalidPattern}`; - } - } - - /** - * Generate auto-fix for fixedCollection structures - */ - private static generateFixedCollectionAutofix(nodeType: string, invalidValue: any): any { - switch (nodeType) { - case 'nodes-base.switch': - // If it has rules.conditions.values, convert to rules.values - if (invalidValue.conditions?.values) { - return { - values: Array.isArray(invalidValue.conditions.values) - ? invalidValue.conditions.values.map((condition: any, index: number) => ({ - conditions: condition, - outputKey: `output${index + 1}` - })) - : [{ - conditions: invalidValue.conditions.values, - outputKey: 'output1' - }] - }; - } - break; - case 'nodes-base.if': - case 'nodes-base.filter': - // If it has conditions.values, extract the values - if (invalidValue.values) { - return invalidValue.values; - } - break; - } - return invalidValue; - } /** * Validate Switch node structure specifically diff --git a/src/utils/fixed-collection-validator.ts b/src/utils/fixed-collection-validator.ts new file mode 100644 index 0000000..5e9451f --- /dev/null +++ b/src/utils/fixed-collection-validator.ts @@ -0,0 +1,355 @@ +/** + * Generic utility for validating and fixing fixedCollection structures in n8n nodes + * Prevents the "propertyValues[itemName] is not iterable" error + */ + +export interface FixedCollectionPattern { + nodeType: string; + property: string; + subProperty?: string; + expectedStructure: string; + invalidPatterns: string[]; +} + +export interface FixedCollectionValidationResult { + isValid: boolean; + errors: Array<{ + pattern: string; + message: string; + fix: string; + }>; + autofix?: any; +} + +export class FixedCollectionValidator { + /** + * Known problematic patterns for various n8n nodes + */ + private static readonly KNOWN_PATTERNS: FixedCollectionPattern[] = [ + // Conditional nodes (already fixed) + { + nodeType: 'switch', + property: 'rules', + expectedStructure: 'rules.values array', + invalidPatterns: ['rules.conditions', 'rules.conditions.values'] + }, + { + nodeType: 'if', + property: 'conditions', + expectedStructure: 'conditions array/object', + invalidPatterns: ['conditions.values'] + }, + { + nodeType: 'filter', + property: 'conditions', + expectedStructure: 'conditions array/object', + invalidPatterns: ['conditions.values'] + }, + // New nodes identified by research + { + nodeType: 'summarize', + property: 'fieldsToSummarize', + subProperty: 'values', + expectedStructure: 'fieldsToSummarize.values array', + invalidPatterns: ['fieldsToSummarize.values.values'] + }, + { + nodeType: 'comparedatasets', + property: 'mergeByFields', + subProperty: 'values', + expectedStructure: 'mergeByFields.values array', + invalidPatterns: ['mergeByFields.values.values'] + }, + { + nodeType: 'sort', + property: 'sortFieldsUi', + subProperty: 'sortField', + expectedStructure: 'sortFieldsUi.sortField array', + invalidPatterns: ['sortFieldsUi.sortField.values'] + }, + { + nodeType: 'aggregate', + property: 'fieldsToAggregate', + subProperty: 'fieldToAggregate', + expectedStructure: 'fieldsToAggregate.fieldToAggregate array', + invalidPatterns: ['fieldsToAggregate.fieldToAggregate.values'] + }, + { + nodeType: 'set', + property: 'fields', + subProperty: 'values', + expectedStructure: 'fields.values array', + invalidPatterns: ['fields.values.values'] + }, + { + nodeType: 'html', + property: 'extractionValues', + subProperty: 'values', + expectedStructure: 'extractionValues.values array', + invalidPatterns: ['extractionValues.values.values'] + }, + { + nodeType: 'httprequest', + property: 'body', + subProperty: 'parameters', + expectedStructure: 'body.parameters array', + invalidPatterns: ['body.parameters.values'] + }, + { + nodeType: 'airtable', + property: 'sort', + subProperty: 'sortField', + expectedStructure: 'sort.sortField array', + invalidPatterns: ['sort.sortField.values'] + } + ]; + + /** + * Validate a node configuration for fixedCollection issues + */ + static validate( + nodeType: string, + config: Record + ): FixedCollectionValidationResult { + const normalizedNodeType = this.normalizeNodeType(nodeType); + const pattern = this.getPatternForNode(normalizedNodeType); + + if (!pattern) { + return { isValid: true, errors: [] }; + } + + const result: FixedCollectionValidationResult = { + isValid: true, + errors: [] + }; + + // Check for invalid patterns + for (const invalidPattern of pattern.invalidPatterns) { + if (this.hasInvalidStructure(config, invalidPattern)) { + result.isValid = false; + result.errors.push({ + pattern: invalidPattern, + message: `Invalid structure for nodes-base.${pattern.nodeType} node: found nested "${invalidPattern}" but expected "${pattern.expectedStructure}". This causes "propertyValues[itemName] is not iterable" error in n8n.`, + fix: this.generateFixMessage(pattern) + }); + + // Generate autofix + if (!result.autofix) { + result.autofix = this.generateAutofix(config, pattern); + } + } + } + + return result; + } + + /** + * Apply autofix to a configuration + */ + static applyAutofix( + config: Record, + pattern: FixedCollectionPattern + ): Record { + const fixedConfig = this.generateAutofix(config, pattern); + // For If/Filter nodes, the autofix might return just the values array + if (pattern.nodeType === 'if' || pattern.nodeType === 'filter') { + if (config.conditions?.values) { + return config.conditions.values; + } + } + return fixedConfig; + } + + /** + * Normalize node type to handle various formats + */ + private static normalizeNodeType(nodeType: string): string { + return nodeType + .replace('n8n-nodes-base.', '') + .replace('nodes-base.', '') + .replace('@n8n/n8n-nodes-langchain.', '') + .toLowerCase(); + } + + /** + * Get pattern configuration for a specific node type + */ + private static getPatternForNode(nodeType: string): FixedCollectionPattern | undefined { + return this.KNOWN_PATTERNS.find(p => p.nodeType === nodeType); + } + + /** + * Check if configuration has an invalid structure + */ + private static hasInvalidStructure( + config: Record, + pattern: string + ): boolean { + const parts = pattern.split('.'); + let current = config; + + for (const part of parts) { + if (!current || typeof current !== 'object' || !current[part]) { + return false; + } + current = current[part]; + } + + return true; + } + + /** + * Generate a fix message for the specific pattern + */ + private static generateFixMessage(pattern: FixedCollectionPattern): string { + switch (pattern.nodeType) { + case 'switch': + return 'Use: { "rules": { "values": [{ "conditions": {...}, "outputKey": "output1" }] } }'; + case 'if': + case 'filter': + return 'Use: { "conditions": {...} } or { "conditions": [...] } directly, not nested under "values"'; + case 'summarize': + return 'Use: { "fieldsToSummarize": { "values": [...] } } not nested values.values'; + case 'comparedatasets': + return 'Use: { "mergeByFields": { "values": [...] } } not nested values.values'; + case 'sort': + return 'Use: { "sortFieldsUi": { "sortField": [...] } } not sortField.values'; + case 'aggregate': + return 'Use: { "fieldsToAggregate": { "fieldToAggregate": [...] } } not fieldToAggregate.values'; + case 'set': + return 'Use: { "fields": { "values": [...] } } not nested values.values'; + case 'html': + return 'Use: { "extractionValues": { "values": [...] } } not nested values.values'; + case 'httprequest': + return 'Use: { "body": { "parameters": [...] } } not parameters.values'; + case 'airtable': + return 'Use: { "sort": { "sortField": [...] } } not sortField.values'; + default: + return `Use ${pattern.expectedStructure} structure`; + } + } + + /** + * Generate autofix for invalid structures + */ + private static generateAutofix( + config: Record, + pattern: FixedCollectionPattern + ): any { + const fixedConfig = { ...config }; + + switch (pattern.nodeType) { + case 'switch': + if (config.rules?.conditions?.values) { + fixedConfig.rules = { + values: Array.isArray(config.rules.conditions.values) + ? config.rules.conditions.values.map((condition: any, index: number) => ({ + conditions: condition, + outputKey: `output${index + 1}` + })) + : [{ + conditions: config.rules.conditions.values, + outputKey: 'output1' + }] + }; + } else if (config.rules?.conditions) { + fixedConfig.rules = { + values: [{ + conditions: config.rules.conditions, + outputKey: 'output1' + }] + }; + } + break; + + case 'if': + case 'filter': + if (config.conditions?.values) { + return config.conditions.values; + } + break; + + case 'summarize': + if (config.fieldsToSummarize?.values?.values) { + fixedConfig.fieldsToSummarize = { + values: config.fieldsToSummarize.values.values + }; + } + break; + + case 'comparedatasets': + if (config.mergeByFields?.values?.values) { + fixedConfig.mergeByFields = { + values: config.mergeByFields.values.values + }; + } + break; + + case 'sort': + if (config.sortFieldsUi?.sortField?.values) { + fixedConfig.sortFieldsUi = { + sortField: config.sortFieldsUi.sortField.values + }; + } + break; + + case 'aggregate': + if (config.fieldsToAggregate?.fieldToAggregate?.values) { + fixedConfig.fieldsToAggregate = { + fieldToAggregate: config.fieldsToAggregate.fieldToAggregate.values + }; + } + break; + + case 'set': + if (config.fields?.values?.values) { + fixedConfig.fields = { + values: config.fields.values.values + }; + } + break; + + case 'html': + if (config.extractionValues?.values?.values) { + fixedConfig.extractionValues = { + values: config.extractionValues.values.values + }; + } + break; + + case 'httprequest': + if (config.body?.parameters?.values) { + fixedConfig.body = { + ...config.body, + parameters: config.body.parameters.values + }; + } + break; + + case 'airtable': + if (config.sort?.sortField?.values) { + fixedConfig.sort = { + sortField: config.sort.sortField.values + }; + } + break; + } + + return fixedConfig; + } + + /** + * Get all known patterns (for testing and documentation) + */ + static getAllPatterns(): FixedCollectionPattern[] { + return [...this.KNOWN_PATTERNS]; + } + + /** + * Check if a node type is susceptible to fixedCollection issues + */ + static isNodeSusceptible(nodeType: string): boolean { + const normalizedType = this.normalizeNodeType(nodeType); + return this.KNOWN_PATTERNS.some(p => p.nodeType === normalizedType); + } +} \ No newline at end of file diff --git a/tests/unit/utils/fixed-collection-validator.test.ts b/tests/unit/utils/fixed-collection-validator.test.ts new file mode 100644 index 0000000..0a466f7 --- /dev/null +++ b/tests/unit/utils/fixed-collection-validator.test.ts @@ -0,0 +1,336 @@ +import { describe, test, expect } from 'vitest'; +import { FixedCollectionValidator } from '../../../src/utils/fixed-collection-validator'; + +describe('FixedCollectionValidator', () => { + describe('Core Functionality', () => { + test('should return valid for non-susceptible nodes', () => { + const result = FixedCollectionValidator.validate('n8n-nodes-base.cron', { + triggerTimes: { hour: 10, minute: 30 } + }); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + test('should normalize node types correctly', () => { + const nodeTypes = [ + 'n8n-nodes-base.switch', + 'nodes-base.switch', + '@n8n/n8n-nodes-langchain.switch', + 'SWITCH' + ]; + + nodeTypes.forEach(nodeType => { + expect(FixedCollectionValidator.isNodeSusceptible(nodeType)).toBe(true); + }); + }); + + test('should get all known patterns', () => { + const patterns = FixedCollectionValidator.getAllPatterns(); + expect(patterns.length).toBeGreaterThan(10); // We have at least 11 patterns + expect(patterns.some(p => p.nodeType === 'switch')).toBe(true); + expect(patterns.some(p => p.nodeType === 'summarize')).toBe(true); + }); + }); + + describe('Switch Node Validation', () => { + test('should detect invalid nested conditions structure', () => { + const invalidConfig = { + rules: { + conditions: { + values: [ + { + value1: '={{$json.status}}', + operation: 'equals', + value2: 'active' + } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('n8n-nodes-base.switch', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors).toHaveLength(2); // Both rules.conditions and rules.conditions.values match + // Check that we found the specific pattern + const conditionsValuesError = result.errors.find(e => e.pattern === 'rules.conditions.values'); + expect(conditionsValuesError).toBeDefined(); + expect(conditionsValuesError!.message).toContain('propertyValues[itemName] is not iterable'); + expect(result.autofix).toBeDefined(); + expect(result.autofix.rules.values).toBeDefined(); + expect(result.autofix.rules.values[0].outputKey).toBe('output1'); + }); + + test('should provide correct autofix for switch node', () => { + const invalidConfig = { + rules: { + conditions: { + values: [ + { value1: '={{$json.a}}', operation: 'equals', value2: '1' }, + { value1: '={{$json.b}}', operation: 'equals', value2: '2' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('switch', invalidConfig); + + expect(result.autofix.rules.values).toHaveLength(2); + expect(result.autofix.rules.values[0].outputKey).toBe('output1'); + expect(result.autofix.rules.values[1].outputKey).toBe('output2'); + }); + }); + + describe('If/Filter Node Validation', () => { + test('should detect invalid nested values structure', () => { + const invalidConfig = { + conditions: { + values: [ + { + value1: '={{$json.age}}', + operation: 'largerEqual', + value2: 18 + } + ] + } + }; + + const ifResult = FixedCollectionValidator.validate('n8n-nodes-base.if', invalidConfig); + const filterResult = FixedCollectionValidator.validate('n8n-nodes-base.filter', invalidConfig); + + expect(ifResult.isValid).toBe(false); + expect(ifResult.errors[0].fix).toContain('directly, not nested under "values"'); + expect(ifResult.autofix).toEqual([ + { + value1: '={{$json.age}}', + operation: 'largerEqual', + value2: 18 + } + ]); + + expect(filterResult.isValid).toBe(false); + expect(filterResult.autofix).toEqual(ifResult.autofix); + }); + }); + + describe('New Nodes Validation', () => { + test('should validate Summarize node', () => { + const invalidConfig = { + fieldsToSummarize: { + values: { + values: [ + { field: 'amount', aggregation: 'sum' }, + { field: 'count', aggregation: 'count' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('summarize', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('fieldsToSummarize.values.values'); + expect(result.errors[0].fix).toContain('not nested values.values'); + expect(result.autofix.fieldsToSummarize.values).toHaveLength(2); + }); + + test('should validate Compare Datasets node', () => { + const invalidConfig = { + mergeByFields: { + values: { + values: [ + { field1: 'id', field2: 'userId' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('compareDatasets', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('mergeByFields.values.values'); + expect(result.autofix.mergeByFields.values).toHaveLength(1); + }); + + test('should validate Sort node', () => { + const invalidConfig = { + sortFieldsUi: { + sortField: { + values: [ + { fieldName: 'date', order: 'descending' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('sort', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('sortFieldsUi.sortField.values'); + expect(result.errors[0].fix).toContain('not sortField.values'); + expect(result.autofix.sortFieldsUi.sortField).toHaveLength(1); + }); + + test('should validate Aggregate node', () => { + const invalidConfig = { + fieldsToAggregate: { + fieldToAggregate: { + values: [ + { fieldToAggregate: 'price', aggregation: 'average' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('aggregate', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('fieldsToAggregate.fieldToAggregate.values'); + expect(result.autofix.fieldsToAggregate.fieldToAggregate).toHaveLength(1); + }); + + test('should validate Set node', () => { + const invalidConfig = { + fields: { + values: { + values: [ + { name: 'status', value: 'active' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('set', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('fields.values.values'); + expect(result.autofix.fields.values).toHaveLength(1); + }); + + test('should validate HTML node', () => { + const invalidConfig = { + extractionValues: { + values: { + values: [ + { key: 'title', cssSelector: 'h1' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('html', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('extractionValues.values.values'); + expect(result.autofix.extractionValues.values).toHaveLength(1); + }); + + test('should validate HTTP Request node', () => { + const invalidConfig = { + body: { + parameters: { + values: [ + { name: 'api_key', value: '123' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('httpRequest', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('body.parameters.values'); + expect(result.errors[0].fix).toContain('not parameters.values'); + expect(result.autofix.body.parameters).toHaveLength(1); + }); + + test('should validate Airtable node', () => { + const invalidConfig = { + sort: { + sortField: { + values: [ + { fieldName: 'Created', direction: 'desc' } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('airtable', invalidConfig); + + expect(result.isValid).toBe(false); + expect(result.errors[0].pattern).toBe('sort.sortField.values'); + expect(result.autofix.sort.sortField).toHaveLength(1); + }); + }); + + describe('Edge Cases', () => { + test('should handle empty config', () => { + const result = FixedCollectionValidator.validate('switch', {}); + expect(result.isValid).toBe(true); + }); + + test('should handle null/undefined properties', () => { + const result = FixedCollectionValidator.validate('switch', { + rules: null + }); + expect(result.isValid).toBe(true); + }); + + test('should handle valid structures', () => { + const validSwitch = { + rules: { + values: [ + { + conditions: { value1: '={{$json.x}}', operation: 'equals', value2: 1 }, + outputKey: 'output1' + } + ] + } + }; + + const result = FixedCollectionValidator.validate('switch', validSwitch); + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + test('should handle deeply nested invalid structures', () => { + const deeplyNested = { + rules: { + conditions: { + values: [ + { + value1: '={{$json.deep}}', + operation: 'equals', + value2: 'nested' + } + ] + } + } + }; + + const result = FixedCollectionValidator.validate('switch', deeplyNested); + expect(result.isValid).toBe(false); + expect(result.errors).toHaveLength(2); // Both patterns match + }); + }); + + describe('applyAutofix Method', () => { + test('should apply autofix correctly', () => { + const invalidConfig = { + conditions: { + values: [ + { value1: '={{$json.test}}', operation: 'equals', value2: 'yes' } + ] + } + }; + + const pattern = FixedCollectionValidator.getAllPatterns().find(p => p.nodeType === 'if'); + const fixed = FixedCollectionValidator.applyAutofix(invalidConfig, pattern!); + + expect(fixed).toEqual([ + { value1: '={{$json.test}}', operation: 'equals', value2: 'yes' } + ]); + }); + }); +}); \ No newline at end of file