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.

Author: danielsharvey

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

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);
   });
 

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 = {