Merge pull request #87 from cdimascio/enums

enum validation errors should include a list of allowed enum values

Author: cdimascio

src/middlewares/openapi.request.validator.ts

@@ -3,6 +3,7 @@ import {
   extractContentType,
   validationError,
   ajvErrorsToValidatorError,
+  augmentAjvErrors,
 } from './util';
 import ono from 'ono';
 import { NextFunction, Response } from 'express';
@@ -22,7 +23,11 @@ export class RequestValidator {
     this.ajv = createRequestAjv(apiDocs, options);
   }
 
-  public validate(req: OpenApiRequest, res: Response, next: NextFunction): void {
+  public validate(
+    req: OpenApiRequest,
+    res: Response,
+    next: NextFunction,
+  ): void {
     if (!req.openapi) {
       // this path was not found in open api and
       // this path is not defined under an openapi base path
@@ -138,12 +143,11 @@ export class RequestValidator {
           : undefined,
       };
       const valid = validator(reqToValidate);
-      // save errors, Ajv overwrites errors on each validation call (race condition?)
-      // TODO look into Ajv async errors plugins
-      const errors = [...(validator.errors || [])];
       if (valid) {
         next();
       } else {
+        // TODO look into Ajv async errors plugins
+        const errors = augmentAjvErrors([...(validator.errors || [])]);
         const err = ajvErrorsToValidatorError(400, errors);
         const message = this.ajv.errorsText(errors, { dataVar: 'request' });
         throw ono(err, message);
@@ -166,8 +170,6 @@ export class RequestValidator {
     }
   }
 
-  
-
   private requestBodyToSchema(path, contentType, requestBody: any = {}) {
     if (requestBody.content) {
       const content = requestBody.content[contentType];

src/middlewares/openapi.response.validator.ts

@@ -3,6 +3,7 @@ import * as Ajv from 'ajv';
 import mung from './modded.express.mung';
 import { createResponseAjv } from './ajv';
 import {
+  augmentAjvErrors,
   extractContentType,
   ajvErrorsToValidatorError,
   validationError,
@@ -84,7 +85,7 @@ export class ResponseValidator {
     });
 
     if (!valid) {
-      const errors = validator.errors;
+      const errors = augmentAjvErrors(validator.errors);
       const message = this.ajv.errorsText(errors, {
         dataVar: '', // responses
       });

src/middlewares/util.ts

@@ -38,6 +38,23 @@ export function validationError(
   return ono(err, message);
 }
 
+/**
+ * (side-effecting) modifies the errors object
+ * TODO - do this some other way
+ * @param errors
+ */
+export function augmentAjvErrors(errors: Ajv.ErrorObject[] = []): Ajv.ErrorObject[] {
+  errors.forEach(e => {
+    if (e.keyword === 'enum') {
+      const params: any = e.params;
+      const allowedEnumValues = params && params.allowedValues;
+      e.message = !!allowedEnumValues
+        ? `${e.message}: ${allowedEnumValues.join(', ')}`
+        : e.message;
+    }
+  });
+  return errors;
+}
 export function ajvErrorsToValidatorError(
   status: number,
   errors: Ajv.ErrorObject[],
@@ -51,7 +68,7 @@ export function ajvErrorsToValidatorError(
         params.missingProperty &&
         e.dataPath + '.' + params.missingProperty;
       const additionalProperty =
-        e.params &&
+        params &&
         params.additionalProperty &&
         e.dataPath + '.' + params.additionalProperty;
       const path = required || additionalProperty || e.dataPath || e.schemaPath;

test/routes.spec.ts

@@ -102,7 +102,7 @@ describe(packageJson.name, () => {
             expect(e).to.have.length(1);
             expect(e[0].path).to.contain('testJson');
             expect(e[0].message).to.equal(
-              'should be equal to one of the allowed values',
+              'should be equal to one of the allowed values: bar, baz',
             );
           }));
 
@@ -126,7 +126,7 @@ describe(packageJson.name, () => {
             expect(e).to.have.length(1);
             expect(e[0].path).to.contain('testArray');
             expect(e[0].message).to.equal(
-              'should be equal to one of the allowed values',
+              'should be equal to one of the allowed values: foo, bar, baz',
             );
           }));
 
@@ -152,7 +152,7 @@ describe(packageJson.name, () => {
             expect(e).to.have.length(1);
             expect(e[0].path).to.contain('testArrayExplode');
             expect(e[0].message).to.equal(
-              'should be equal to one of the allowed values',
+              'should be equal to one of the allowed values: foo, bar, baz',
             );
           }));
     });
@@ -316,6 +316,23 @@ describe(packageJson.name, () => {
           });
       });
 
+      it('should return 400 an invalid enum value is given', async () => {
+        return request(apps[i])
+          .get(`${basePath}/pets`)
+          .query({
+            limit: 10,
+            test: 'one',
+            testArray: ['unknown_value'],
+          })
+          .expect(400)
+          .then(r => {
+            const e = r.body.errors;
+            expect(e[0].message).equals(
+              'should be equal to one of the allowed values: foo, bar, baz',
+            );
+          });
+      });
+
       it('should handle multiple path params with coereion', async () => {
         const id = '10';
         const attributeId = '12';