(fix) #400 readOnly does not work together with polymorphic oneOf. (#414

) * (fix) #400 readOnly does not work together with polymorphic oneOf. * fix typo * (feat) enhanced readonly handling * add oneOf test * enable tests
cdimascio · Oct 26, 2020 · da55514 · da55514
1 parent 80e84a3
commit da55514
Show file tree

Hide file tree

Showing 16 changed files with 403 additions and 7,613 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -36,6 +36,7 @@
     "content-type": "^1.0.4",
     "js-yaml": "^3.14.0",
     "json-schema-ref-parser": "^9.0.6",
+    "lodash.clonedeep": "^4.5.0",
     "lodash.merge": "^4.6.2",
     "lodash.uniq": "^4.5.0",
     "lodash.zipobject": "^4.1.3",

diff --git a/src/framework/openapi.context.ts b/src/framework/openapi.context.ts
@@ -56,13 +56,17 @@ export class OpenApiContext {
   // side-effecting builds express/openapi route maps
   private buildRouteMaps(routes: RouteMetadata[]): void {
     for (const route of routes) {
-      const routeMethods = this.expressRouteMap[route.expressRoute];
+      const { basePath, expressRoute, openApiRoute, method } = route;
+      const routeMethods = this.expressRouteMap[expressRoute];
+      const pathKey = openApiRoute.substring(basePath.length);
+      const schema = this.apiDoc.paths[pathKey][method.toLowerCase()];
       if (routeMethods) {
-        routeMethods[route.method] = route.schema;
+        routeMethods[route.method] = schema;
       } else {
-        const { schema, openApiRoute, expressRoute } = route;
+        const { basePath, openApiRoute, expressRoute } = route;
         const routeMethod = { [route.method]: schema };
         const routeDetails = {
+          basePath,
           _openApiRoute: openApiRoute,
           _expressRoute: expressRoute,
           ...routeMethod,

diff --git a/src/framework/openapi.spec.loader.ts b/src/framework/openapi.spec.loader.ts
@@ -12,11 +12,11 @@ export interface Spec {
 }
 
 export interface RouteMetadata {
+  basePath: string;
   expressRoute: string;
   openApiRoute: string;
   method: string;
   pathParams: string[];
-  schema: OpenAPIV3.OperationObject;
 }
 
 interface DiscoveredRoutes {
@@ -57,19 +57,14 @@ export class OpenApiSpecLoader {
           const bp = bpa.replace(/\/$/, '');
           for (const [path, methods] of Object.entries(apiDoc.paths)) {
             for (const [method, schema] of Object.entries(methods)) {
-              if (method.startsWith('x-') || ['parameters', 'summary', 'description'].includes(method)) {
+              if (
+                method.startsWith('x-') ||
+                ['parameters', 'summary', 'description'].includes(method)
+              ) {
                 continue;
               }
-              const schemaParameters = new Set();
-              (schema.parameters ?? []).forEach(parameter =>
-                schemaParameters.add(parameter),
-              );
-              (methods.parameters ?? []).forEach(parameter =>
-                schemaParameters.add(parameter),
-              );
-              schema.parameters = Array.from(schemaParameters);
               const pathParams = new Set<string>();
-              for (const param of schema.parameters) {
+              for (const param of schema.parameters ?? []) {
                 if (param.in === 'path') {
                   pathParams.add(param.name);
                 }
@@ -81,11 +76,11 @@ export class OpenApiSpecLoader {
                 .join('/');
 
               routes.push({
+                basePath: bp,
                 expressRoute,
                 openApiRoute,
                 method: method.toUpperCase(),
                 pathParams: Array.from(pathParams),
-                schema,
               });
             }
           }

diff --git a/src/middlewares/openapi.metadata.ts b/src/middlewares/openapi.metadata.ts
@@ -1,19 +1,24 @@
 import * as _zipObject from 'lodash.zipobject';
 import { pathToRegexp } from 'path-to-regexp';
+import * as deepCopy from 'lodash.clonedeep';
 import { Response, NextFunction } from 'express';
 import { OpenApiContext } from '../framework/openapi.context';
 import {
   OpenApiRequest,
   OpenApiRequestHandler,
   OpenApiRequestMetadata,
+  OpenAPIV3,
 } from '../framework/types';
 
 export function applyOpenApiMetadata(
   openApiContext: OpenApiContext,
+  responseApiDoc: OpenAPIV3.Document,
 ): OpenApiRequestHandler {
   return (req: OpenApiRequest, res: Response, next: NextFunction): void => {
-    // note base path is empty when path is fully qualified i.e. req.path.startsWith('') 
-    const path = req.path.startsWith(req.baseUrl) ? req.path : `${req.baseUrl}/${req.path}`
+    // note base path is empty when path is fully qualified i.e. req.path.startsWith('')
+    const path = req.path.startsWith(req.baseUrl)
+      ? req.path
+      : `${req.baseUrl}/${req.path}`;
     if (openApiContext.shouldIgnoreRoute(path)) {
       return next();
     }
@@ -27,6 +32,10 @@ export function applyOpenApiMetadata(
         schema: schema,
       };
       req.params = pathParams;
+      if (responseApiDoc) {
+        // add the response schema if validating responses
+        (<any>req.openapi)._responseSchema = (<any>matched)._responseSchema;
+      }
     } else if (openApiContext.isManagedRoute(path)) {
       req.openapi = {};
     }
@@ -38,9 +47,11 @@ export function applyOpenApiMetadata(
     const method = req.method;
     const routeEntries = Object.entries(openApiContext.expressRouteMap);
     for (const [expressRoute, methods] of routeEntries) {
-      const schema = methods[method];
       const routePair = openApiContext.routePair(expressRoute);
       const openApiRoute = routePair.openApiRoute;
+      const pathKey = openApiRoute.substring((<any>methods).basePath.length);
+      const schema = openApiContext.apiDoc.paths[pathKey][method.toLowerCase()];
+      const _schema = responseApiDoc?.paths[pathKey][method.toLowerCase()];
 
       const keys = [];
       const strict = !!req.app.enabled('strict routing');
@@ -53,18 +64,18 @@ export function applyOpenApiMetadata(
       const matchedRoute = regexp.exec(path);
 
       if (matchedRoute) {
-        const paramKeys = keys.map(k => k.name);
+        const paramKeys = keys.map((k) => k.name);
         const paramsVals = matchedRoute.slice(1).map(decodeURIComponent);
         const pathParams = _zipObject(paramKeys, paramsVals);
 
-        return {
+        const r = {
           schema,
-          // schema may or may not contain express and openApi routes,
-          // thus we include them here
           expressRoute,
           openApiRoute,
           pathParams,
         };
+        (<any>r)._responseSchema = _schema;
+        return r;
       }
     }
 

diff --git a/src/middlewares/openapi.multipart.ts b/src/middlewares/openapi.multipart.ts
@@ -15,11 +15,11 @@ import { MulterError } from 'multer';
 const multer = require('multer');
 
 export function multipart(
-  context: OpenApiContext,
+  apiDoc: OpenAPIV3.Document,
   options: MultipartOpts,
 ): OpenApiRequestHandler {
   const mult = multer(options.multerOpts);
-  const Ajv = createRequestAjv(context.apiDoc, {
+  const Ajv = createRequestAjv(apiDoc, {
     unknownFormats: options.unknownFormats,
   });
   return (req, res, next) => {

diff --git a/src/middlewares/openapi.response.validator.ts b/src/middlewares/openapi.response.validator.ts
@@ -47,7 +47,9 @@ export class ResponseValidator {
     return mung.json((body, req, res) => {
       if (req.openapi) {
         const openapi = <OpenApiRequestMetadata>req.openapi;
-        const responses = openapi.schema?.responses;
+        // instead of openapi.schema, use openapi._responseSchema to get the response copy
+        const responses: OpenAPIV3.ResponsesObject = (<any>openapi)
+          ._responseSchema?.responses;
 
         const validators = this._getOrBuildValidator(req, responses);
         const path = req.originalUrl;

diff --git a/src/middlewares/openapi.security.ts b/src/middlewares/openapi.security.ts
@@ -7,11 +7,8 @@ import {
   NotFound,
   MethodNotAllowed,
   InternalServerError,
-  Unauthorized,
-  Forbidden,
   HttpError,
 } from '../framework/types';
-import { OpenApiContext } from '../framework/openapi.context';
 
 const defaultSecurityHandler = (
   req: Express.Request,
@@ -28,7 +25,7 @@ interface SecurityHandlerResult {
   error?: string;
 }
 export function security(
-  context: OpenApiContext,
+  apiDoc: OpenAPIV3.Document,
   securityHandlers: SecurityHandlers,
 ): OpenApiRequestHandler {
   return async (req, res, next) => {
@@ -65,15 +62,15 @@ export function security(
 
     // use the local security object or fallbac to api doc's security or undefined
     const securities: OpenAPIV3.SecurityRequirementObject[] =
-      openapi.schema.security ?? context.apiDoc.security;
+      openapi.schema.security ?? apiDoc.security;
 
     const path: string = openapi.openApiRoute;
 
     if (!path || !Array.isArray(securities) || securities.length === 0) {
       return next();
     }
 
-    const securitySchemes = context.apiDoc.components?.securitySchemes;
+    const securitySchemes = apiDoc.components?.securitySchemes;
 
     if (!securitySchemes) {
       const message = `security referenced at path ${path}, but not defined in 'components.securitySchemes'`;

diff --git a/src/middlewares/parsers/body.parse.ts b/src/middlewares/parsers/body.parse.ts
@@ -7,6 +7,10 @@ import {
   UnsupportedMediaType,
 } from '../../framework/types';
 
+type SchemaObject = OpenAPIV3.SchemaObject;
+type ReferenceObject = OpenAPIV3.ReferenceObject;
+type Schema = ReferenceObject | SchemaObject;
+
 export class BodySchemaParser {
   private _apiDoc: OpenAPIV3.Document;
   private ajv: Ajv;
@@ -30,7 +34,6 @@ export class BodySchemaParser {
     const requestBody = <OpenAPIV3.RequestBodyObject>schemaRequestBody;
     if (requestBody?.hasOwnProperty('content')) {
       return this.toSchema(path, contentType, requestBody);
-      // if (requestBody.required) required.push('body');
     }
     return {};
   }
@@ -40,72 +43,41 @@ export class BodySchemaParser {
     contentType: ContentType,
     requestBody: OpenAPIV3.RequestBodyObject,
   ): BodySchema {
-    if (requestBody.content) {
-      let content = null;
-      for (const type of contentType.equivalents()) {
-        content = requestBody.content[type];
-        if (content) break;
-      }
+    if (!requestBody?.content) return {};
 
-      if (!content) {
-        for (const requestContentType of Object.keys(requestBody.content)
-          .sort()
-          .reverse()) {
-          if (requestContentType === '*/*') {
-            content = requestBody.content[requestContentType];
-            break;
-          }
+    let content = null;
+    for (const type of contentType.equivalents()) {
+      content = requestBody.content[type];
+      if (content) break;
+    }
 
-          if (!new RegExp(/^[a-z]+\/\*$/).test(requestContentType)) continue; // not a wildcard of type application/*
+    if (!content) {
+      for (const requestContentType of Object.keys(requestBody.content)
+        .sort()
+        .reverse()) {
+        if (requestContentType === '*/*') {
+          content = requestBody.content[requestContentType];
+          break;
+        }
 
-          const [type] = requestContentType.split('/', 1);
+        if (!new RegExp(/^[a-z]+\/\*$/).test(requestContentType)) continue; // not a wildcard of type application/*
 
-          if (new RegExp(`^${type}\/.+$`).test(contentType.contentType)) {
-            content = requestBody.content[requestContentType];
-            break;
-          }
-        }
-      }
+        const [type] = requestContentType.split('/', 1);
 
-      if (!content) {
-        const msg =
-          contentType.contentType === 'not_provided'
-            ? 'media type not specified'
-            : `unsupported media type ${contentType.contentType}`;
-        throw new UnsupportedMediaType({ path: path, message: msg });
+        if (new RegExp(`^${type}\/.+$`).test(contentType.contentType)) {
+          content = requestBody.content[requestContentType];
+          break;
+        }
       }
-      const schema = this.cleanseContentSchema(content);
-
-      return schema ?? content.schema ?? {};
     }
-    return {};
-  }
 
-  private cleanseContentSchema(content: OpenAPIV3.MediaTypeObject): BodySchema {
-    let contentRefSchema = null;
-    if (content.schema && '$ref' in content.schema) {
-      const resolved = this.ajv.getSchema(content.schema.$ref);
-      const schema = <OpenAPIV3.SchemaObject>resolved?.schema;
-      contentRefSchema = schema?.properties ? { ...schema } : null;
-    }
-    // handle readonly / required request body refs
-    // don't need to copy schema if validator gets its own copy of the api spec
-    // currently all middleware i.e. req and res validators share the spec
-    const schema = contentRefSchema || content.schema;
-    if (schema && schema.properties) {
-      Object.keys(schema.properties).forEach((prop) => {
-        const propertyValue = schema.properties[prop];
-        const required = schema.required;
-        if (propertyValue.readOnly && required) {
-          const index = required.indexOf(prop);
-          if (index > -1) {
-            schema.required = required
-              .slice(0, index)
-              .concat(required.slice(index + 1));
-          }
-        }
-      });
-      return schema;
+    if (!content) {
+      const msg =
+        contentType.contentType === 'not_provided'
+          ? 'media type not specified'
+          : `unsupported media type ${contentType.contentType}`;
+      throw new UnsupportedMediaType({ path: path, message: msg });
     }
+    return content.schema ?? {};
   }
 }
diff --git a/src/middlewares/parsers/req.parameter.mutator.ts b/src/middlewares/parsers/req.parameter.mutator.ts
@@ -67,7 +67,7 @@ export class RequestParameterMutator {
       url.parse(req.originalUrl).query,
     );
 
-    parameters.forEach((p) => {
+    (parameters || []).forEach((p) => {
       const parameter = dereferenceParameter(this._apiDocs, p);
       const { name, schema } = normalizeParameter(this.ajv, parameter);