add includeHttpRequestContext additional property defaulting to "true" for reactive and "false" for blocking. Implement tests and add samples to check compilation success. #22890
+18,554
−386
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
…" for reactive and "false" for blocking. Implement tests and add samples to check compilation success.
…Context-to-java-spring-codegen # Conflicts: # .github/workflows/samples-jdk17.yaml
Contributor
Author
|
I seem unable to edit the title of the PR. If I could, this is what I would change it to: [java-spring] - add 'includeHttpRequestContext' additional property defaulting to "true" for reactive and "false" for blocking to include ServerWebExchange/HttpServletRequest |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 318 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
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="docs/generators/spring.md">
<violation number="1" location="docs/generators/spring.md:64">
P3: Documentation inconsistency: the description says the default differs for reactive vs blocking, but the Default column lists only `false`, which can mislead reactive users.</violation>
</file>
<file name="docs/generators/java-camel.md">
<violation number="1" location="docs/generators/java-camel.md:71">
P3: The documentation row for includeHttpRequestContext is internally inconsistent: the description says the default is conditional (true for reactive, false for blocking), but the Default column shows a fixed false. For java-camel (inherits SpringCodegen), the default is conditional based on the reactive flag. Update the Default column or description to match actual behavior to avoid misleading users.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/JavaSpring/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaSpring/api.mustache:27">
P2: ApiIgnore import is gated by includeHttpRequestContext, but @ApiIgnore is still used for pageable/provide-args even when includeHttpRequestContext is false, leading to missing import compilation errors.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/JavaSpring/api.mustache:283">
P2: includeHttpRequestContext on blocking + no params can generate a method signature missing a comma before Pageable/provide-args, leading to invalid Java code.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/JavaSpring/apiDelegate.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaSpring/apiDelegate.mustache:82">
P2: Pagination separator logic still relies on `hasParams` and assumes a reactive exchange parameter always exists. When the request context parameter is optional, this can generate invalid method signatures (missing or leading comma) for pagination-only operations.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{>formParams}}{{>cookieParams}}{{^-last}}, | ||
| {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, | ||
| {{/hasParams}}{{#swagger2AnnotationLibrary}}@Parameter(hidden = true){{/swagger2AnnotationLibrary}}{{#springFoxDocumentationProvider}}@ApiIgnore{{/springFoxDocumentationProvider}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, | ||
| {{/-last}}{{/allParams}}{{#includeHttpRequestContext}}{{#hasParams}}, |
There was a problem hiding this comment.
P2: includeHttpRequestContext on blocking + no params can generate a method signature missing a comma before Pageable/provide-args, leading to invalid Java code.
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/JavaSpring/api.mustache, line 283:
<comment>includeHttpRequestContext on blocking + no params can generate a method signature missing a comma before Pageable/provide-args, leading to invalid Java code.</comment>
<file context>
@@ -270,24 +280,24 @@ public interface {{classname}} {
{{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{>formParams}}{{>cookieParams}}{{^-last}},
- {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}},
- {{/hasParams}}{{#swagger2AnnotationLibrary}}@Parameter(hidden = true){{/swagger2AnnotationLibrary}}{{#springFoxDocumentationProvider}}@ApiIgnore{{/springFoxDocumentationProvider}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}},
+ {{/-last}}{{/allParams}}{{#includeHttpRequestContext}}{{#hasParams}},
+ {{/hasParams}}{{#swagger2AnnotationLibrary}}@Parameter(hidden = true){{/swagger2AnnotationLibrary}}{{#springFoxDocumentationProvider}}@ApiIgnore{{/springFoxDocumentationProvider}} final {{#reactive}}ServerWebExchange exchange{{/reactive}}{{^reactive}}HttpServletRequest servletRequest{{/reactive}}{{/includeHttpRequestContext}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}},
{{/hasParams}}{{^hasParams}}{{#reactive}},{{/reactive}}{{/hasParams}}{{#springFoxDocumentationProvider}}@ApiIgnore {{/springFoxDocumentationProvider}}{{#springDocDocumentationProvider}}@ParameterObject {{/springDocDocumentationProvider}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}{{#vendorExtensions.x-spring-provide-args}}{{#hasParams}},
</file context>
Comment on lines
+27
to
+29
| {{#springFoxDocumentationProvider}}{{#includeHttpRequestContext}} | ||
| import springfox.documentation.annotations.ApiIgnore; | ||
| {{/includeHttpRequestContext}}{{/springFoxDocumentationProvider}} |
There was a problem hiding this comment.
P2: ApiIgnore import is gated by includeHttpRequestContext, but @ApiIgnore is still used for pageable/provide-args even when includeHttpRequestContext is false, leading to missing import compilation errors.
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/JavaSpring/api.mustache, line 27:
<comment>ApiIgnore import is gated by includeHttpRequestContext, but @ApiIgnore is still used for pageable/provide-args even when includeHttpRequestContext is false, leading to missing import compilation errors.</comment>
<file context>
@@ -24,6 +24,9 @@ import io.swagger.v3.oas.annotations.media.ExampleObject;
{{#swagger1AnnotationLibrary}}
import io.swagger.annotations.*;
{{/swagger1AnnotationLibrary}}
+{{#springFoxDocumentationProvider}}{{#includeHttpRequestContext}}
+import springfox.documentation.annotations.ApiIgnore;
+{{/includeHttpRequestContext}}{{/springFoxDocumentationProvider}}
</file context>
Suggested change
| {{#springFoxDocumentationProvider}}{{#includeHttpRequestContext}} | |
| import springfox.documentation.annotations.ApiIgnore; | |
| {{/includeHttpRequestContext}}{{/springFoxDocumentationProvider}} | |
| {{#springFoxDocumentationProvider}} | |
| import springfox.documentation.annotations.ApiIgnore; | |
| {{/springFoxDocumentationProvider}} |
| {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, | ||
| {{/hasParams}}ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}}{{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}} { | ||
| {{/-last}}{{/allParams}}{{#includeHttpRequestContext}}{{#hasParams}}, | ||
| {{/hasParams}}{{#reactive}}ServerWebExchange exchange{{/reactive}}{{^reactive}}HttpServletRequest servletRequest{{/reactive}}{{/includeHttpRequestContext}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}}{{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}} { |
There was a problem hiding this comment.
P2: Pagination separator logic still relies on hasParams and assumes a reactive exchange parameter always exists. When the request context parameter is optional, this can generate invalid method signatures (missing or leading comma) for pagination-only operations.
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/JavaSpring/apiDelegate.mustache, line 82:
<comment>Pagination separator logic still relies on `hasParams` and assumes a reactive exchange parameter always exists. When the request context parameter is optional, this can generate invalid method signatures (missing or leading comma) for pagination-only operations.</comment>
<file context>
@@ -71,8 +78,8 @@ public interface {{classname}}Delegate {
- {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}},
- {{/hasParams}}ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}}{{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}} {
+ {{/-last}}{{/allParams}}{{#includeHttpRequestContext}}{{#hasParams}},
+ {{/hasParams}}{{#reactive}}ServerWebExchange exchange{{/reactive}}{{^reactive}}HttpServletRequest servletRequest{{/reactive}}{{/includeHttpRequestContext}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}}{{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}} {
{{>methodBody}}{{! prevent indent}}
}{{/jdk8-default-interface}}
</file context>
Suggested change
| {{/hasParams}}{{#reactive}}ServerWebExchange exchange{{/reactive}}{{^reactive}}HttpServletRequest servletRequest{{/reactive}}{{/includeHttpRequestContext}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}}{{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}} { | |
| {{/hasParams}}{{#reactive}}ServerWebExchange exchange{{/reactive}}{{^reactive}}HttpServletRequest servletRequest{{/reactive}}{{/includeHttpRequestContext}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#includeHttpRequestContext}}, {{/includeHttpRequestContext}}{{/hasParams}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}}{{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}} { |
| |ignoreAnyOfInEnum|Ignore anyOf keyword in enum| |false| | ||
| |implicitHeaders|Skip header parameters in the generated API methods using @ApiImplicitParams annotation.| |false| | ||
| |implicitHeadersRegex|Skip header parameters that matches given regex in the generated API methods using @ApiImplicitParams annotation. Note: this parameter is ignored when implicitHeaders=true| |null| | ||
| |includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |false| |
There was a problem hiding this comment.
P3: Documentation inconsistency: the description says the default differs for reactive vs blocking, but the Default column lists only false, which can mislead reactive users.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/generators/spring.md, line 64:
<comment>Documentation inconsistency: the description says the default differs for reactive vs blocking, but the Default column lists only `false`, which can mislead reactive users.</comment>
<file context>
@@ -61,6 +61,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|ignoreAnyOfInEnum|Ignore anyOf keyword in enum| |false|
|implicitHeaders|Skip header parameters in the generated API methods using @ApiImplicitParams annotation.| |false|
|implicitHeadersRegex|Skip header parameters that matches given regex in the generated API methods using @ApiImplicitParams annotation. Note: this parameter is ignored when implicitHeaders=true| |null|
+|includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |false|
|interfaceOnly|Whether to generate only API interface stubs without the server files.| |false|
|invokerPackage|root package for generated code| |org.openapitools.api|
</file context>
Suggested change
| |includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |false| | |
| |includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |true (reactive) / false (blocking)| |
| |ignoreAnyOfInEnum|Ignore anyOf keyword in enum| |false| | ||
| |implicitHeaders|Skip header parameters in the generated API methods using @ApiImplicitParams annotation.| |false| | ||
| |implicitHeadersRegex|Skip header parameters that matches given regex in the generated API methods using @ApiImplicitParams annotation. Note: this parameter is ignored when implicitHeaders=true| |null| | ||
| |includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |false| |
There was a problem hiding this comment.
P3: The documentation row for includeHttpRequestContext is internally inconsistent: the description says the default is conditional (true for reactive, false for blocking), but the Default column shows a fixed false. For java-camel (inherits SpringCodegen), the default is conditional based on the reactive flag. Update the Default column or description to match actual behavior to avoid misleading users.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/generators/java-camel.md, line 71:
<comment>The documentation row for includeHttpRequestContext is internally inconsistent: the description says the default is conditional (true for reactive, false for blocking), but the Default column shows a fixed false. For java-camel (inherits SpringCodegen), the default is conditional based on the reactive flag. Update the Default column or description to match actual behavior to avoid misleading users.</comment>
<file context>
@@ -68,6 +68,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|ignoreAnyOfInEnum|Ignore anyOf keyword in enum| |false|
|implicitHeaders|Skip header parameters in the generated API methods using @ApiImplicitParams annotation.| |false|
|implicitHeadersRegex|Skip header parameters that matches given regex in the generated API methods using @ApiImplicitParams annotation. Note: this parameter is ignored when implicitHeaders=true| |null|
+|includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |false|
|interfaceOnly|Whether to generate only API interface stubs without the server files.| |false|
|invokerPackage|root package for generated code| |org.openapitools.api|
</file context>
Suggested change
| |includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |false| | |
| |includeHttpRequestContext|Whether to include HttpServletRequest (blocking) or ServerWebExchange (reactive) as additional parameter in generated methods. Defaults to 'true' for reactive and 'false' for blocking.| |true (reactive) / false (blocking)| |
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.
As was promised by me, this is to bring java-spring and kotlin-spring closer to parity functionality-wise. I implemented the change in such a way that it does not impact current users of the java-spring generator at all:
reactivestack, theincludeHttpRequestContextdefaults totrueand thus theServerWebExchangeis present in the methods. (Same as it was before introduction of this flag)blockingstack, theincludeHttpRequestContextdefaults tofalseand thus theHttpServletRequestis NOT present in the methods. (Same as it was before introduction of this flag)Explicitly setting the flag allows the developer to control the presence/absence of this http request context object.
PR 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)Summary by cubic
Adds a new includeHttpRequestContext option to the Spring generator to pass the HTTP request context to controller/delegate methods. Defaults to true for reactive (ServerWebExchange) and false for blocking (HttpServletRequest).
New Features
Migration
Written for commit 49fa455. Summary will update on new commits.