From cadbbd7a17d4b746636e3c36fa66a4d6568c51f7 Mon Sep 17 00:00:00 2001 From: Mohankumar Ramachandran Date: Mon, 26 Jan 2026 20:56:08 +0530 Subject: [PATCH] feat: Update discriminator optimization when it has mixed fields --- src/parser/jsonParser.ts | 42 ++++++++++---- src/test/parser.test.ts | 115 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 10 deletions(-) diff --git a/src/parser/jsonParser.ts b/src/parser/jsonParser.ts index ebd5f5f..a5a71c8 100644 --- a/src/parser/jsonParser.ts +++ b/src/parser/jsonParser.ts @@ -627,14 +627,17 @@ function validate(n: ASTNode | undefined, schema: JSONSchema, validationResult: return undefined; } - const buildConstMap = (getSchemas: (alt: JSONSchema, idx: number) => [string | number, JSONSchema][] | undefined) => { + const buildConstMap = (getSchemas: (alt: JSONSchema, idx: number) => [string | number, JSONSchema][] | undefined | null) => { const constMap = new Map>(); for (let i = 0; i < alternatives.length; i++) { const schemas = getSchemas(asSchema(alternatives[i]), i); - if (!schemas) { + if (schemas === undefined) { return undefined; // Early exit if any alternative can't be processed } + if (schemas === null) { + continue; // Skip alternatives that don't have schemas + } schemas.forEach(([key, schema]) => { if (schema.const !== undefined) { @@ -649,21 +652,35 @@ function validate(n: ASTNode | undefined, schema: JSONSchema, validationResult: } }); } - return constMap; + + // Only return the map if we found const values + return constMap.size > 0 ? constMap : undefined; }; const findDiscriminator = (constMap: Map>, getValue: (key: string | number) => any) => { + // If there are multiple discriminator keys, don't optimize + // This avoids issues with anyOf where different alternatives use different discriminator properties + if (constMap.size > 1) { + return undefined; + } + for (const [key, valueMap] of constMap) { const coveredAlts = new Set(); valueMap.forEach(indices => indices.forEach(idx => coveredAlts.add(idx))); - if (coveredAlts.size === alternatives.length) { + // Only optimize if discriminator covers at least 2 alternatives and it's worth it + // We don't require ALL alternatives to have discriminators (some might be primitives) + if (coveredAlts.size >= 2) { const discriminatorValue = getValue(key); - const matchingIndices = valueMap.get(discriminatorValue); - if (matchingIndices?.length) { - return matchingIndices.map(idx => alternatives[idx]); + if (discriminatorValue !== undefined) { + const matchingIndices = valueMap.get(discriminatorValue); + if (matchingIndices?.length) { + return matchingIndices.map(idx => alternatives[idx]); + } + // Discriminator value doesn't match any alternative with const + // Don't optimize - return undefined to test all alternatives + break; } - break; // Found valid discriminator but no match } } return undefined; @@ -671,7 +688,7 @@ function validate(n: ASTNode | undefined, schema: JSONSchema, validationResult: if (node.type === 'object' && node.properties?.length) { const constMap = buildConstMap((schema) => - schema.properties ? Object.entries(schema.properties).map(([k, v]) => [k, asSchema(v)]) : undefined + schema.properties ? Object.entries(schema.properties).map(([k, v]) => [k, asSchema(v)]) : null ); if (constMap) { return findDiscriminator(constMap, (propName) => { @@ -682,7 +699,12 @@ function validate(n: ASTNode | undefined, schema: JSONSchema, validationResult: } else if (node.type === 'array' && node.items?.length) { const constMap = buildConstMap((schema) => { const itemSchemas = schema.prefixItems || (Array.isArray(schema.items) ? schema.items : undefined); - return itemSchemas ? itemSchemas.map((item, idx) => [idx, asSchema(item)]) : undefined; + if (!itemSchemas || itemSchemas.length === 0) { + return null; + } + // Only check index 0 for discriminated unions + const firstItem = asSchema(itemSchemas[0]); + return firstItem && firstItem.const !== undefined ? [[0, firstItem]] : null; }); if (constMap) { return findDiscriminator(constMap, (itemIndex) => { diff --git a/src/test/parser.test.ts b/src/test/parser.test.ts index 2724c42..bd96154 100644 --- a/src/test/parser.test.ts +++ b/src/test/parser.test.ts @@ -3404,4 +3404,119 @@ suite('JSON Parser', () => { assert.strictEqual(res.length, 0); }); + test('discriminator optimization with deeply nested self-referencing schema', function () { + // Schema representing IExpression type with discriminated unions + // This tests performance with deep nesting (50-100 levels) + const schema: JSONSchema = { + $id: 'https://example.com/expression', + oneOf: [ + { type: 'string' }, + { type: 'number' }, + { type: 'boolean' }, + { type: 'null' }, + { + type: 'array', + prefixItems: [ + { const: 'logical.and' }, + { $ref: '#' }, + { $ref: '#' } + ], + items: { $ref: '#' }, + minItems: 3 + }, + { + type: 'array', + prefixItems: [ + { const: 'logical.or' }, + { $ref: '#' }, + { $ref: '#' } + ], + items: { $ref: '#' }, + minItems: 3 + }, + { + type: 'array', + prefixItems: [ + { const: 'logical.not' }, + { $ref: '#' } + ], + minItems: 2, + maxItems: 2 + }, + { + type: 'array', + prefixItems: [ + { const: 'compare.eq' }, + { $ref: '#' }, + { $ref: '#' } + ], + minItems: 3, + maxItems: 3 + } + ] + }; + { + // Simple expression + const { textDoc, jsonDoc } = toDocument('["logical.and", true, false]'); + const semanticErrors = validate2(jsonDoc, textDoc, schema, SchemaDraft.v2020_12); + assert.strictEqual(semanticErrors!.length, 0); + } + { + // Nested expression (5 levels) + const { textDoc, jsonDoc } = toDocument('["logical.or", ["logical.and", true, false], ["logical.not", true]]'); + const semanticErrors = validate2(jsonDoc, textDoc, schema, SchemaDraft.v2020_12); + assert.strictEqual(semanticErrors!.length, 0); + } + { + // Build a deeply nested expression (500 levels) + let deepExpression = 'true'; + for (let i = 0; i < 500; i++) { + deepExpression = `["logical.not", ${deepExpression}]`; + } + const { textDoc, jsonDoc } = toDocument(deepExpression); + const semanticErrors = validate2(jsonDoc, textDoc, schema, SchemaDraft.v2020_12); + assert.strictEqual(semanticErrors!.length, 0); + } + { + // Build an even deeper nested expression (1000 levels) + let veryDeepExpression = '42'; + for (let i = 0; i < 1000; i++) { + if (i % 2 === 0) { + veryDeepExpression = `["logical.not", ${veryDeepExpression}]`; + } else { + veryDeepExpression = `["logical.and", ${veryDeepExpression}, false]`; + } + } + const { textDoc, jsonDoc } = toDocument(veryDeepExpression); + const semanticErrors = validate2(jsonDoc, textDoc, schema, SchemaDraft.v2020_12); + assert.strictEqual(semanticErrors!.length, 0); + } + { + // Invalid - unknown operator (should fail quickly with discriminator optimization) + const { textDoc, jsonDoc } = toDocument('["unknown.operator", true, false]'); + const semanticErrors = validate2(jsonDoc, textDoc, schema, SchemaDraft.v2020_12); + assert.ok(semanticErrors!.length > 0); + } + { + // Invalid - wrong number of arguments for logical.not + const { textDoc, jsonDoc } = toDocument('["logical.not", true, false]'); + const semanticErrors = validate2(jsonDoc, textDoc, schema, SchemaDraft.v2020_12); + assert.ok(semanticErrors!.length > 0); + } + { + // Invalid - deeply nested with error (wrong array structure) + let deepInvalid = '42'; + for (let i = 0; i < 200; i++) { + deepInvalid = `["logical.not", ${deepInvalid}]`; + } + // Invalid: logical.and needs at least 2 arguments after the operator, but we provide wrong structure + deepInvalid = `["logical.and", ${deepInvalid}]`; // Missing second required argument + + const { textDoc, jsonDoc } = toDocument(deepInvalid); + const semanticErrors = validate2(jsonDoc, textDoc, schema, SchemaDraft.v2020_12); + // Should detect the validation error (not enough items) + assert.ok(semanticErrors!.length > 0); + } + }); + });