Skip to content

feat: Update discriminator optimization when it has mixed fields #305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mohankumarelec wants to merge 1 commit into
base: main
Choose a base branch
from
+142 −10
Open

feat: Update discriminator optimization when it has mixed fields #305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions src/parser/jsonParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string | number, Map<any, number[]>>();

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) {
Expand All @@ -649,29 +652,43 @@ 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<string | number, Map<any, number[]>>, 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<number>();
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;
};

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) => {
Expand All @@ -682,7 +699,7 @@ 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;
return itemSchemas ? itemSchemas.map((item, idx) => [idx, asSchema(item)]) : null;
});
if (constMap) {
return findDiscriminator(constMap, (itemIndex) => {
Expand Down
115 changes: 115 additions & 0 deletions src/test/parser.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});

});