1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-02-14 00:19:16 +01:00

Chore: remove "dataPath" from data OpenAPI data errors. (#5272)

The `dataPath` was present (but not in the type) in previous versions of
the
error library that we use. But with the recent major upgrade, it's
been removed and the `instancePath` property has finally come into use.

This PR removes all the handling for the previous property and
replaces it with `instancePath`. Because the `dataPath` used full
stops and the `instancePath` uses slashes, we need to change a little
bit of the handling too.
This commit is contained in:
Thomas Heartman 2023-11-07 09:26:14 +01:00 committed by GitHub
parent 92e2b1890c
commit b3054c9277
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 29 additions and 47 deletions

View File

@ -38,7 +38,7 @@ class BadDataError extends UnleashError {
export default BadDataError;
const constructPath = (pathToParent: string, propertyName: string) =>
[pathToParent, propertyName].filter(Boolean).join('.');
[pathToParent, propertyName].filter(Boolean).join('/');
const missingRequiredPropertyMessage = (
pathToParentObject: string,
@ -74,10 +74,10 @@ const genericErrorMessage = (
propertyName: string,
errorMessage: string = 'is invalid',
) => {
const input = getProp(requestBody, propertyName);
const input = getProp(requestBody, propertyName.split('/'));
const youSent = JSON.stringify(input);
const description = `The \`.${propertyName}\` property ${errorMessage}. You sent ${youSent}.`;
const description = `The \`${propertyName}\` property ${errorMessage}. You sent ${youSent}.`;
return {
description,
message: description,
@ -122,16 +122,11 @@ const enumMessage = (
};
};
// Sometimes, the error object contains a dataPath, even if it's not
type ActualErrorObject = ErrorObject & { dataPath?: string };
export const fromOpenApiValidationError =
(requestBody: object) =>
(validationError: ActualErrorObject): ValidationErrorDescription => {
const { instancePath, params, message, dataPath } = validationError;
const propertyName =
dataPath?.substring('.body.'.length) ??
instancePath.substring('/body/'.length);
(validationError: ErrorObject): ValidationErrorDescription => {
const { instancePath, params, message } = validationError;
const propertyName = instancePath.substring('/body/'.length);
switch (validationError.keyword) {
case 'required':

View File

@ -50,8 +50,7 @@ describe('OpenAPI error conversion', () => {
it('Gives useful error messages for missing properties', () => {
const error = {
keyword: 'required',
instancePath: '',
dataPath: '.body',
instancePath: '/body',
schemaPath: '#/components/schemas/addonCreateUpdateSchema/required',
params: {
missingProperty: 'enabled',
@ -75,8 +74,7 @@ describe('OpenAPI error conversion', () => {
it('Gives useful error messages for type errors', () => {
const error = {
keyword: 'type',
instancePath: '',
dataPath: '.body.parameters',
instancePath: '/body/parameters',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/parameters/type',
params: {
@ -101,13 +99,12 @@ describe('OpenAPI error conversion', () => {
expect(result.description).toContain(JSON.stringify(parameterValue));
});
it.each(['.body', '.body.subObject'])(
it.each(['/body', '/body/subObject'])(
'Gives useful error messages for oneOf errors in %s',
(dataPath) => {
(instancePath) => {
const error = {
keyword: 'oneOf',
instancePath: '',
dataPath,
instancePath,
schemaPath: '#/components/schemas/createApiTokenSchema/oneOf',
params: {
passingSchemas: null,
@ -125,7 +122,7 @@ describe('OpenAPI error conversion', () => {
description:
// it provides the message
expect.stringContaining(error.message),
path: dataPath.substring('.body.'.length),
path: instancePath.substring('/body/'.length),
});
// it tells the user what happened
@ -134,18 +131,17 @@ describe('OpenAPI error conversion', () => {
);
// it tells the user what part of the request body this pertains to
expect(result.description).toContain(
dataPath === '.body'
instancePath === '/body'
? 'root object'
: `"${dataPath.substring('.body.'.length)}" property`,
: `"${instancePath.substring('/body/'.length)}" property`,
);
},
);
it('Gives useful pattern error messages', () => {
const error = {
instancePath: '',
keyword: 'pattern',
dataPath: '.body.description',
instancePath: '/body/description',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/description/pattern',
params: {
@ -199,9 +195,8 @@ describe('OpenAPI error conversion', () => {
it('Gives useful min/maxlength error messages', () => {
const error = {
instancePath: '',
keyword: 'maxLength',
dataPath: '.body.description',
instancePath: '/body/description',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/description/maxLength',
params: {
@ -230,8 +225,7 @@ describe('OpenAPI error conversion', () => {
it('Handles numerical min/max errors', () => {
const error = {
keyword: 'maximum',
instancePath: '',
dataPath: '.body.newprop',
instancePath: '/body/newprop',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/newprop/maximum',
params: {
@ -265,9 +259,7 @@ describe('OpenAPI error conversion', () => {
const errors: [ErrorObject, ...ErrorObject[]] = [
{
keyword: 'maximum',
instancePath: '',
// @ts-expect-error
dataPath: '.body.newprop',
instancePath: '/body/newprop',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/newprop/maximum',
params: {
@ -279,8 +271,7 @@ describe('OpenAPI error conversion', () => {
},
{
keyword: 'required',
instancePath: '',
dataPath: '.body',
instancePath: '/body',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/required',
params: {
@ -312,8 +303,7 @@ describe('OpenAPI error conversion', () => {
it('gives useful messages for base-level properties', () => {
const openApiError = {
keyword: 'additionalProperties',
instancePath: '',
dataPath: '.body',
instancePath: '/body',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/additionalProperties',
params: { additionalProperty: 'bogus' },
@ -343,8 +333,7 @@ describe('OpenAPI error conversion', () => {
};
const openApiError = {
keyword: 'additionalProperties',
instancePath: '',
dataPath: '.body.nestedObject.nested2',
instancePath: '/body/nestedObject/nested2',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/nestedObject/properties/nested2/additionalProperties',
params: { additionalProperty: 'extraPropertyName' },
@ -354,8 +343,8 @@ describe('OpenAPI error conversion', () => {
const error = fromOpenApiValidationError(request2)(openApiError);
expect(error).toMatchObject({
description: expect.stringContaining('nestedObject.nested2'),
path: 'nestedObject.nested2.extraPropertyName',
description: expect.stringContaining('nestedObject/nested2'),
path: 'nestedObject/nested2/extraPropertyName',
});
expect(error.description).toContain(
@ -368,12 +357,11 @@ describe('OpenAPI error conversion', () => {
it('Handles deeply nested properties gracefully', () => {
const error = {
keyword: 'type',
dataPath: '.body.nestedObject.a.b',
instancePath: '/body/nestedObject/a/b',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/properties/nestedObject/properties/a/properties/b/type',
params: { type: 'string' },
message: 'should be string',
instancePath: '',
};
const result = fromOpenApiValidationError({
@ -381,8 +369,8 @@ describe('OpenAPI error conversion', () => {
})(error);
expect(result).toMatchObject({
description: expect.stringMatching(/\bnestedObject.a.b\b/),
path: 'nestedObject.a.b',
description: expect.stringMatching(/\bnestedObject\/a\/b\b/),
path: 'nestedObject/a/b',
});
expect(result.description).toContain('[]');
@ -391,11 +379,10 @@ describe('OpenAPI error conversion', () => {
it('Handles deeply nested properties on referenced schemas', () => {
const error = {
keyword: 'type',
dataPath: '.body.nestedObject.a.b',
instancePath: '/body/nestedObject/a/b',
schemaPath: '#/components/schemas/parametersSchema/type',
params: { type: 'object' },
message: 'should be object',
instancePath: '',
};
const illegalValue = 'illegal string';
@ -405,10 +392,10 @@ describe('OpenAPI error conversion', () => {
expect(result).toMatchObject({
description: expect.stringContaining(illegalValue),
path: 'nestedObject.a.b',
path: 'nestedObject/a/b',
});
expect(result.description).toMatch(/\bnestedObject.a.b\b/);
expect(result.description).toMatch(/\bnestedObject\/a\/b\b/);
});
});