[typescript-nestjs-server] #22928 improve request parameter handling #22960
+542
−47
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rings from generating imports
… various parameter types
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
7 issues found across 38 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/typescript-nestjs-server/controller.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/typescript-nestjs-server/controller.mustache:3">
P2: Controller template imports `../cookies-decorator`, but there is no generator template/file for `cookies-decorator`, so generated code will fail to compile due to a missing module.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/typescript-nestjs-server/controller.mustache:16">
P2: Header parameter extraction may fail because @Headers uses lowercased keys; using the original OpenAPI header casing ({{baseName}}) can return undefined for headers like 'X-Auth-Token'.</violation>
</file>
<file name="samples/server/petstore/typescript-nestjs-server/builds/default/controllers/PetApi.controller.ts">
<violation number="1" location="samples/server/petstore/typescript-nestjs-server/builds/default/controllers/PetApi.controller.ts:42">
P2: Form and file parameters are missing NestJS parameter decorators, so these values will be undefined at runtime and the API handlers receive no form/file data.</violation>
<violation number="2" location="samples/server/petstore/typescript-nestjs-server/builds/default/controllers/PetApi.controller.ts:47">
P2: Upload endpoint defines a `file` parameter but lacks `@UseInterceptors(FileInterceptor(...))` and `@UploadedFile()`; NestJS won’t extract multipart file data, so `file` will be undefined and the upload endpoint won’t work.</violation>
</file>
<file name="samples/server/petstore/typescript-nestjs-server/test/app.e2e-spec.ts">
<violation number="1" location="samples/server/petstore/typescript-nestjs-server/test/app.e2e-spec.ts:61">
P2: Missing return/await on the supertest request means this test can pass without running the async assertions.</violation>
</file>
<file name="samples/server/petstore/typescript-nestjs-server/builds/parameters/controllers/DefaultApi.controller.ts">
<violation number="1" location="samples/server/petstore/typescript-nestjs-server/builds/parameters/controllers/DefaultApi.controller.ts:11">
P2: Numeric query/header/cookie params are typed as number but will arrive as strings at runtime unless ParseIntPipe or global ValidationPipe transform is enabled, leading to type mismatch when DefaultApi expects numbers.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptNestjsServerCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptNestjsServerCodegen.java:442">
P2: Union import parsing skips required model imports when the union starts with a primitive (e.g., "string | MyModel"), because it only checks needToImport on the first union part.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {{#operation}} | ||
| @{{#vendorExtensions.x-http-method}}{{.}}{{/vendorExtensions.x-http-method}}{{^vendorExtensions.x-http-method}}{{httpMethod}}{{/vendorExtensions.x-http-method}}('{{path}}') | ||
| {{operationId}}({{#allParams}}{{#isPathParam}}@Param('{{paramName}}') {{/isPathParam}}{{#isQueryParam}}@Query('{{paramName}}') {{/isQueryParam}}{{#isBodyParam}}@Body() {{/isBodyParam}}{{paramName}}: {{{dataType}}}, {{/allParams}}@Req() request: Request): {{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}} | Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> | Observable<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> { | ||
| {{operationId}}({{#allParams}}{{#isPathParam}}@Param('{{baseName}}') {{/isPathParam}}{{#isQueryParam}}@Query('{{baseName}}') {{/isQueryParam}}{{#isHeaderParam}}@Headers('{{baseName}}') {{/isHeaderParam}}{{#isCookieParam}}@Cookies('{{baseName}}') {{/isCookieParam}}{{#isBodyParam}}@Body() {{/isBodyParam}}{{paramName}}: {{{dataType}}}{{#isNullable}} | null{{/isNullable}}{{^required}} | undefined{{/required}}{{#defaultValue}} = {{{defaultValue}}}{{/defaultValue}}, {{/allParams}}@Req() request: Request): {{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}} | Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> | Observable<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> { |
Contributor
There was a problem hiding this comment.
P2: Header parameter extraction may fail because @headers uses lowercased keys; using the original OpenAPI header casing ({{baseName}}) can return undefined for headers like 'X-Auth-Token'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/typescript-nestjs-server/controller.mustache, line 16:
<comment>Header parameter extraction may fail because @Headers uses lowercased keys; using the original OpenAPI header casing ({{baseName}}) can return undefined for headers like 'X-Auth-Token'.</comment>
<file context>
@@ -10,7 +13,7 @@ export class {{classname}}Controller {
{{#operation}}
@{{#vendorExtensions.x-http-method}}{{.}}{{/vendorExtensions.x-http-method}}{{^vendorExtensions.x-http-method}}{{httpMethod}}{{/vendorExtensions.x-http-method}}('{{path}}')
- {{operationId}}({{#allParams}}{{#isPathParam}}@Param('{{paramName}}') {{/isPathParam}}{{#isQueryParam}}@Query('{{paramName}}') {{/isQueryParam}}{{#isBodyParam}}@Body() {{/isBodyParam}}{{paramName}}: {{{dataType}}}, {{/allParams}}@Req() request: Request): {{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}} | Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> | Observable<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> {
+ {{operationId}}({{#allParams}}{{#isPathParam}}@Param('{{baseName}}') {{/isPathParam}}{{#isQueryParam}}@Query('{{baseName}}') {{/isQueryParam}}{{#isHeaderParam}}@Headers('{{baseName}}') {{/isHeaderParam}}{{#isCookieParam}}@Cookies('{{baseName}}') {{/isCookieParam}}{{#isBodyParam}}@Body() {{/isBodyParam}}{{paramName}}: {{{dataType}}}{{#isNullable}} | null{{/isNullable}}{{^required}} | undefined{{/required}}{{#defaultValue}} = {{{defaultValue}}}{{/defaultValue}}, {{/allParams}}@Req() request: Request): {{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}} | Promise<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> | Observable<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> {
return this.{{classVarName}}.{{operationId}}({{#useSingleRequestParameter}}{ {{/useSingleRequestParameter}}{{#allParams}}{{paramName}}, {{/allParams}}{{#useSingleRequestParameter}}}, {{/useSingleRequestParameter}}request);
}
</file context>
| import { Body, Controller{{#httpMethods}}, {{.}}{{/httpMethods}}, Param, Query, Req } from '@nestjs/common'; | ||
| import { Body, Controller{{#httpMethods}}, {{.}}{{/httpMethods}}, Param, Query, Headers, Req } from '@nestjs/common'; | ||
| import { Observable } from 'rxjs'; | ||
| import { Cookies } from '../cookies-decorator'; |
Contributor
There was a problem hiding this comment.
P2: Controller template imports ../cookies-decorator, but there is no generator template/file for cookies-decorator, so generated code will fail to compile due to a missing module.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/typescript-nestjs-server/controller.mustache, line 3:
<comment>Controller template imports `../cookies-decorator`, but there is no generator template/file for `cookies-decorator`, so generated code will fail to compile due to a missing module.</comment>
<file context>
@@ -1,7 +1,10 @@
-import { Body, Controller{{#httpMethods}}, {{.}}{{/httpMethods}}, Param, Query, Req } from '@nestjs/common';
+import { Body, Controller{{#httpMethods}}, {{.}}{{/httpMethods}}, Param, Query, Headers, Req } from '@nestjs/common';
import { Observable } from 'rxjs';
+import { Cookies } from '../cookies-decorator';
import { {{classname}} } from '../{{apiPackage}}';
+{{#tsImports.0}}
</file context>
|
|
||
| @Post('/pet/:petId/uploadImage') | ||
| uploadFile(@Param('petId') petId: number, additionalMetadata: string, file: Blob, @Req() request: Request): ApiResponse | Promise<ApiResponse> | Observable<ApiResponse> { | ||
| uploadFile(@Param('petId') petId: number, additionalMetadata: string | undefined, file: Blob | undefined, @Req() request: Request): ApiResponse | Promise<ApiResponse> | Observable<ApiResponse> { |
Contributor
There was a problem hiding this comment.
P2: Upload endpoint defines a file parameter but lacks @UseInterceptors(FileInterceptor(...)) and @UploadedFile(); NestJS won’t extract multipart file data, so file will be undefined and the upload endpoint won’t work.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/typescript-nestjs-server/builds/default/controllers/PetApi.controller.ts, line 47:
<comment>Upload endpoint defines a `file` parameter but lacks `@UseInterceptors(FileInterceptor(...))` and `@UploadedFile()`; NestJS won’t extract multipart file data, so `file` will be undefined and the upload endpoint won’t work.</comment>
<file context>
@@ -38,12 +39,12 @@ export class PetApiController {
@Post('/pet/:petId/uploadImage')
- uploadFile(@Param('petId') petId: number, additionalMetadata: string, file: Blob, @Req() request: Request): ApiResponse | Promise<ApiResponse> | Observable<ApiResponse> {
+ uploadFile(@Param('petId') petId: number, additionalMetadata: string | undefined, file: Blob | undefined, @Req() request: Request): ApiResponse | Promise<ApiResponse> | Observable<ApiResponse> {
return this.petApi.uploadFile(petId, additionalMetadata, file, request);
}
</file context>
| @@ -1,5 +1,6 @@ | |||
| import { Body, Controller, Delete, Get, Post, Put, Param, Query, Req } from '@nestjs/common'; | |||
| import { Body, Controller, Delete, Get, Post, Put, Param, Query, Headers, Req } from '@nestjs/common'; | |||
Contributor
There was a problem hiding this comment.
P2: Form and file parameters are missing NestJS parameter decorators, so these values will be undefined at runtime and the API handlers receive no form/file data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/typescript-nestjs-server/builds/default/controllers/PetApi.controller.ts, line 42:
<comment>Form and file parameters are missing NestJS parameter decorators, so these values will be undefined at runtime and the API handlers receive no form/file data.</comment>
<file context>
@@ -38,12 +39,12 @@ export class PetApiController {
@Post('/pet/:petId')
- updatePetWithForm(@Param('petId') petId: number, name: string, status: string, @Req() request: Request): void | Promise<void> | Observable<void> {
+ updatePetWithForm(@Param('petId') petId: number, name: string | undefined, status: string | undefined, @Req() request: Request): void | Promise<void> | Observable<void> {
return this.petApi.updatePetWithForm(petId, name, status, request);
}
</file context>
| it('should set default parameters', () => { | ||
| let defaultService: DefaultService = app.get(DefaultApi); | ||
|
|
||
| request(app.getHttpServer()) |
Contributor
There was a problem hiding this comment.
P2: Missing return/await on the supertest request means this test can pass without running the async assertions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/typescript-nestjs-server/test/app.e2e-spec.ts, line 61:
<comment>Missing return/await on the supertest request means this test can pass without running the async assertions.</comment>
<file context>
@@ -29,25 +31,95 @@ describe('AppModule (e2e)', () => {
+ it('should set default parameters', () => {
+ let defaultService: DefaultService = app.get(DefaultApi);
+
+ request(app.getHttpServer())
+ .get('/test/parameters/path_a/path_b')
+ .expect(200, () => {
</file context>
Suggested change
| request(app.getHttpServer()) | |
| return request(app.getHttpServer()) |
| constructor(private readonly defaultApi: DefaultApi) {} | ||
|
|
||
| @Get('/test/parameters/:path_default/:path_nullable') | ||
| findPetsByStatus(@Param('path_default') pathDefault: string, @Param('path_nullable') pathNullable: string, @Query('query_default') queryDefault: string | undefined = 'available', @Query('query_default_enum') queryDefaultEnum: 'A' | 'B' | 'C' | undefined = 'B', @Query('query_default_int') queryDefaultInt: number | undefined = 3, @Headers('header_default') headerDefault: string | undefined = 'available', @Headers('header_default_enum') headerDefaultEnum: 'A' | 'B' | 'C' | undefined = 'B', @Headers('header_default_int') headerDefaultInt: number | undefined = 3, @Cookies('cookie_default') cookieDefault: string | undefined = 'available', @Cookies('cookie_default_enum') cookieDefaultEnum: 'A' | 'B' | 'C' | undefined = 'B', @Cookies('cookie_default_int') cookieDefaultInt: number | undefined = 3, @Query('query_nullable') queryNullable: string | null | undefined, @Headers('header_nullable') headerNullable: string | null | undefined, @Cookies('cookie_nullable') cookieNullable: string | null | undefined, @Query('$query-$dollar-sign') $query$dollarSign: string | undefined, @Req() request: Request): void | Promise<void> | Observable<void> { |
Contributor
There was a problem hiding this comment.
P2: Numeric query/header/cookie params are typed as number but will arrive as strings at runtime unless ParseIntPipe or global ValidationPipe transform is enabled, leading to type mismatch when DefaultApi expects numbers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/typescript-nestjs-server/builds/parameters/controllers/DefaultApi.controller.ts, line 11:
<comment>Numeric query/header/cookie params are typed as number but will arrive as strings at runtime unless ParseIntPipe or global ValidationPipe transform is enabled, leading to type mismatch when DefaultApi expects numbers.</comment>
<file context>
@@ -0,0 +1,15 @@
+ constructor(private readonly defaultApi: DefaultApi) {}
+
+ @Get('/test/parameters/:path_default/:path_nullable')
+ findPetsByStatus(@Param('path_default') pathDefault: string, @Param('path_nullable') pathNullable: string, @Query('query_default') queryDefault: string | undefined = 'available', @Query('query_default_enum') queryDefaultEnum: 'A' | 'B' | 'C' | undefined = 'B', @Query('query_default_int') queryDefaultInt: number | undefined = 3, @Headers('header_default') headerDefault: string | undefined = 'available', @Headers('header_default_enum') headerDefaultEnum: 'A' | 'B' | 'C' | undefined = 'B', @Headers('header_default_int') headerDefaultInt: number | undefined = 3, @Cookies('cookie_default') cookieDefault: string | undefined = 'available', @Cookies('cookie_default_enum') cookieDefaultEnum: 'A' | 'B' | 'C' | undefined = 'B', @Cookies('cookie_default_int') cookieDefaultInt: number | undefined = 3, @Query('query_nullable') queryNullable: string | null | undefined, @Headers('header_nullable') headerNullable: string | null | undefined, @Cookies('cookie_nullable') cookieNullable: string | null | undefined, @Query('$query-$dollar-sign') $query$dollarSign: string | undefined, @Req() request: Request): void | Promise<void> | Observable<void> {
+ return this.defaultApi.findPetsByStatus({ pathDefault, pathNullable, queryDefault, queryDefaultEnum, queryDefaultInt, headerDefault, headerDefaultEnum, headerDefaultInt, cookieDefault, cookieDefaultEnum, cookieDefaultInt, queryNullable, headerNullable, cookieNullable, $query$dollarSign, }, request);
+ }
</file context>
| if (name.indexOf(" | ") >= 0) { | ||
| String[] parts = name.split(" \\| "); | ||
| Collections.addAll(newImports, parts); | ||
| if (needToImport(parts[0])) { |
Contributor
There was a problem hiding this comment.
P2: Union import parsing skips required model imports when the union starts with a primitive (e.g., "string | MyModel"), because it only checks needToImport on the first union part.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptNestjsServerCodegen.java, line 442:
<comment>Union import parsing skips required model imports when the union starts with a primitive (e.g., "string | MyModel"), because it only checks needToImport on the first union part.</comment>
<file context>
@@ -429,7 +439,9 @@ private Set<String> parseImports(CodegenModel cm) {
if (name.indexOf(" | ") >= 0) {
String[] parts = name.split(" \\| ");
- Collections.addAll(newImports, parts);
+ if (needToImport(parts[0])) {
+ Collections.addAll(newImports, parts);
+ }
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
addresses #22928
parameter-test-spec.yamlto correctly handle various parameter typesPR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04) @joscha (2024/10) @dennisameling (2026/02)
@wing328
Summary by cubic
Improves parameter handling for the TypeScript NestJS server generator. Fixes #22928 by generating correct nullability/optional types, handling header/cookie params, and avoiding bad inline enum imports.
New Features
Bug Fixes
Written for commit 617a8f6. Summary will update on new commits.