feat(oas3): path-defined params uniq per operation (#218)

Co-authored-by: Jakub Rożek <[email protected]>

src/oas/accessors.ts

@@ -21,9 +21,13 @@ export function createOasParamsIterator(
     const seenParams = new Set();
     const { parentId, context } = this;
     const opParams = Array.isArray(operation.parameters) ? operation.parameters : [];
-    const params = [...opParams, ...(Array.isArray(path.parameters) ? path.parameters : [])];
+    const pathParams = Array.isArray(path.parameters) ? path.parameters : [];
+    const params = [...opParams, ...pathParams];
 
     for (let i = 0; i < params.length; i++) {
+      this.context = i < opParams.length ? 'operation' : 'path';
+      this.parentId = this.ids[this.context];
+
       const maybeParameterObject = this.maybeResolveLocalRef(params[i]);
       if (isReferenceObject(maybeParameterObject)) {
         yield params[i];
@@ -47,10 +51,6 @@ export function createOasParamsIterator(
       }
       seenParams.add(key);
 
-      if (this.context !== 'service') {
-        this.context = i < opParams.length ? 'operation' : 'path';
-      }
-
       yield maybeParameterObject;
     }
 

src/oas3/__tests__/__fixtures__/id/bundled.ts

@@ -184,14 +184,14 @@ export default {
           {
             description: 'Id of an existing user.',
             examples: [],
-            id: 'http_path_param-http_path-service_abc-/users/{}-userId',
+            id: 'http_path_param-http_operation-service_abc-get-/users/{}-userId',
             name: 'userId',
             required: true,
             schema: {
               $schema: 'http://json-schema.org/draft-07/schema#',
               type: 'integer',
               'x-stoplight': {
-                id: 'schema-http_path_param-http_path-service_abc-/users/{}-userId-',
+                id: 'schema-http_path_param-http_operation-service_abc-get-/users/{}-userId-',
               },
             },
             style: 'simple',
@@ -314,14 +314,14 @@ export default {
           {
             description: 'Id of an existing user.',
             examples: [],
-            id: 'http_path_param-http_path-service_abc-/users/{}-userId',
+            id: 'http_path_param-http_operation-service_abc-post-/users/{}-userId',
             name: 'userId',
             required: true,
             schema: {
               $schema: 'http://json-schema.org/draft-07/schema#',
               type: 'integer',
               'x-stoplight': {
-                id: 'schema-http_path_param-http_path-service_abc-/users/{}-userId-',
+                id: 'schema-http_path_param-http_operation-service_abc-post-/users/{}-userId-',
               },
             },
             style: 'simple',

src/oas3/__tests__/__fixtures__/id/legacy.ts

@@ -200,19 +200,19 @@ export default [
       path: [
         {
           // hash(`http_path_param-${parentId}-${param.name}`)
-          // This was defined on the path, so we use the path to generate the id (thus if another operation on this path was in this doc, it would have path param with same id)
+          // This was defined on the path, so we use the *operation* to generate the id (thus if another operation on this path was in this doc, it would have path param with a different id)
           // path's id = hash(`http_path-${parentId}-${path}`)
-          // The closest parent id to a path, is the service, so this equals... (remember that path segments have characters removed, since they are basically meaningless)
+          // The closest parent id to a operation, is the service, so this equals... (remember that path segments have characters removed, since they are basically meaningless)
           // hash('http_path-service_abc-/users/{}') = '05574f79'
           // and then the final path param id...
-          // hash('http_path_param-05574f79-userId') = 'http_path_param-http_path-service_abc-/users/{}-userId'
-          id: 'http_path_param-http_path-service_abc-/users/{}-userId',
+          // hash('http_path_param-05574f79-userId') = 'http_path_param-http_operation-service_abc-get-/users/{}-userId'
+          id: 'http_path_param-http_operation-service_abc-get-/users/{}-userId',
           name: 'userId',
           required: true,
           description: 'Id of an existing user.',
           style: 'simple',
           schema: {
-            'x-stoplight': { id: 'schema-http_path_param-http_path-service_abc-/users/{}-userId-' },
+            'x-stoplight': { id: 'schema-http_path_param-http_operation-service_abc-get-/users/{}-userId-' },
             $schema: 'http://json-schema.org/draft-07/schema#',
             type: 'integer',
           },
@@ -422,9 +422,9 @@ export default [
       cookie: [],
       path: [
         {
-          // Same process as other path param, resulting in the same
-          // ID (so this path param node will end up as single instance in the graph, with an edge from each operation pointing at it)
-          id: 'http_path_param-http_path-service_abc-/users/{}-userId',
+          // Same process as other path param, resulting in a new
+          // ID (so this path param node will end up as multiple instances in the graph, with an edge from each operation pointing at it)
+          id: 'http_path_param-http_operation-service_abc-post-/users/{}-userId',
           name: 'userId',
           required: true,
           description: 'Id of an existing user.',
@@ -434,7 +434,7 @@ export default [
             // hash(`http_media-${parentId}-${key}`)
             // closest parent with an id is the response, so ends up being...
             // hash('http_media-http_response-service_abc-ErrorResponse-application/json')
-            'x-stoplight': { id: 'schema-http_path_param-http_path-service_abc-/users/{}-userId-' },
+            'x-stoplight': { id: 'schema-http_path_param-http_operation-service_abc-post-/users/{}-userId-' },
             type: 'integer',
           },
           examples: [],

src/oas3/transformers/__tests__/request.test.ts

@@ -1,4 +1,7 @@
+import { IHttpOperationRequest } from '@stoplight/types';
+
 import { createContext } from '../../../oas/context';
+import { bundleOas3Service } from '../../service';
 import { translateToRequest as _translateToRequest } from '../request';
 
 const translateToRequest = (path: Record<string, unknown>, operation: Record<string, unknown>) =>
@@ -100,4 +103,49 @@ describe('translateOas3ToRequest', () => {
       },
     });
   });
+
+  it('given path-defined parameters should create unique parameters', () => {
+    const getOperation = {
+      parameters: [],
+    };
+    const postOperation = {
+      parameters: [],
+    };
+
+    const path = {
+      parameters: [
+        {
+          name: 'param-name-1',
+          in: 'query',
+          description: 'descr',
+          deprecated: true,
+          content: {
+            'content-a': {
+              schema: {},
+            },
+          },
+        },
+      ],
+      get: getOperation,
+      post: postOperation,
+    };
+
+    const service = {
+      paths: {
+        '/': path,
+      },
+    };
+
+    const bundledService = bundleOas3Service({
+      document: service,
+    });
+
+    const getRequest = bundledService.operations[0].request as IHttpOperationRequest<true>;
+    const postRequest = bundledService.operations[1].request as IHttpOperationRequest<true>;
+
+    const getQueryParam = getRequest.query?.[0];
+    const postQueryParam = postRequest.query?.[0];
+
+    expect(getQueryParam).not.toEqual(postQueryParam);
+  });
 });

src/oas3/transformers/request.ts

@@ -93,6 +93,12 @@ const translateParameterObjectSchema = withContext<
 export const translateParameterObject = withContext<
   Oas3TranslateFunction<[parameterObject: ParameterObject], IHttpParam<true>>
 >(function (parameterObject) {
+  if (this.context === 'path') {
+    // we do not have a path representation to hang this path-defined parameter on, so this becomes a new operation-defined parameter
+    this.context = 'operation';
+    this.parentId = this.ids['operation'];
+  }
+
   const kind = parameterObject.in === 'path' ? 'pathParam' : parameterObject.in;
   const name = parameterObject.name;
   const keyOrName = getSharedKey(parameterObject, name);
@@ -174,7 +180,7 @@ export const translateToRequest = withContext<
     if (isReferenceObject(param)) {
       target.push(syncReferenceObject(param, this.references));
     } else {
-      target.push(translateParameterObject.call(this, param) as any);
+      target.push(translateParameterObject.call(this, param));
     }
   }