fix: Improve correctness of generated OpenAPI 3 output

Uploaded to https://redoc.ly/developer-portal yielding these issues and fixes. Refactor '$ref' checks using new common code. Fix cross-generated-route data pollution by ensuring source swagger data cloned. Add underscore to path variable names, used to differentiate the PK value used for paths from query variables. Specifically, differentiate the PK value used for shortcut blueprint update routes, which allow for PK update using query parameters. Some validators expect unique names across all parameter types. Sort tag name specified as part of Operation. Update test data and test fixtures.
theoomoregbee · Jun 4, 2020 · 534af7f · 534af7f
1 parent c389d10
commit 534af7f
Show file tree

Hide file tree

Showing 6 changed files with 3,085 additions and 309 deletions.
diff --git a/api/controllers/UserController.js b/api/controllers/UserController.js
@@ -44,7 +44,7 @@ module.exports = {
    *   parameters:
    *     - name: example-only
    *       description: Username to use for logout (dummy for test)
-   *       in: path
+   *       in: query
    *       required: true
    *       schema:
    *         type: string

diff --git a/lib/generators.ts b/lib/generators.ts
@@ -353,6 +353,12 @@ export const generatePaths = (routes: SwaggerRouteInfo[], templates: BlueprintAc
 
     let pathEntry: OpenApi.Operation;
 
+    const isParam = (inType: string, name: string): boolean => {
+      return !!pathEntry.parameters
+        .map(parameter => resolveRef(specification, parameter) as OpenApi.Parameter)
+        .find(parameter => parameter && 'in' in parameter && parameter.in == inType && parameter.name == name);
+    }
+
     if (route.middlewareType === MiddlewareType.BLUEPRINT && route.model) {
 
       if(route.model.swagger?.modelSchema?.exclude === true) {
@@ -373,7 +379,7 @@ export const generatePaths = (routes: SwaggerRouteInfo[], templates: BlueprintAc
             if (components.parameters && !components.parameters[pname]) {
               components.parameters[pname] = {
                 in: 'path',
-                name: primaryKey,
+                name: '_' + primaryKey, // note '_' as per transformSailsPathsToSwaggerPaths()
                 required: true,
                 schema: generateAttributeSchema(attributeInfo),
                 description: subst('The desired **{globalId}** record\'s primary key value'),
@@ -391,12 +397,10 @@ export const generatePaths = (routes: SwaggerRouteInfo[], templates: BlueprintAc
         tags: route.swagger?.tags || route.model.swagger.modelSchema?.tags || route.model.swagger.actions?.allactions?.tags || [route.model.globalId],
         parameters,
         responses: cloneDeep(defaultsValues.responses || {}),
-        ...omit(route.model.swagger.actions?.allactions || {}, 'exclude'),
-        ...omit(route.swagger || {}, 'exclude')
+        ...cloneDeep(omit(route.model.swagger.actions?.allactions || {}, 'exclude')),
+        ...cloneDeep(omit(route.swagger || {}, 'exclude')),
       };
 
-      const isParam = (inType: string, name: string): boolean => !!pathEntry.parameters.find(parameter => 'in' in parameter && parameter.in == inType && parameter.name == name);
-
       const modifiers = {
 
         addPopulateQueryParam: () => {
@@ -625,7 +629,7 @@ export const generatePaths = (routes: SwaggerRouteInfo[], templates: BlueprintAc
       pathEntry = {
         parameters: [],
         responses: cloneDeep(defaultsValues.responses || {}),
-        ...omit(route.swagger || {}, 'exclude')
+        ...cloneDeep(omit(route.swagger || {}, 'exclude')),
       }
 
       if (route.actionType === 'actions2') {
@@ -761,21 +765,9 @@ export const generatePaths = (routes: SwaggerRouteInfo[], templates: BlueprintAc
     }
 
     if (route.variables) {
-      // first resolve '$ref' parameters
-      const resolved = pathEntry.parameters.map(parameter => {
-        let ref = '$ref' in parameter && parameter.$ref;
-        if (!ref) return parameter;
-        const _prefix = '#/components/parameters/';
-        if (ref.startsWith(_prefix)) {
-          ref = ref.substring(_prefix.length);
-          const dereferenced = components.parameters![ref];
-          return dereferenced ? dereferenced : parameter;
-        }
-      }) as OpenApi.Parameter[];
-
       // now add patternVariables that don't already exist
       route.variables.map(v => {
-        const existing = resolved.find(p => p && 'in' in p && p.in == 'path' && p.name == v);
+        const existing = isParam('path', v);
         if (existing) return;
         pathEntry.parameters.push({
           in: 'path',
@@ -787,6 +779,10 @@ export const generatePaths = (routes: SwaggerRouteInfo[], templates: BlueprintAc
       });
     }
 
+    if(pathEntry.tags) {
+      pathEntry.tags.sort();
+    }
+
     set(paths, [route.path, route.verb], pathEntry);
   });
 

diff --git a/lib/transformations.ts b/lib/transformations.ts
@@ -16,19 +16,26 @@ const transformSailsPathToSwaggerPath = (path: string): string => {
  * Maps from a Sails route path of the form `/path/:id` to a
  * Swagger path of the form `/path/{id}`.
  *
- * Also transform standard Sails '{id}' to '{primaryKeyAttributeName}' where
- * primary key is not `id`.
+ * Also transform standard Sails primary key reference '{id}' to
+ * '_{primaryKeyAttributeName}'.
+ *
+ * Add underscore to path variable names, used to differentiate the PK value
+ * used for paths from query variables. Specifically, differentiate the PK value
+ * used for shortcut blueprint update routes, which allow for PK update
+ * using query parameters. Some validators expect unique names across all
+ * parameter types.
  */
 export const transformSailsPathsToSwaggerPaths = (routes: SwaggerRouteInfo[]): void => {
 
   routes.map(route => {
 
     route.path = transformSailsPathToSwaggerPath(route.path);
 
-    if (route.model?.primaryKey && route.model.primaryKey !== 'id') {
-      route.path = route.path.replace('{id}', `{${route.model.primaryKey}}`);
-      route.variables = route.variables.map(v => v === 'id' ? route.model!.primaryKey : v);
-      route.optionalVariables = route.optionalVariables.map(v => v === 'id' ? route.model!.primaryKey : v);
+    if (route.model?.primaryKey) {
+      const pathVariable = '_' + route.model.primaryKey;
+      route.path = route.path.replace('{id}', `{${pathVariable}}`);
+      route.variables = route.variables.map(v => v === 'id' ? pathVariable : v);
+      route.optionalVariables = route.optionalVariables.map(v => v === 'id' ? pathVariable : v);
     }
 
   });
@@ -123,7 +130,7 @@ export const transformSailsPathsToSwaggerPaths = (routes: SwaggerRouteInfo[]): v
           // first route becomes 'aggregated' version
           const g = actionGroup[0];
           const prefix = g.match[1];
-          const pk = g.route.model!.primaryKey;
+          const pk = '_' + g.route.model!.primaryKey; // note '_' as per transformSailsPathsToSwaggerPaths()
           const shortcutRoutePart = g.match[3] || '';
           const childPart = g.match[4] || '';
           const aggregatedRoute = {