From b1f3d530dd933ccd15954c599ccf8df179d14f65 Mon Sep 17 00:00:00 2001 From: "Daniel A. White" Date: Wed, 15 Mar 2023 14:08:37 -0400 Subject: [PATCH] feat(oas3): path-defined params uniq per operation (#218) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jakub Rożek --- src/oas/accessors.ts | 10 ++-- src/oas3/__tests__/__fixtures__/id/bundled.ts | 8 ++-- src/oas3/__tests__/__fixtures__/id/legacy.ts | 18 +++---- .../transformers/__tests__/request.test.ts | 48 +++++++++++++++++++ src/oas3/transformers/request.ts | 8 +++- 5 files changed, 73 insertions(+), 19 deletions(-) diff --git a/src/oas/accessors.ts b/src/oas/accessors.ts index f66552b..0d758df 100644 --- a/src/oas/accessors.ts +++ b/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; } diff --git a/src/oas3/__tests__/__fixtures__/id/bundled.ts b/src/oas3/__tests__/__fixtures__/id/bundled.ts index 5e6adfe..54af34d 100644 --- a/src/oas3/__tests__/__fixtures__/id/bundled.ts +++ b/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', diff --git a/src/oas3/__tests__/__fixtures__/id/legacy.ts b/src/oas3/__tests__/__fixtures__/id/legacy.ts index cf3a9d8..f82767e 100644 --- a/src/oas3/__tests__/__fixtures__/id/legacy.ts +++ b/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: [], diff --git a/src/oas3/transformers/__tests__/request.test.ts b/src/oas3/transformers/__tests__/request.test.ts index e206931..a2af83a 100644 --- a/src/oas3/transformers/__tests__/request.test.ts +++ b/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, operation: Record) => @@ -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; + const postRequest = bundledService.operations[1].request as IHttpOperationRequest; + + const getQueryParam = getRequest.query?.[0]; + const postQueryParam = postRequest.query?.[0]; + + expect(getQueryParam).not.toEqual(postQueryParam); + }); }); diff --git a/src/oas3/transformers/request.ts b/src/oas3/transformers/request.ts index 8f66017..3b12918 100644 --- a/src/oas3/transformers/request.ts +++ b/src/oas3/transformers/request.ts @@ -93,6 +93,12 @@ const translateParameterObjectSchema = withContext< export const translateParameterObject = withContext< Oas3TranslateFunction<[parameterObject: ParameterObject], IHttpParam> >(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)); } }