Skip to content

Commit 81a5f03

Browse files
SamyPesseSamyPesse
committed
Improve validation error for oneOf
1 parent 9497a83 commit 81a5f03

File tree

3 files changed

+112
-11
lines changed

3 files changed

+112
-11
lines changed

Diff for: ‎src/compileValueSchema.ts

+72-8
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,10 @@ function compileOneOfSchema(compiler: Compiler, schema: OpenAPIOneOfSchema) {
105105

106106
// Declare the variable to use as a result, then iterate over each schema
107107
const resultIdentifier = builders.identifier('result');
108+
const errorIdentifier = builders.identifier('error');
108109
nodes.push(
109110
builders.variableDeclaration('let', [builders.variableDeclarator(resultIdentifier)]),
111+
builders.variableDeclaration('let', [builders.variableDeclarator(errorIdentifier)]),
110112
);
111113

112114
schema.oneOf.forEach((subSchema, index) => {
@@ -125,14 +127,50 @@ function compileOneOfSchema(compiler: Compiler, schema: OpenAPIOneOfSchema) {
125127

126128
nodes.push(
127129
builders.ifStatement(
128-
builders.unaryExpression(
129-
'!',
130-
builders.binaryExpression(
131-
'instanceof',
132-
altIdentifier,
133-
ValidationErrorIdentifier,
134-
),
130+
builders.binaryExpression(
131+
'instanceof',
132+
altIdentifier,
133+
ValidationErrorIdentifier,
135134
),
135+
builders.blockStatement([
136+
// Repalce the error if the new error is more specific (longer path)
137+
builders.ifStatement(
138+
builders.logicalExpression(
139+
'||',
140+
builders.binaryExpression(
141+
'===',
142+
errorIdentifier,
143+
builders.identifier('undefined'),
144+
),
145+
builders.binaryExpression(
146+
'<',
147+
builders.memberExpression(
148+
builders.memberExpression(
149+
errorIdentifier,
150+
builders.identifier('path'),
151+
),
152+
builders.identifier('length'),
153+
),
154+
builders.memberExpression(
155+
builders.memberExpression(
156+
altIdentifier,
157+
builders.identifier('path'),
158+
),
159+
builders.identifier('length'),
160+
),
161+
),
162+
),
163+
builders.blockStatement([
164+
builders.expressionStatement(
165+
builders.assignmentExpression(
166+
'=',
167+
errorIdentifier,
168+
altIdentifier,
169+
),
170+
),
171+
]),
172+
),
173+
]),
136174
builders.blockStatement([
137175
...(index > 0
138176
? [
@@ -165,7 +203,33 @@ function compileOneOfSchema(compiler: Compiler, schema: OpenAPIOneOfSchema) {
165203
resultIdentifier,
166204
builders.identifier('undefined'),
167205
),
168-
builders.blockStatement([builders.returnStatement(error('expected to match one'))]),
206+
builders.blockStatement([
207+
// If the path is longer than one level deep, then we return the error from the evaluation
208+
// other we say that we expected to only match one of the schemas
209+
builders.ifStatement(
210+
builders.logicalExpression(
211+
'&&',
212+
errorIdentifier,
213+
builders.binaryExpression(
214+
'>',
215+
builders.memberExpression(
216+
builders.memberExpression(
217+
errorIdentifier,
218+
builders.identifier('path'),
219+
),
220+
builders.identifier('length'),
221+
),
222+
builders.binaryExpression(
223+
'+',
224+
builders.memberExpression(path, builders.identifier('length')),
225+
builders.literal(1),
226+
),
227+
),
228+
),
229+
builders.blockStatement([builders.returnStatement(errorIdentifier)]),
230+
builders.returnStatement(error('expected to match one')),
231+
),
232+
]),
169233
),
170234
);
171235

Diff for: ‎src/tests/__snapshots__/compileValueSchema.test.ts.snap

+15-3
Original file line numberDiff line numberDiff line change
@@ -1074,19 +1074,31 @@ function obj2(path, value, context) {
10741074
}
10751075
function obj0(path, value, context) {
10761076
let result;
1077+
let error;
10771078
const alt0 = obj1(path, value, context);
1078-
if (!(alt0 instanceof ValidationError)) {
1079+
if (alt0 instanceof ValidationError) {
1080+
if (error === undefined || error.path.length < alt0.path.length) {
1081+
error = alt0;
1082+
}
1083+
} else {
10791084
result = alt0;
10801085
}
10811086
const alt1 = obj2(path, value, context);
1082-
if (!(alt1 instanceof ValidationError)) {
1087+
if (alt1 instanceof ValidationError) {
1088+
if (error === undefined || error.path.length < alt1.path.length) {
1089+
error = alt1;
1090+
}
1091+
} else {
10831092
if (result !== undefined) {
10841093
return new ValidationError(path, 'expected to only match one of the schemas');
10851094
}
10861095
result = alt1;
10871096
}
10881097
if (result === undefined) {
1089-
return new ValidationError(path, 'expected to match one');
1098+
if (error && error.path.length > path.length + 1) {
1099+
return error;
1100+
} else
1101+
return new ValidationError(path, 'expected to match one');
10901102
}
10911103
return result;
10921104
}"

Diff for: ‎tests/gitbook.test.ts

+25
Original file line numberDiff line numberDiff line change
@@ -311,4 +311,29 @@ describe('componentSchemas', () => {
311311
'expected "build" to be defined',
312312
);
313313
});
314+
315+
test('should return a best-effort error for a oneOf', () => {
316+
const validate = componentSchemas['DocumentBlocksEssentials'];
317+
expect(validate).toBeInstanceOf(Function);
318+
319+
const error = validate([], {
320+
object: 'block',
321+
type: 'paragraph',
322+
nodes: [
323+
// Invalid
324+
{
325+
object: 'block',
326+
type: 'paragraph',
327+
nodes: [
328+
{
329+
object: 'text',
330+
text: 'Hello, world!',
331+
},
332+
],
333+
},
334+
],
335+
});
336+
337+
expect(error instanceof ValidationError ? error.path : null).toEqual(['nodes', 0]);
338+
});
314339
});

0 commit comments

Comments
 (0)