[typescript-nestjs-server] #22928 improve request parameter handling#22960
[typescript-nestjs-server] #22928 improve request parameter handling#22960aryobenholzner wants to merge 7 commits intoOpenAPITools:masterfrom
Conversation
…rings from generating imports
… various parameter types
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.
modules/openapi-generator/src/main/resources/typescript-nestjs-server/controller.mustache
Outdated
Show resolved
Hide resolved
modules/openapi-generator/src/main/resources/typescript-nestjs-server/controller.mustache
Outdated
Show resolved
Hide resolved
|
|
||
| @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> { |
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'; | |||
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>
samples/server/petstore/typescript-nestjs-server/test/app.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...ver/petstore/typescript-nestjs-server/builds/parameters/controllers/DefaultApi.controller.ts
Outdated
Show resolved
Hide resolved
...enerator/src/main/java/org/openapitools/codegen/languages/TypeScriptNestjsServerCodegen.java
Outdated
Show resolved
Hide resolved
...enerator/src/main/java/org/openapitools/codegen/languages/TypeScriptNestjsServerCodegen.java
Show resolved
Hide resolved
...les/openapi-generator/src/main/resources/typescript-nestjs-server/cookies-decorator.mustache
Outdated
Show resolved
Hide resolved
...les/openapi-generator/src/main/resources/typescript-nestjs-server/cookies-decorator.mustache
Show resolved
Hide resolved
…ons and test execution
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
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/cookies-decorator.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/typescript-nestjs-server/cookies-decorator.mustache:20">
P2: Signed cookies parsed by cookie-parser are moved to `request.signedCookies`, so this decorator will return `undefined` for signed cookie parameters in Express setups with a secret. Consider falling back to `signedCookies` when the cookie is not found in `cookies`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...les/openapi-generator/src/main/resources/typescript-nestjs-server/cookies-decorator.mustache
Outdated
Show resolved
Hide resolved
… parameters, use DefaultValuePipe for default values
There was a problem hiding this comment.
2 issues found across 19 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/server/petstore/typescript-nestjs-server/builds/default/decorators/headers-decorator.ts">
<violation number="1" location="samples/server/petstore/typescript-nestjs-server/builds/default/decorators/headers-decorator.ts:15">
P2: Header lookup is case-sensitive against request.headers, which are lowercased by Node. Mixed-case header names (e.g., 'X-Request-ID') will return undefined unless normalized.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/typescript-nestjs-server/paramPipe.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/typescript-nestjs-server/paramPipe.mustache:1">
P2: Optional numeric parameters without defaults will be parsed by ParseIntPipe/ParseFloatPipe and throw BadRequest when omitted, effectively making them required.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
samples/server/petstore/typescript-nestjs-server/builds/default/decorators/headers-decorator.ts
Outdated
Show resolved
Hide resolved
modules/openapi-generator/src/main/resources/typescript-nestjs-server/paramPipe.mustache
Outdated
Show resolved
Hide resolved
…, check each import for unions
…rs for number parse pipes
|
addressed nearly all of the feedback. the multipart and file upload requests should probably be a separate issue/PR as its not related to the original issue. |
macjohnny
left a comment
There was a problem hiding this comment.
should we update the generated readme to indicate some additional imports/middleware are needed? or what exactly are users supposed to configure to make proper use of these changes?
| * ``` | ||
| */ | ||
| export const Headers = createParamDecorator((data: string, ctx: ExecutionContext) => { | ||
| const request = ctx.switchToHttp().getRequest(); |
There was a problem hiding this comment.
please fix the indentation here
| * findAll(@Headers('name') name: string) {} | ||
| * ``` | ||
| */ | ||
| export const Headers = createParamDecorator((data: string, ctx: ExecutionContext) => { |
There was a problem hiding this comment.
| export const Headers = createParamDecorator((data: string, ctx: ExecutionContext) => { | |
| export const Headers = createParamDecorator((headerName: string, ctx: ExecutionContext) => { |
| * findAll(@Cookies('name') name: string) {} | ||
| * ``` | ||
| */ | ||
| export const Cookies = createParamDecorator((data: string, ctx: ExecutionContext) => { |
There was a problem hiding this comment.
| export const Cookies = createParamDecorator((data: string, ctx: ExecutionContext) => { | |
| export const Cookies = createParamDecorator((cookieName: string, ctx: ExecutionContext) => { |
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 request parameter handling for the TypeScript NestJS server generator to fix #22928, adding default values, numeric parsing with optional pipes, header/cookie binding, and safer type/import generation.
New Features
Bug Fixes
Written for commit 57ec23f. Summary will update on new commits.