[JAVA] [Spring] JSpecify, fix nullable + required field - #24711
Conversation
merge master
# Conflicts: # modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
There was a problem hiding this comment.
6 issues found across 125 files
Prompt for AI agents (unresolved 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/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/FileApi.java">
<violation number="1" location="samples/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/FileApi.java:234">
P2: When the JVM default charset is not UTF-8, this response parser corrupts non-ASCII `FileContent` values before Jackson deserializes them. Decode JSON with `StandardCharsets.UTF_8` explicitly.</violation>
</file>
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java:7186">
P2: These jspecify tests verify that a required non-nullable property (`getRequiredDt`) still gets `@NotNull`, but they never assert that the required+nullable properties (`getStr`/`setStr`) in the new `RequiredAndNullable` model do NOT get `@NotNull` — which is the core behavior this PR changes. The `fileContains` signatures such as `"@Nullable String getStr()"` are substring matches and still pass even if `@NotNull` were emitted as `@NotNull @Nullable String getStr()`, so they give false confidence. Add an explicit negative assertion, e.g. `.assertMethod("getStr").assertMethodAnnotations().doesNotContainWithName("NotNull")` (and likewise for `setStr`), so a regression that re-adds `@NotNull` on required+nullable members fails these tests.</violation>
</file>
<file name="samples/client/petstore/java/restclient-springBoot4-jackson3-jspecify-openapiNullable/src/main/java/org/openapitools/client/model/FileContent.java">
<violation number="1" location="samples/client/petstore/java/restclient-springBoot4-jackson3-jspecify-openapiNullable/src/main/java/org/openapitools/client/model/FileContent.java:92">
P2: The @JsonCreator constructor parameters for the nullable fields `size` and `virusScan` are missing the `@Nullable` annotation, even though the same fields are emitted as `@Nullable` on their getters and on the builder chain-setters (`size(@Nullable Integer size)`, `virusScan(@Nullable VirusScanEnum virusScan)`). The `@JsonCreator` constructor comes from the readOnly-constructor branch of `pojo.mustache` (restclient, lines ~115-124), which still renders `{{{datatypeWithEnum}}}` without the `{{>nullableArgumentWithEnum}}` helper applied to the all-args constructor at line 133. So for this all-readOnly model, every nullable constructor arg lacks `@Nullable`. This contradicts the PR's stated goal (emit jspecify `@Nullable` on nullable constructor args) and is inconsistent with `RequiredAndNullable.java` in the same sample, whose all-args constructor carries `@Nullable`. Update the readOnly constructor rendering to use the nullable args helper too.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/Java/nullableArgument_builder.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/nullableArgument_builder.mustache:1">
P2: With `useJspecify`, this wrapper removes the `@Nullable` emitted for nullable builder arguments, making them unannotated non-null types. Strip annotations only from raw `datatypeWithEnum`, while rendering `nullable_var_annotations` outside `removeAnnotations`.</violation>
</file>
<file name="samples/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/RequiredAndNullableApi.java">
<violation number="1" location="samples/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/RequiredAndNullableApi.java:155">
P2: When a server controls `Content-Disposition`, `prepareDownloadFile` can write outside its temporary directory because it resolves the raw filename. Reduce the filename to a basename before resolving it.</violation>
<violation number="2" location="samples/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/RequiredAndNullableApi.java:234">
P2: On a JVM whose default charset is not UTF-8, non-ASCII JSON response data is decoded incorrectly before Jackson deserializes it. Decode the response body as UTF-8 explicitly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
|
|
||
|
|
||
| String responseBody = new String(localVarResponseBody.readAllBytes()); |
There was a problem hiding this comment.
P2: When the JVM default charset is not UTF-8, this response parser corrupts non-ASCII FileContent values before Jackson deserializes them. Decode JSON with StandardCharsets.UTF_8 explicitly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/FileApi.java, line 234:
<comment>When the JVM default charset is not UTF-8, this response parser corrupts non-ASCII `FileContent` values before Jackson deserializes them. Decode JSON with `StandardCharsets.UTF_8` explicitly.</comment>
<file context>
@@ -217,20 +221,31 @@ public ApiResponse<Void> fileIdGetWithHttpInfo(String id, Map<String, String> he
+
+
+
+ String responseBody = new String(localVarResponseBody.readAllBytes());
+ FileContent responseValue = responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<FileContent>() {});
+
</file context>
| String responseBody = new String(localVarResponseBody.readAllBytes()); | |
| String responseBody = new String(localVarResponseBody.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8); |
|
|
||
|
|
||
|
|
||
| String responseBody = new String(localVarResponseBody.readAllBytes()); |
There was a problem hiding this comment.
P2: On a JVM whose default charset is not UTF-8, non-ASCII JSON response data is decoded incorrectly before Jackson deserializes it. Decode the response body as UTF-8 explicitly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/RequiredAndNullableApi.java, line 234:
<comment>On a JVM whose default charset is not UTF-8, non-ASCII JSON response data is decoded incorrectly before Jackson deserializes it. Decode the response body as UTF-8 explicitly.</comment>
<file context>
@@ -0,0 +1,289 @@
+
+
+
+ String responseBody = new String(localVarResponseBody.readAllBytes());
+ RequiredAndNullable responseValue = responseBody.isBlank()? null: memberVarObjectMapper.readValue(responseBody, new TypeReference<RequiredAndNullable>() {});
+
</file context>
| String responseBody = new String(localVarResponseBody.readAllBytes()); | |
| String responseBody = new String(localVarResponseBody.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8); |
| File file = null; | ||
| if (filename != null) { | ||
| java.nio.file.Path tempDir = java.nio.file.Files.createTempDirectory("swagger-gen-native"); | ||
| java.nio.file.Path filePath = java.nio.file.Files.createFile(tempDir.resolve(filename)); |
There was a problem hiding this comment.
P2: When a server controls Content-Disposition, prepareDownloadFile can write outside its temporary directory because it resolves the raw filename. Reduce the filename to a basename before resolving it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/java/native-jackson3-jspecify/src/main/java/org/openapitools/client/api/RequiredAndNullableApi.java, line 155:
<comment>When a server controls `Content-Disposition`, `prepareDownloadFile` can write outside its temporary directory because it resolves the raw filename. Reduce the filename to a basename before resolving it.</comment>
<file context>
@@ -0,0 +1,289 @@
+ File file = null;
+ if (filename != null) {
+ java.nio.file.Path tempDir = java.nio.file.Files.createTempDirectory("swagger-gen-native");
+ java.nio.file.Path filePath = java.nio.file.Files.createFile(tempDir.resolve(filename));
+ file = filePath.toFile();
+ tempDir.toFile().deleteOnExit(); // best effort cleanup
</file context>
| java.nio.file.Path filePath = java.nio.file.Files.createFile(tempDir.resolve(filename)); | |
| java.nio.file.Path filePath = java.nio.file.Files.createFile(tempDir.resolve(java.nio.file.Path.of(filename).getFileName())); |
| ).fileDoesNotContain( | ||
| "javax.annotation.Nullable", | ||
| "jakarta.annotation.Nullable") | ||
| .assertMethod("getRequiredDt").assertMethodAnnotations().containsWithName("NotNull").containsWithName("Valid"); |
There was a problem hiding this comment.
P2: These jspecify tests verify that a required non-nullable property (getRequiredDt) still gets @NotNull, but they never assert that the required+nullable properties (getStr/setStr) in the new RequiredAndNullable model do NOT get @NotNull — which is the core behavior this PR changes. The fileContains signatures such as "@Nullable String getStr()" are substring matches and still pass even if @NotNull were emitted as @NotNull @Nullable String getStr(), so they give false confidence. Add an explicit negative assertion, e.g. .assertMethod("getStr").assertMethodAnnotations().doesNotContainWithName("NotNull") (and likewise for setStr), so a regression that re-adds @NotNull on required+nullable members fails these tests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java, line 7186:
<comment>These jspecify tests verify that a required non-nullable property (`getRequiredDt`) still gets `@NotNull`, but they never assert that the required+nullable properties (`getStr`/`setStr`) in the new `RequiredAndNullable` model do NOT get `@NotNull` — which is the core behavior this PR changes. The `fileContains` signatures such as `"@Nullable String getStr()"` are substring matches and still pass even if `@NotNull` were emitted as `@NotNull @Nullable String getStr()`, so they give false confidence. Add an explicit negative assertion, e.g. `.assertMethod("getStr").assertMethodAnnotations().doesNotContainWithName("NotNull")` (and likewise for `setStr`), so a regression that re-adds `@NotNull` on required+nullable members fails these tests.</comment>
<file context>
@@ -7179,7 +7180,10 @@ public void testJspecify(String library, int springBootVersion) throws IOExcepti
+ ).fileDoesNotContain(
+ "javax.annotation.Nullable",
+ "jakarta.annotation.Nullable")
+ .assertMethod("getRequiredDt").assertMethodAnnotations().containsWithName("NotNull").containsWithName("Valid");
JavaFileAssert.assertThat(files.get("FooApi.java"))
.assertTypeAnnotations().doesImportAnnotation("org.jspecify.annotations.Nullable").toType()
</file context>
There was a problem hiding this comment.
All reported issues were addressed across 27 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Make JSonNullable<> field = null for nullable+required
There was a problem hiding this comment.
All reported issues were addressed across 197 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
fix #24686
For a type:
Ensure that the following json are accepted:
Keep jspecify
@Nullableannotation for builder with an improved RemoveAnnotationLambda. Other annotations like@Validare still removed.Ensure setter, chain setters, builder, constructors contain the correct JSpecify
@Nullableannotation.Add more combination of readonly, nullable and required in the samples.
For nullable attributes:
Update
@Schemaannotation to addnullable=trueFor nullable container attributes + openapiNullable
Fix a potential NullPointerException in add and put item. For example
new Foo()._list(null).addListItem("item")For required+nullable attributes:
java
@NullableannotationSpring
@NotNull. The bean validation has no way to find out if the value is absent.@NotNull. This is a not intuitive. It should be possible to distinguish between null and absent. This is not the case of the Spring bean validation. It usesJsonNullableValueExtractororJsonNullableJakartaValueExtractorto extract the value. So@NotNullfails for null and for absent.Workaround: use an annotation
@Presentwith a validator like the following. (it might be a nice addition to the jackson-databind-nullable project)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.
Summary by cubic
Aligns Java and Spring generators with
jspecifyfor required+nullable fields and corrects Bean Validation. Previously Spring emitted@NotNullfor required+nullable; now generators annotate nullability consistently and stop emitting@NotNullfor these cases, and models include@Schema(nullable = true).nullable_var_annotations.mustache; exposeremoveAnnotationslambda; compute the nullable annotation fromjavaxPackagesojavax.annotation.Nullableis not emitted underjspecify.@NotNulltonotNull.mustache; emit only when “required AND not readOnly AND not nullable”. WithopenApiNullable=true, required+nullableJsonNullable<T>fields still initialize tonull.@Schema(..., nullable = true)across Java library templates.RequiredAndNullableschema and endpoints; assert constructor/chain-setter@Nullableand absence ofjavax.annotation.Nullableunderjspecify; changeFileApito returnFileContentwithAccept: application/json.Migration
@NotNullfor required+nullable, including withopenApiNullable. If you relied on Bean Validation to reject nulls, add explicit constraints or mark fields non-nullable.openApiNullableis enabled, required+nullableJsonNullable<T>fields default tonull. If you depended onundefined(), update handling accordingly.Written for commit b3f6f5e. Summary will update on new commits.