From a2bda7575696cf1ca3b18ad0f8ed0418977922fe Mon Sep 17 00:00:00 2001 From: Ruben Aguiar Date: Fri, 17 May 2024 15:06:25 +0100 Subject: [PATCH 1/2] Fix case where query parameters are not added We've introduced query parameters in the generator by adding them as normal parameters in the request body for our users. This was done by simply pushing these parameters to the `data` field the users use to send the necessary data to the endpoint. However, there was an edge case. Since we first process the query params and then the requestBody, for cases where the requestBody was a `anyOf`/`allOf` we were fully substituting the parameters to be the requestBody, overwriting the query parameters that were previously added. To fix this, simply inverse the order of operations, first add the requestBody to the parameters and then process query parameters. Signed-off-by: Ruben Aguiar --- src/openApi/v3/parser/getOperation.ts | 36 +++++++-------- test/__snapshots__/index.spec.ts.snap | 66 +++++++++++++++++++++++++++ test/spec/v3.json | 23 ++++++++++ 3 files changed, 107 insertions(+), 18 deletions(-) diff --git a/src/openApi/v3/parser/getOperation.ts b/src/openApi/v3/parser/getOperation.ts index fadee93af..727f3dcd6 100644 --- a/src/openApi/v3/parser/getOperation.ts +++ b/src/openApi/v3/parser/getOperation.ts @@ -78,6 +78,24 @@ export const getOperation = ( mediaType: null, }; + if (op.requestBody) { + const requestBodyDef = getRef(openApi, op.requestBody); + const requestBody = getOperationRequestBody(openApi, requestBodyDef); + operation.imports.push(...requestBody.imports); + operation.parametersBody = requestBody; + dataParameter.properties.push(...requestBody.properties); + if (requestBody.export === 'array') { + // if the requestBody is an array, there are no properties to showcase. We instead want to + // use the whole model as the parameter + dataParameter.properties.push(requestBody); + } else if (requestBody.export === 'one-of' || requestBody.export === 'all-of') { + // if the requestBody is a one-of/all-of, the properties cannot be used since they have no + // names. Instead we want to use the whole model as the parameter. + dataParameter.properties = [requestBody]; + } + dataParameter.isRequired = requestBody.isRequired ? true : dataParameter.isRequired; + } + // Parse the operation parameters (path, query, body, etc). if (op.parameters) { const parameters = getOperationParameters(openApi, op.parameters); @@ -99,24 +117,6 @@ export const getOperation = ( operation.parametersBody = parameters.parametersBody; } - if (op.requestBody) { - const requestBodyDef = getRef(openApi, op.requestBody); - const requestBody = getOperationRequestBody(openApi, requestBodyDef); - operation.imports.push(...requestBody.imports); - operation.parametersBody = requestBody; - dataParameter.properties.push(...requestBody.properties); - if (requestBody.export === 'array') { - // if the requestBody is an array, there are no properties to showcase. We instead want to - // use the whole model as the parameter - dataParameter.properties.push(requestBody); - } else if (requestBody.export === 'one-of' || requestBody.export === 'all-of') { - // if the requestBody is a one-of/all-of, the properties cannot be used since they have no - // names. Instead we want to use the whole model as the parameter. - dataParameter.properties = [requestBody]; - } - dataParameter.isRequired = requestBody.isRequired ? true : dataParameter.isRequired; - } - // Parse the operation responses. if (op.responses) { const operationResponses = getOperationResponses(openApi, op.responses); diff --git a/test/__snapshots__/index.spec.ts.snap b/test/__snapshots__/index.spec.ts.snap index aa0cdbf6b..38b0404a7 100644 --- a/test/__snapshots__/index.spec.ts.snap +++ b/test/__snapshots__/index.spec.ts.snap @@ -5269,6 +5269,7 @@ export { MultipleTags1Service } from './services/MultipleTags1Service.js'; export { MultipleTags2Service } from './services/MultipleTags2Service.js'; export { MultipleTags3Service } from './services/MultipleTags3Service.js'; export { NoContentService } from './services/NoContentService.js'; +export { OneOfService } from './services/OneOfService.js'; export { ParametersService } from './services/ParametersService.js'; export { RequestBodyService } from './services/RequestBodyService.js'; export { ResponseService } from './services/ResponseService.js'; @@ -5297,6 +5298,7 @@ import { MultipleTags1Service } from './services/MultipleTags1Service.js'; import { MultipleTags2Service } from './services/MultipleTags2Service.js'; import { MultipleTags3Service } from './services/MultipleTags3Service.js'; import { NoContentService } from './services/NoContentService.js'; +import { OneOfService } from './services/OneOfService.js'; import { ParametersService } from './services/ParametersService.js'; import { RequestBodyService } from './services/RequestBodyService.js'; import { ResponseService } from './services/ResponseService.js'; @@ -5378,6 +5380,7 @@ applyMixins(LuneClient, [ MultipleTags2Service, MultipleTags3Service, NoContentService, + OneOfService, ParametersService, RequestBodyService, ResponseService, @@ -5402,6 +5405,7 @@ export interface LuneClient extends MultipleTags2Service, MultipleTags3Service, NoContentService, + OneOfService, ParametersService, RequestBodyService, ResponseService, @@ -5493,6 +5497,7 @@ export { MultipleTags1Service } from './services/MultipleTags1Service.js'; export { MultipleTags2Service } from './services/MultipleTags2Service.js'; export { MultipleTags3Service } from './services/MultipleTags3Service.js'; export { NoContentService } from './services/NoContentService.js'; +export { OneOfService } from './services/OneOfService.js'; export { ParametersService } from './services/ParametersService.js'; export { RequestBodyService } from './services/RequestBodyService.js'; export { ResponseService } from './services/ResponseService.js'; @@ -10093,6 +10098,67 @@ export abstract class NoContentService { }" `; +exports[`v3 should generate: ./test/generated/v3/services/OneOfService.ts 1`] = ` +"// ========================================================================================= +// +// This file is AUTO-GENERATED by https://github.com/lune-climate/openapi-typescript-codegen +// +// In most cases you DON'T WANT TO MAKE MANUAL CHANGES to it – they WILL BE OVERWRITTEN. +// +// ========================================================================================= + +/* istanbul ignore file */ +/* tslint:disable */ +/* eslint-disable */ +import type { ModelWithArray } from '../models/ModelWithArray.js'; +import type { ModelWithDictionary } from '../models/ModelWithDictionary.js'; +import type { ModelWithEnum } from '../models/ModelWithEnum.js'; +import type { ModelWithString } from '../models/ModelWithString.js'; + +import { ClientConfig } from '../core/ClientConfig.js' +import { request as __request } from '../core/request.js' +import { ApiError } from '../core/ApiError.js' +import { AxiosInstance } from 'axios' +import { Result } from 'ts-results-es' + +export abstract class OneOfService { + protected abstract client: AxiosInstance + protected abstract config: ClientConfig + + /** + * @param data Request data + * @param options Additional operation options + */ + public queryParamsWithOneOf( + data?: { + /** + * Testing allOf request body at the root level + */ + modelWithOneOfRoot: (ModelWithString | ModelWithEnum | ModelWithArray | ModelWithDictionary); + /** + * This is a reusable parameter + */ + parameter?: string; + }, + options?: { + /** + * Account Id to be used to perform the API call + */ + accountId?: string; + }, + ): Promise> { + return __request(this.client, this.config, options || {}, { + method: 'POST', + url: '/api/v{api-version}/queryParamsWithOneOf', + query: { + 'parameter': data?.parameter, + }, + }); + } + +}" +`; + exports[`v3 should generate: ./test/generated/v3/services/ParametersService.ts 1`] = ` "// ========================================================================================= // diff --git a/test/spec/v3.json b/test/spec/v3.json index 60e7def2e..d150db9a8 100644 --- a/test/spec/v3.json +++ b/test/spec/v3.json @@ -1478,6 +1478,29 @@ } } } + }, + "/api/v{api-version}/queryParamsWithOneOf": { + "post": { + "tags": ["OneOf"], + "parameters": [ + { + "$ref": "#/components/parameters/SimpleParameter" + } + ], + "operationId": "QueryParamsWithOneOf", + "requestBody": { + "description": "Testing allOf request body at the root level", + "required": true, + "nullable": false, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ModelWithOneOfRoot" + } + } + } + } + } } }, "components": { From bb2fa57dd3f900a67e9650c6f01641ff12c330e9 Mon Sep 17 00:00:00 2001 From: Ruben Aguiar Date: Fri, 17 May 2024 15:55:34 +0100 Subject: [PATCH 2/2] Fix order of operations Signed-off-by: Ruben Aguiar --- src/openApi/v3/parser/getOperation.ts | 30 +++++++++++----------- test/__snapshots__/index.spec.ts.snap | 36 ++++++++++++++------------- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/src/openApi/v3/parser/getOperation.ts b/src/openApi/v3/parser/getOperation.ts index 727f3dcd6..843a4e39f 100644 --- a/src/openApi/v3/parser/getOperation.ts +++ b/src/openApi/v3/parser/getOperation.ts @@ -78,6 +78,20 @@ export const getOperation = ( mediaType: null, }; + // Parse the operation parameters (path, body, etc). Query parameters are not included here + const parameters = op.parameters ? getOperationParameters(openApi, op.parameters) : undefined; + if (parameters) { + const newParams = parameters.parameters.filter(p => p.in !== 'query'); + operation.imports.push(...parameters.imports); + operation.parameters.push(...newParams); + operation.parametersPath.push(...parameters.parametersPath); + operation.parametersQuery.push(...parameters.parametersQuery); + operation.parametersForm.push(...parameters.parametersForm); + operation.parametersHeader.push(...parameters.parametersHeader); + operation.parametersCookie.push(...parameters.parametersCookie); + operation.parametersBody = parameters.parametersBody; + } + if (op.requestBody) { const requestBodyDef = getRef(openApi, op.requestBody); const requestBody = getOperationRequestBody(openApi, requestBodyDef); @@ -96,25 +110,13 @@ export const getOperation = ( dataParameter.isRequired = requestBody.isRequired ? true : dataParameter.isRequired; } - // Parse the operation parameters (path, query, body, etc). - if (op.parameters) { - const parameters = getOperationParameters(openApi, op.parameters); - + // Add query parameters after path and body parameters are processed. + if (parameters) { const queryParams = parameters.parameters.filter(p => p.in === 'query'); if (queryParams.length !== 0) { dataParameter.properties.push(...queryParams); dataParameter.isRequired = !!queryParams.find(p => p.isRequired); } - - const newParams = parameters.parameters.filter(p => p.in !== 'query'); - operation.imports.push(...parameters.imports); - operation.parameters.push(...newParams); - operation.parametersPath.push(...parameters.parametersPath); - operation.parametersQuery.push(...parameters.parametersQuery); - operation.parametersForm.push(...parameters.parametersForm); - operation.parametersHeader.push(...parameters.parametersHeader); - operation.parametersCookie.push(...parameters.parametersCookie); - operation.parametersBody = parameters.parametersBody; } // Parse the operation responses. diff --git a/test/__snapshots__/index.spec.ts.snap b/test/__snapshots__/index.spec.ts.snap index 38b0404a7..876685551 100644 --- a/test/__snapshots__/index.spec.ts.snap +++ b/test/__snapshots__/index.spec.ts.snap @@ -9732,14 +9732,14 @@ export abstract class FormDataService { */ public postApiFormData( data?: { - /** - * This is a reusable parameter - */ - parameter?: string; /** * This is a simple string property */ prop?: string; + /** + * This is a reusable parameter + */ + parameter?: string; }, options?: { /** @@ -10153,6 +10153,8 @@ export abstract class OneOfService { query: { 'parameter': data?.parameter, }, + body: data?.modelWithOneOfRoot, + mediaType: 'application/json', }); } @@ -10257,14 +10259,14 @@ export abstract class ParametersService { * This is the parameter that goes into the request query params */ parameterQuery: string | null; - /** - * This is the parameter with a reserved keyword - */ - default?: string; /** * This is a simple string property */ prop?: string; + /** + * This is the parameter with a reserved keyword + */ + default?: string; }, parameterPath1?: string, parameterPath2?: string, @@ -10309,15 +10311,15 @@ export abstract class ParametersService { * @param options Additional operation options */ public getCallWithOptionalParam( - data: { - /** - * This is an optional parameter - */ - parameter?: string; + data?: { /** * This is a simple string property */ prop?: string; + /** + * This is an optional parameter + */ + parameter?: string; }, options?: { /** @@ -10405,14 +10407,14 @@ export abstract class RequestBodyService { */ public postApiRequestBody( data?: { - /** - * This is a reusable parameter - */ - parameter?: string; /** * This is a simple string property */ prop?: string; + /** + * This is a reusable parameter + */ + parameter?: string; }, options?: { /**