Skip to content

Commit 4c28a6a

Browse files
committed
fix: include AJV error params and multiple violations in schema details
Signed-off-by: Alessandro Yuichi Okimoto <yuichijpn@gmail.com>
1 parent 679898a commit 4c28a6a

3 files changed

Lines changed: 276 additions & 4 deletions

File tree

ui/dashboard/src/pages/feature-flag-details/variation/schema-section/schema-dialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ const SchemaDialog = ({ isOpen, feature, onClose }: SchemaDialogProps) => {
308308
</span>
309309
</div>
310310
{!result.passed && result.detail && (
311-
<span className="truncate max-w-[500px] text-gray-500">
311+
<span className="max-w-[500px] break-words text-gray-500">
312312
{result.detail}
313313
</span>
314314
)}
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { VariationValueSchema } from '@types';
3+
import {
4+
createValueValidator,
5+
getSupportedSchemaTypes,
6+
isSchemaSupported,
7+
validateSchemaDefinition
8+
} from './variation-value-schema';
9+
10+
const enumSchema = (values: string[]): VariationValueSchema => ({
11+
type: 'ENUM',
12+
enumValidator: { values }
13+
});
14+
15+
const regexSchema = (pattern: string): VariationValueSchema => ({
16+
type: 'REGEX',
17+
regexValidator: { pattern }
18+
});
19+
20+
const jsonSchema = (schema: object): VariationValueSchema => ({
21+
type: 'JSON_SCHEMA',
22+
jsonSchemaValidator: { schema: JSON.stringify(schema) }
23+
});
24+
25+
describe('getSupportedSchemaTypes / isSchemaSupported', () => {
26+
it('matches the backend v1 type matrix', () => {
27+
expect(getSupportedSchemaTypes('STRING')).toEqual(['ENUM', 'REGEX']);
28+
expect(getSupportedSchemaTypes('NUMBER')).toEqual(['ENUM']);
29+
expect(getSupportedSchemaTypes('JSON')).toEqual(['JSON_SCHEMA']);
30+
expect(getSupportedSchemaTypes('BOOLEAN')).toEqual([]);
31+
expect(getSupportedSchemaTypes('YAML')).toEqual([]);
32+
});
33+
34+
it('reports support only for types with at least one validator', () => {
35+
expect(isSchemaSupported('STRING')).toBe(true);
36+
expect(isSchemaSupported('BOOLEAN')).toBe(false);
37+
});
38+
});
39+
40+
describe('validateSchemaDefinition', () => {
41+
it('rejects schema types unsupported for the variation type', () => {
42+
expect(validateSchemaDefinition(regexSchema('a+'), 'NUMBER')).toBe(
43+
'type-unsupported'
44+
);
45+
});
46+
47+
it('rejects empty enums', () => {
48+
expect(validateSchemaDefinition(enumSchema([]), 'STRING')).toBe(
49+
'enum-empty'
50+
);
51+
});
52+
53+
it('accepts plain decimal enum values for NUMBER flags', () => {
54+
expect(
55+
validateSchemaDefinition(enumSchema(['1', '-1.5', '1e3', '.5']), 'NUMBER')
56+
).toBeNull();
57+
});
58+
59+
it('rejects non-decimal syntaxes the backend parser rejects', () => {
60+
for (const value of ['0x10', '0b10', 'Infinity', ' 1', '']) {
61+
expect(validateSchemaDefinition(enumSchema([value]), 'NUMBER')).toBe(
62+
'enum-not-number'
63+
);
64+
}
65+
});
66+
67+
it('rejects empty regex patterns', () => {
68+
expect(validateSchemaDefinition(regexSchema(''), 'STRING')).toBe(
69+
'regex-empty'
70+
);
71+
});
72+
73+
it('accepts Go-only RE2 constructs like inline flags', () => {
74+
expect(
75+
validateSchemaDefinition(regexSchema('(?i)^a+$'), 'STRING')
76+
).toBeNull();
77+
});
78+
79+
it('rejects Perl-only constructs like lookaheads, matching Go RE2', () => {
80+
expect(validateSchemaDefinition(regexSchema('(?=a)'), 'STRING')).toBe(
81+
'regex-invalid'
82+
);
83+
});
84+
85+
it('rejects empty and malformed JSON Schemas', () => {
86+
expect(
87+
validateSchemaDefinition(
88+
{ type: 'JSON_SCHEMA', jsonSchemaValidator: { schema: ' ' } },
89+
'JSON'
90+
)
91+
).toBe('json-schema-empty');
92+
expect(
93+
validateSchemaDefinition(
94+
{ type: 'JSON_SCHEMA', jsonSchemaValidator: { schema: '{ not json' } },
95+
'JSON'
96+
)
97+
).toBe('json-schema-invalid');
98+
});
99+
100+
it('accepts a valid JSON Schema', () => {
101+
expect(
102+
validateSchemaDefinition(jsonSchema({ type: 'object' }), 'JSON')
103+
).toBeNull();
104+
});
105+
});
106+
107+
describe('createValueValidator: ENUM', () => {
108+
it('validates string values by exact match', () => {
109+
const validate = createValueValidator(
110+
enumSchema(['ssh', 'email']),
111+
'STRING'
112+
)!;
113+
expect(validate('ssh').valid).toBe(true);
114+
expect(validate('SSH').valid).toBe(false);
115+
expect(validate('').valid).toBe(false);
116+
});
117+
118+
it('validates number values numerically but rejects non-decimal syntax', () => {
119+
const validate = createValueValidator(enumSchema(['16', '1.5']), 'NUMBER')!;
120+
expect(validate('16').valid).toBe(true);
121+
expect(validate('16.0').valid).toBe(true);
122+
// Number('0x10') === 16, but the backend's strconv.ParseFloat rejects it.
123+
expect(validate('0x10').valid).toBe(false);
124+
expect(validate('2').valid).toBe(false);
125+
});
126+
});
127+
128+
describe('createValueValidator: REGEX', () => {
129+
it('returns null for patterns that do not compile as RE2', () => {
130+
expect(createValueValidator(regexSchema('(?=a)'), 'STRING')).toBeNull();
131+
});
132+
133+
it('supports Go-only inline flags', () => {
134+
const validate = createValueValidator(regexSchema('(?i)^ssh$'), 'STRING')!;
135+
expect(validate('SSH').valid).toBe(true);
136+
expect(validate('email').valid).toBe(false);
137+
});
138+
139+
it('matches unanchored, mirroring the backend regexp.MatchString', () => {
140+
const validate = createValueValidator(regexSchema('b+'), 'STRING')!;
141+
expect(validate('abc').valid).toBe(true);
142+
expect(validate('ac').valid).toBe(false);
143+
});
144+
});
145+
146+
describe('createValueValidator: JSON_SCHEMA', () => {
147+
const schema = jsonSchema({
148+
type: 'object',
149+
required: ['name'],
150+
additionalProperties: false,
151+
properties: {
152+
name: { type: 'string' },
153+
theme: { enum: ['light', 'dark'] }
154+
}
155+
});
156+
157+
it('returns null when the schema itself does not compile', () => {
158+
const invalid: VariationValueSchema = {
159+
type: 'JSON_SCHEMA',
160+
jsonSchemaValidator: { schema: '{ not json' }
161+
};
162+
expect(createValueValidator(invalid, 'JSON')).toBeNull();
163+
});
164+
165+
it('accepts a conforming value without detail', () => {
166+
const validate = createValueValidator(schema, 'JSON')!;
167+
expect(validate('{"name":"a","theme":"dark"}')).toEqual({ valid: true });
168+
});
169+
170+
it('reports the failing path for missing required properties', () => {
171+
const validate = createValueValidator(schema, 'JSON')!;
172+
const result = validate('{}');
173+
expect(result.valid).toBe(false);
174+
expect(result.detail).toBe("/ must have required property 'name'");
175+
});
176+
177+
it('names the offending additional property', () => {
178+
const validate = createValueValidator(schema, 'JSON')!;
179+
const result = validate('{"name":"a","extraField":"x"}');
180+
expect(result.valid).toBe(false);
181+
expect(result.detail).toContain(
182+
"must NOT have additional properties ('extraField')"
183+
);
184+
});
185+
186+
it('lists the allowed values for enum violations', () => {
187+
const validate = createValueValidator(schema, 'JSON')!;
188+
const result = validate('{"name":"a","theme":"blue"}');
189+
expect(result.valid).toBe(false);
190+
expect(result.detail).toContain(
191+
'/theme must be equal to one of the allowed values'
192+
);
193+
expect(result.detail).toContain('"light", "dark"');
194+
});
195+
196+
it('truncates long enum allowed-value lists', () => {
197+
const manyValues = jsonSchema({
198+
type: 'object',
199+
properties: { size: { enum: ['a', 'b', 'c', 'd', 'e', 'f', 'g'] } }
200+
});
201+
const validate = createValueValidator(manyValues, 'JSON')!;
202+
const result = validate('{"size":"z"}');
203+
expect(result.valid).toBe(false);
204+
expect(result.detail).toContain('"a", "b", "c", "d", "e", +2 more');
205+
});
206+
207+
it('reports multiple violations, capped with a remainder count', () => {
208+
const multi = jsonSchema({
209+
type: 'object',
210+
required: ['a', 'b', 'c', 'd'],
211+
properties: {
212+
a: { type: 'string' },
213+
b: { type: 'string' },
214+
c: { type: 'string' },
215+
d: { type: 'string' }
216+
}
217+
});
218+
const validate = createValueValidator(multi, 'JSON')!;
219+
const result = validate('{}');
220+
expect(result.valid).toBe(false);
221+
expect(result.detail).toBe(
222+
"/ must have required property 'a'; " +
223+
"/ must have required property 'b'; " +
224+
"/ must have required property 'c' (+1 more)"
225+
);
226+
});
227+
228+
it('reports values that are not valid JSON', () => {
229+
const validate = createValueValidator(schema, 'JSON')!;
230+
expect(validate('not json')).toEqual({
231+
valid: false,
232+
detail: 'invalid JSON'
233+
});
234+
});
235+
});

ui/dashboard/src/utils/variation-value-schema.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,49 @@ export interface ValueValidationResult {
113113
detail?: string;
114114
}
115115

116+
// Caps keep the detail usable in a single-line inline form error; a badly
117+
// broken document can produce dozens of AJV errors.
118+
const MAX_DETAIL_ERRORS = 3;
119+
const MAX_ALLOWED_VALUES = 5;
120+
121+
// AJV's default messages omit specifics it collects in error.params (which
122+
// additional property is present, which values an enum allows), so append
123+
// them to make the error actionable without re-reading the schema.
124+
const formatAjvError = (error: ErrorObject): string => {
125+
let message = error.message ?? '';
126+
if (
127+
error.keyword === 'additionalProperties' &&
128+
typeof error.params.additionalProperty === 'string'
129+
) {
130+
message += ` ('${error.params.additionalProperty}')`;
131+
} else if (
132+
error.keyword === 'enum' &&
133+
Array.isArray(error.params.allowedValues)
134+
) {
135+
const allowed = error.params.allowedValues as unknown[];
136+
const shown = allowed
137+
.slice(0, MAX_ALLOWED_VALUES)
138+
.map(value => JSON.stringify(value))
139+
.join(', ');
140+
const more =
141+
allowed.length > MAX_ALLOWED_VALUES
142+
? `, +${allowed.length - MAX_ALLOWED_VALUES} more`
143+
: '';
144+
message += `: ${shown}${more}`;
145+
}
146+
return `${error.instancePath || '/'} ${message}`.trim();
147+
};
148+
116149
const formatAjvErrors = (
117150
errors: ErrorObject[] | null | undefined
118151
): string | undefined => {
119-
const error = errors?.[0];
120-
if (!error) return undefined;
121-
return `${error.instancePath || '/'} ${error.message ?? ''}`.trim();
152+
if (!errors || errors.length === 0) return undefined;
153+
const shown = errors.slice(0, MAX_DETAIL_ERRORS).map(formatAjvError);
154+
const more =
155+
errors.length > MAX_DETAIL_ERRORS
156+
? ` (+${errors.length - MAX_DETAIL_ERRORS} more)`
157+
: '';
158+
return shown.join('; ') + more;
122159
};
123160

124161
// Returns a value validator, or null when client-side validation is not

0 commit comments

Comments
 (0)