[java-spring] - add 'includeHttpRequestContext' additional property defaulting to "true" for reactive and "false" for blocking to include ServerWebExchange/HttpServletRequest#22910
Open
Picazsoo wants to merge 19 commits intoOpenAPITools:masterfrom
Conversation
…" 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
…Context-to-java-spring-codegen
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 132 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="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/configuration/SpringFoxConfiguration.java">
<violation number="1" location="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/configuration/SpringFoxConfiguration.java:67">
P2: Configurable basePath is inserted into replaceFirst as a regex without escaping, so regex metacharacters in basePath can cause incorrect path stripping (e.g., "/v1.0" will match "/v10" prefixes). Escape basePath to treat it literally.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/User.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/User.java:276">
P2: toString() includes the raw password value, which can leak credentials if User objects are logged or included in error messages.</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:291">
P2: Missing comma in paginated signatures when includeHttpRequestContext is enabled for blocking mode (reactive=false) and hasParams=false, causing generated Java syntax errors.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/configuration/SpringDocConfiguration.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/configuration/SpringDocConfiguration.java:33">
P2: OAuth2 security scheme is defined without required `flows`, producing an invalid OpenAPI document.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/JavaSpring/methodBody.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaSpring/methodBody.mustache:52">
P2: Reactive stubs lose the NOT_IMPLEMENTED status when includeHttpRequestContext is false, so unimplemented endpoints return 200 OK instead of 501.</violation>
</file>
<file name="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/api/FakeApi.java">
<violation number="1" location="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/api/FakeApi.java:404">
P2: GET endpoint is constrained by `consumes = application/x-www-form-urlencoded`, which requires a Content-Type header even though the request has no body, so standard GET calls won’t match this handler.</violation>
<violation number="2" location="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/api/FakeApi.java:404">
P2: Endpoint consumes application/x-www-form-urlencoded but includes a @RequestPart MultipartFile parameter, which requires multipart/form-data. The file part will not bind under form-urlencoded requests, causing 400/415 errors.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java:36">
P2: clone() returns the same instance, which breaks DateFormat cloning semantics and defeats thread-safety (DateFormat/StdDateFormat are not thread-safe). Jackson expects clones to provide per-thread copies, so returning `this` risks shared mutable state in concurrent use.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...ttp-request-context/src/main/java/org/openapitools/configuration/SpringFoxConfiguration.java
Show resolved
Hide resolved
...ore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/User.java
Show resolved
Hide resolved
|
|
||
| // Override this method | ||
| {{#jdk8-default-interface}}default {{/jdk8-default-interface}} {{>responseType}} {{operationId}}({{#allParams}}{{^isFile}}{{^isBodyParam}}{{>optionalDataType}}{{/isBodyParam}}{{#isBodyParam}}{{^reactive}}{{>optionalDataType}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}}{{/isBodyParam}}{{/isFile}}{{#isFile}}{{#reactive}}{{#isArray}}Flux<{{/isArray}}Part{{#isArray}}>{{/isArray}}{{/reactive}}{{^reactive}}{{#isArray}}List<{{/isArray}}MultipartFile{{#isArray}}>{{/isArray}}{{/reactive}}{{/isFile}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, {{/hasParams}}{{#springFoxDocumentationProvider}}@ApiIgnore{{/springFoxDocumentationProvider}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}{{#springFoxDocumentationProvider}}@ApiIgnore{{/springFoxDocumentationProvider}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}} { | ||
| {{#jdk8-default-interface}}default {{/jdk8-default-interface}} {{>responseType}} {{operationId}}({{#allParams}}{{^isFile}}{{^isBodyParam}}{{>optionalDataType}}{{/isBodyParam}}{{#isBodyParam}}{{^reactive}}{{>optionalDataType}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}}{{/isBodyParam}}{{/isFile}}{{#isFile}}{{#reactive}}{{#isArray}}Flux<{{/isArray}}Part{{#isArray}}>{{/isArray}}{{/reactive}}{{^reactive}}{{#isArray}}List<{{/isArray}}MultipartFile{{#isArray}}>{{/isArray}}{{/reactive}}{{/isFile}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#includeHttpRequestContext}}{{#hasParams}}, {{/hasParams}}{{#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}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}} { |
Contributor
There was a problem hiding this comment.
P2: Missing comma in paginated signatures when includeHttpRequestContext is enabled for blocking mode (reactive=false) and hasParams=false, causing generated Java syntax 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 291:
<comment>Missing comma in paginated signatures when includeHttpRequestContext is enabled for blocking mode (reactive=false) and hasParams=false, causing generated Java syntax errors.</comment>
<file context>
@@ -270,24 +277,24 @@ public interface {{classname}} {
// Override this method
- {{#jdk8-default-interface}}default {{/jdk8-default-interface}} {{>responseType}} {{operationId}}({{#allParams}}{{^isFile}}{{^isBodyParam}}{{>optionalDataType}}{{/isBodyParam}}{{#isBodyParam}}{{^reactive}}{{>optionalDataType}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}}{{/isBodyParam}}{{/isFile}}{{#isFile}}{{#reactive}}{{#isArray}}Flux<{{/isArray}}Part{{#isArray}}>{{/isArray}}{{/reactive}}{{^reactive}}{{#isArray}}List<{{/isArray}}MultipartFile{{#isArray}}>{{/isArray}}{{/reactive}}{{/isFile}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, {{/hasParams}}{{#springFoxDocumentationProvider}}@ApiIgnore{{/springFoxDocumentationProvider}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}{{#springFoxDocumentationProvider}}@ApiIgnore{{/springFoxDocumentationProvider}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}} {
+ {{#jdk8-default-interface}}default {{/jdk8-default-interface}} {{>responseType}} {{operationId}}({{#allParams}}{{^isFile}}{{^isBodyParam}}{{>optionalDataType}}{{/isBodyParam}}{{#isBodyParam}}{{^reactive}}{{>optionalDataType}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}}{{/isBodyParam}}{{/isFile}}{{#isFile}}{{#reactive}}{{#isArray}}Flux<{{/isArray}}Part{{#isArray}}>{{/isArray}}{{/reactive}}{{^reactive}}{{#isArray}}List<{{/isArray}}MultipartFile{{#isArray}}>{{/isArray}}{{/reactive}}{{/isFile}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#includeHttpRequestContext}}{{#hasParams}}, {{/hasParams}}{{#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}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}} {
{{/delegate-method}}
{{^isDelegate}}
</file context>
Contributor
Author
There was a problem hiding this comment.
with
includeHttpRequestContext: "true"
reactive: "false"
and no params:
/pet/findByStatus:
get:
tags:
- pet
summary: Finds Pets by status
x-spring-paginated: true
description: Multiple status values can be provided with comma separated strings
operationId: findPetsByStatus
responses:
"200":
description: successful operation
content:
application/xml:
schema:
type: array
items:
$ref: "#/components/schemas/Pet"
application/json:
schema:
type: array
items:
$ref: "#/components/schemas/Pet"
"400":
description: Invalid status value
security:
- petstore_auth:
- write:pets
- read:pets
it generates:
default ResponseEntity<List<PetDto>> findPetsByStatus(
@ApiIgnore final HttpServletRequest servletRequest,@ApiIgnore final Pageable pageable
) {
that looks ok to me
...ttp-request-context/src/main/java/org/openapitools/configuration/SpringDocConfiguration.java
Show resolved
Hide resolved
| {{/examples}} | ||
| {{^examples}} | ||
| {{/examples}}{{/includeHttpRequestContext}} | ||
| {{^examples}}{{#includeHttpRequestContext}} |
Contributor
There was a problem hiding this comment.
P2: Reactive stubs lose the NOT_IMPLEMENTED status when includeHttpRequestContext is false, so unimplemented endpoints return 200 OK instead of 501.
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/methodBody.mustache, line 52:
<comment>Reactive stubs lose the NOT_IMPLEMENTED status when includeHttpRequestContext is false, so unimplemented endpoints return 200 OK instead of 501.</comment>
<file context>
@@ -48,14 +48,14 @@ Mono<Void> result = Mono.empty();
- {{/examples}}
-{{^examples}}
+ {{/examples}}{{/includeHttpRequestContext}}
+{{^examples}}{{#includeHttpRequestContext}}
exchange.getResponse().setStatusCode({{#returnSuccessCode}}HttpStatus.OK{{/returnSuccessCode}}{{^returnSuccessCode}}HttpStatus.NOT_IMPLEMENTED{{/returnSuccessCode}});
-{{/examples}}
</file context>
Contributor
Author
There was a problem hiding this comment.
yes - I think this is not an issue. The default behavior is as before and returning 501 without the exchange object would be difficult, no?
...tore/springboot-include-http-request-context/src/main/java/org/openapitools/api/FakeApi.java
Show resolved
Hide resolved
...tore/springboot-include-http-request-context/src/main/java/org/openapitools/api/FakeApi.java
Show resolved
Hide resolved
...ingboot-3-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java
Show resolved
Hide resolved
Contributor
Author
|
@cubic-dev-ai please rerun the review. Take into consideration that many of the files are simply generated and thus if they have some inherent issues (besides failing to compile of course), then they should be solved in a separate future PR |
Contributor
@Picazsoo I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
10 issues found across 132 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="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/configuration/SpringFoxConfiguration.java">
<violation number="1" location="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/configuration/SpringFoxConfiguration.java:67">
P2: Unescaped basePath is used as a regex in replaceFirst, so regex metacharacters in the configured base path can strip the wrong prefix from operation paths.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/Category.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/Category.java:62">
P2: JAXB uses FIELD access here, so the `id` field is already bound. Annotating the getter with @XmlElement creates a second mapping for the same XML element name, which triggers duplicate-property IllegalAnnotationsException when JAXB is used.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/User.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/User.java:276">
P2: toString() includes the plaintext password field, which risks exposing credentials in logs or error messages.</violation>
</file>
<file name="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java">
<violation number="1" location="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java:36">
P3: clone() returns this, violating the DateFormat/Cloneable contract and preventing callers from obtaining an independent formatter instance.</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: Delegate method signature now requires HttpServletRequest/ServerWebExchange when includeHttpRequestContext is enabled, but apiController.mustache still calls delegate.operationId(...) without passing this new parameter, causing a compile-time mismatch.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/JavaSpring/methodBody.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/JavaSpring/methodBody.mustache:52">
P2: When includeHttpRequestContext is false, the template removes the only status-setting logic and still returns Mono.empty(), which Spring WebFlux treats as a 200 OK response. This changes unimplemented reactive stubs from 501 Not Implemented to 200 OK.</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:297">
P2: Pagination comma logic still depends on `reactive` instead of `includeHttpRequestContext`, which can generate invalid method signatures/calls when pagination is enabled with no other params.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/configuration/SpringDocConfiguration.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/configuration/SpringDocConfiguration.java:33">
P2: OAuth2 security schemes require a `flows` object; defining `petstore_auth` with only `.type(OAUTH2)` produces an invalid OpenAPI definition and OAuth2 auth won’t work in Swagger UI.</violation>
</file>
<file name="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/model/ContainerDefaultValueDto.java">
<violation number="1" location="samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/model/ContainerDefaultValueDto.java:102">
P2: @NotNull on a JsonNullable wrapper initialized with JsonNullable.undefined() won’t fail when the field is missing, so the required nullable field is not actually validated for presence.</violation>
</file>
<file name="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java">
<violation number="1" location="samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java:36">
P2: DateFormat.clone() returns `this`, so Jackson’s per-thread cloning still shares a single formatter instance, risking concurrency issues and violating the expected clone contract.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| public String getOperationPath(String operationPath) { | ||
| UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromPath("/"); | ||
| return Paths.removeAdjacentForwardSlashes( | ||
| uriComponentsBuilder.path(operationPath.replaceFirst("^" + basePath, "")).build().toString()); |
Contributor
There was a problem hiding this comment.
P2: Unescaped basePath is used as a regex in replaceFirst, so regex metacharacters in the configured base path can strip the wrong prefix from operation paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/configuration/SpringFoxConfiguration.java, line 67:
<comment>Unescaped basePath is used as a regex in replaceFirst, so regex metacharacters in the configured base path can strip the wrong prefix from operation paths.</comment>
<file context>
@@ -0,0 +1,71 @@
+ public String getOperationPath(String operationPath) {
+ UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromPath("/");
+ return Paths.removeAdjacentForwardSlashes(
+ uriComponentsBuilder.path(operationPath.replaceFirst("^" + basePath, "")).build().toString());
+ }
+ }
</file context>
| @Schema(name = "id", requiredMode = Schema.RequiredMode.NOT_REQUIRED) | ||
| @JsonProperty("id") | ||
| @JacksonXmlProperty(localName = "id") | ||
| @XmlElement(name = "id") |
Contributor
There was a problem hiding this comment.
P2: JAXB uses FIELD access here, so the id field is already bound. Annotating the getter with @xmlelement creates a second mapping for the same XML element name, which triggers duplicate-property IllegalAnnotationsException when JAXB is used.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/Category.java, line 62:
<comment>JAXB uses FIELD access here, so the `id` field is already bound. Annotating the getter with @XmlElement creates a second mapping for the same XML element name, which triggers duplicate-property IllegalAnnotationsException when JAXB is used.</comment>
<file context>
@@ -0,0 +1,196 @@
+ @Schema(name = "id", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
+ @JsonProperty("id")
+ @JacksonXmlProperty(localName = "id")
+ @XmlElement(name = "id")
+ public @Nullable Long getId() {
+ return id;
</file context>
| sb.append(" firstName: ").append(toIndentedString(firstName)).append("\n"); | ||
| sb.append(" lastName: ").append(toIndentedString(lastName)).append("\n"); | ||
| sb.append(" email: ").append(toIndentedString(email)).append("\n"); | ||
| sb.append(" password: ").append(toIndentedString(password)).append("\n"); |
Contributor
There was a problem hiding this comment.
P2: toString() includes the plaintext password field, which risks exposing credentials in logs or error messages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/model/User.java, line 276:
<comment>toString() includes the plaintext password field, which risks exposing credentials in logs or error messages.</comment>
<file context>
@@ -0,0 +1,394 @@
+ sb.append(" firstName: ").append(toIndentedString(firstName)).append("\n");
+ sb.append(" lastName: ").append(toIndentedString(lastName)).append("\n");
+ sb.append(" email: ").append(toIndentedString(email)).append("\n");
+ sb.append(" password: ").append(toIndentedString(password)).append("\n");
+ sb.append(" phone: ").append(toIndentedString(phone)).append("\n");
+ sb.append(" userStatus: ").append(toIndentedString(userStatus)).append("\n");
</file context>
| {{/-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}}{{#includeHttpRequestContext}}, {{/includeHttpRequestContext}}{{/hasParams}}final Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}}{{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}} { |
Contributor
There was a problem hiding this comment.
P2: Delegate method signature now requires HttpServletRequest/ServerWebExchange when includeHttpRequestContext is enabled, but apiController.mustache still calls delegate.operationId(...) without passing this new parameter, causing a compile-time mismatch.
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>Delegate method signature now requires HttpServletRequest/ServerWebExchange when includeHttpRequestContext is enabled, but apiController.mustache still calls delegate.operationId(...) without passing this new parameter, causing a compile-time mismatch.</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}}{{#includeHttpRequestContext}}, {{/includeHttpRequestContext}}{{/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>
| {{/examples}} | ||
| {{^examples}} | ||
| {{/examples}}{{/includeHttpRequestContext}} | ||
| {{^examples}}{{#includeHttpRequestContext}} |
Contributor
There was a problem hiding this comment.
P2: When includeHttpRequestContext is false, the template removes the only status-setting logic and still returns Mono.empty(), which Spring WebFlux treats as a 200 OK response. This changes unimplemented reactive stubs from 501 Not Implemented to 200 OK.
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/methodBody.mustache, line 52:
<comment>When includeHttpRequestContext is false, the template removes the only status-setting logic and still returns Mono.empty(), which Spring WebFlux treats as a 200 OK response. This changes unimplemented reactive stubs from 501 Not Implemented to 200 OK.</comment>
<file context>
@@ -48,14 +48,14 @@ Mono<Void> result = Mono.empty();
- {{/examples}}
-{{^examples}}
+ {{/examples}}{{/includeHttpRequestContext}}
+{{^examples}}{{#includeHttpRequestContext}}
exchange.getResponse().setStatusCode({{#returnSuccessCode}}HttpStatus.OK{{/returnSuccessCode}}{{^returnSuccessCode}}HttpStatus.NOT_IMPLEMENTED{{/returnSuccessCode}});
-{{/examples}}
</file context>
| @@ -58,8 +58,15 @@ import org.springframework.web.context.request.NativeWebRequest; | |||
| {{/isDelegate}} | |||
Contributor
There was a problem hiding this comment.
P2: Pagination comma logic still depends on reactive instead of includeHttpRequestContext, which can generate invalid method signatures/calls when pagination is enabled with no other params.
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 297:
<comment>Pagination comma logic still depends on `reactive` instead of `includeHttpRequestContext`, which can generate invalid method signatures/calls when pagination is enabled with no other params.</comment>
<file context>
@@ -270,24 +277,24 @@ public interface {{classname}} {
{{/isDelegate}}
{{#isDelegate}}
- {{^isVoid}}return {{/isVoid}}{{#isVoid}}{{#useResponseEntity}}return {{/useResponseEntity}}{{^useResponseEntity}}{{#reactive}}return {{/reactive}}{{/useResponseEntity}}{{/isVoid}}getDelegate().{{operationId}}({{#allParams}}{{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, {{/hasParams}}exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}pageable{{/vendorExtensions.x-spring-paginated}});
+ {{^isVoid}}return {{/isVoid}}{{#isVoid}}{{#useResponseEntity}}return {{/useResponseEntity}}{{^useResponseEntity}}{{#reactive}}return {{/reactive}}{{/useResponseEntity}}{{/isVoid}}getDelegate().{{operationId}}({{#allParams}}{{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#includeHttpRequestContext}}{{#hasParams}}, {{/hasParams}}{{#reactive}}exchange{{/reactive}}{{^reactive}}servletRequest{{/reactive}}{{/includeHttpRequestContext}}{{#vendorExtensions.x-spring-paginated}}{{#hasParams}}, {{/hasParams}}{{^hasParams}}{{#reactive}}, {{/reactive}}{{/hasParams}}pageable{{/vendorExtensions.x-spring-paginated}});
{{/isDelegate}}
}{{/jdk8-default-interface}}
</file context>
| .components( | ||
| new Components() | ||
| .addSecuritySchemes("petstore_auth", new SecurityScheme() | ||
| .type(SecurityScheme.Type.OAUTH2) |
Contributor
There was a problem hiding this comment.
P2: OAuth2 security schemes require a flows object; defining petstore_auth with only .type(OAUTH2) produces an invalid OpenAPI definition and OAuth2 auth won’t work in Swagger UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/configuration/SpringDocConfiguration.java, line 33:
<comment>OAuth2 security schemes require a `flows` object; defining `petstore_auth` with only `.type(OAUTH2)` produces an invalid OpenAPI definition and OAuth2 auth won’t work in Swagger UI.</comment>
<file context>
@@ -0,0 +1,43 @@
+ .components(
+ new Components()
+ .addSecuritySchemes("petstore_auth", new SecurityScheme()
+ .type(SecurityScheme.Type.OAUTH2)
+ )
+ .addSecuritySchemes("api_key", new SecurityScheme()
</file context>
| * Get nullableRequiredArray | ||
| * @return nullableRequiredArray | ||
| */ | ||
| @NotNull |
Contributor
There was a problem hiding this comment.
P2: @NotNull on a JsonNullable wrapper initialized with JsonNullable.undefined() won’t fail when the field is missing, so the required nullable field is not actually validated for presence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/model/ContainerDefaultValueDto.java, line 102:
<comment>@NotNull on a JsonNullable wrapper initialized with JsonNullable.undefined() won’t fail when the field is missing, so the required nullable field is not actually validated for presence.</comment>
<file context>
@@ -0,0 +1,223 @@
+ * Get nullableRequiredArray
+ * @return nullableRequiredArray
+ */
+ @NotNull
+ @ApiModelProperty(required = true, value = "")
+ @JsonProperty("nullable_required_array")
</file context>
|
|
||
| @Override | ||
| public Object clone() { | ||
| return this; |
Contributor
There was a problem hiding this comment.
P2: DateFormat.clone() returns this, so Jackson’s per-thread cloning still shares a single formatter instance, risking concurrency issues and violating the expected clone contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/server/petstore/springboot-3-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java, line 36:
<comment>DateFormat.clone() returns `this`, so Jackson’s per-thread cloning still shares a single formatter instance, risking concurrency issues and violating the expected clone contract.</comment>
<file context>
@@ -0,0 +1,38 @@
+
+ @Override
+ public Object clone() {
+ return this;
+ }
+}
</file context>
|
|
||
| @Override | ||
| public Object clone() { | ||
| return this; |
Contributor
There was a problem hiding this comment.
P3: clone() returns this, violating the DateFormat/Cloneable contract and preventing callers from obtaining an independent formatter instance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/springboot-include-http-request-context/src/main/java/org/openapitools/RFC3339DateFormat.java, line 36:
<comment>clone() returns this, violating the DateFormat/Cloneable contract and preventing callers from obtaining an independent formatter instance.</comment>
<file context>
@@ -0,0 +1,38 @@
+
+ @Override
+ public Object clone() {
+ return this;
+ }
+}
</file context>
…Context-to-java-spring-codegen
Member
|
https://github.com/OpenAPITools/openapi-generator/actions/runs/21964113022/job/63449205063?pr=22910 did you have a chance to review the build failure? is that related to this change (PR)? |
Contributor
Author
|
Not yet. i will check it. But this one can definitely wait until next release |
Contributor
Author
|
I think it should go after the jackson 3 one. I don't want to cause issues for that one with merge conflicts etc. |
…Context-to-java-spring-codegen
Contributor
Author
I fixed the build failure. There were some old files stuck around that I needed to delete and regenerate. Not sure why it did not pick them up as not up to date. |
…Context-to-java-spring-codegen # Conflicts: # modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java
Contributor
Author
|
I see the jackson3 is merged. I will resolve the merge conflicts, regenerate samples and docs and will push. |
Contributor
Author
|
Hello @wing328 , the tests pass for this one and it is ready. But as I said before - no need to hurry with this for 7.20 if you already want to set some cut-off. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 includeHttpRequestContext to the Spring generator to control passing the HTTP request context into API and delegate methods. Defaults preserve current behavior: reactive includes ServerWebExchange; blocking excludes HttpServletRequest.
New Features
Bug Fixes
Written for commit e69870c. Summary will update on new commits.