Skip to content

Commit

Permalink
Chore: remove "dataPath" from data OpenAPI data errors. (#5272)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@thomasheartman
thomasheartman authored Nov 7, 2023
1 parent 92e2b18 commit b3054c9
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 47 deletions.
17 changes: 6 additions & 11 deletions src/lib/error/bad-data-error.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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':
Expand Down
59 changes: 23 additions & 36 deletions src/lib/error/unleash-error.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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: {
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand All @@ -279,8 +271,7 @@ describe('OpenAPI error conversion', () => {
},
{
keyword: 'required',
instancePath: '',
dataPath: '.body',
instancePath: '/body',
schemaPath:
'#/components/schemas/addonCreateUpdateSchema/required',
params: {
Expand Down Expand Up @@ -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' },
Expand Down Expand Up @@ -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' },
Expand All @@ -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(
Expand All @@ -368,21 +357,20 @@ 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({
nestedObject: { a: { b: [] } },
})(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('[]');
Expand All @@ -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';
Expand All @@ -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/);
});
});

Expand Down

0 comments on commit b3054c9

Please sign in to comment.