-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
[java] [Spring] useJspecify for java clients and spring generator #23256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
7bd73f8
603af01
7521f10
1cee79c
9af4802
4aae7ce
8023714
7c4b50b
cca147d
c40a345
fffe2e9
96d0f36
4f2fdbc
b1148fb
b4a4a66
f8e0065
ad2359a
b2f8918
8f66a71
1540fcb
cf6b820
bde60be
22abaf7
25cfde3
385c2d9
83e040b
128beb1
d2faaac
15f2b5d
25b782a
6050184
c940aef
fbc576d
ccf5c09
9e98c52
2c4bfd4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| generatorName: java | ||
| outputDir: samples/client/petstore/java/native-jackson3-jspecify | ||
| library: native | ||
| inputSpec: modules/openapi-generator/src/test/resources/3_0/java/jspecify.yaml | ||
| templateDir: modules/openapi-generator/src/main/resources/Java | ||
| validateSpec: false | ||
| additionalProperties: | ||
| artifactId: petstore-native-jackson3 | ||
| hideGenerationTimestamp: "true" | ||
| generateBuilders: true | ||
| useReflectionEqualsHashCode: "true" | ||
| useJackson3: "true" | ||
| openApiNullable: "false" | ||
| useJspecify: true | ||
| typeMappings: | ||
| OffsetDateTime: java.time.Instant | ||
| BigDecimal: java.math.BigDecimal | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| generatorName: java | ||
| outputDir: samples/client/petstore/java/restclient-springBoot4-jackson3-jspecify | ||
| library: restclient | ||
| inputSpec: modules/openapi-generator/src/test/resources/3_0/java/jspecify.yaml | ||
| templateDir: modules/openapi-generator/src/main/resources/Java | ||
| validateSpec: false | ||
| additionalProperties: | ||
| artifactId: petstore-restclient | ||
| hideGenerationTimestamp: "true" | ||
| containerDefaultToNull: "true" | ||
| useSpringBoot4: true | ||
| useJackson3: true | ||
| openApiNullable: false | ||
| useJspecify: true | ||
| typeMappings: | ||
| OffsetDateTime: java.time.Instant | ||
| BigDecimal: java.math.BigDecimal |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| generatorName: java | ||
| outputDir: samples/client/petstore/java/resttemplate-springBoot4-jackson3-jspecify | ||
| library: resttemplate | ||
| inputSpec: modules/openapi-generator/src/test/resources/3_0/java/jspecify.yaml | ||
| templateDir: modules/openapi-generator/src/main/resources/Java | ||
| validateSpec: false | ||
| additionalProperties: | ||
| artifactId: petstore-resttemplate | ||
| hideGenerationTimestamp: "true" | ||
| containerDefaultToNull: "true" | ||
| useJakartaEe: true | ||
| useSpringBoot4: true | ||
| useJackson3: true | ||
| openApiNullable: false | ||
| useJspecify: true | ||
| typeMappings: | ||
| OffsetDateTime: java.time.Instant | ||
| BigDecimal: java.math.BigDecimal |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| generatorName: java | ||
| outputDir: samples/client/petstore/java/webclient-springBoot4-jackson3-jspecify | ||
| library: webclient | ||
| inputSpec: modules/openapi-generator/src/test/resources/3_0/java/jspecify.yaml | ||
| templateDir: modules/openapi-generator/src/main/resources/Java | ||
| validateSpec: false | ||
| additionalProperties: | ||
| artifactId: petstore-webclient | ||
| hideGenerationTimestamp: "true" | ||
| containerDefaultToNull: "true" | ||
| useSpringBoot4: true | ||
| useJackson3: true | ||
| openApiNullable: false | ||
| useJspecify: true | ||
| typeMappings: | ||
| OffsetDateTime: java.time.Instant | ||
| BigDecimal: java.math.BigDecimal |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| generatorName: spring | ||
| library: spring-boot | ||
| outputDir: samples/openapi3/server/petstore/springboot-4-jspecify | ||
| inputSpec: modules/openapi-generator/src/test/resources/3_0/java/jspecify.yaml | ||
| templateDir: modules/openapi-generator/src/main/resources/JavaSpring | ||
| validateSpec: false | ||
| additionalProperties: | ||
| groupId: org.openapitools.openapi3 | ||
| documentationProvider: springdoc | ||
| interfaceOnly: true | ||
| artifactId: springboot | ||
| snapshotVersion: "true" | ||
| useSpringBoot4: true | ||
| useJackson3: true | ||
| useBeanValidation: true | ||
| withXml: true | ||
| hideGenerationTimestamp: "true" | ||
| generateConstructorWithAllArgs: true | ||
| generateBuilders: true | ||
| openApiNullable: false | ||
| useJspecify: true |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -22,8 +22,10 @@ | |||||
| import com.fasterxml.jackson.databind.node.ArrayNode; | ||||||
| import com.fasterxml.jackson.databind.node.ObjectNode; | ||||||
| import com.google.common.base.Strings; | ||||||
| import com.google.common.collect.ImmutableMap; | ||||||
| import com.google.common.collect.Sets; | ||||||
| import com.samskivert.mustache.Mustache; | ||||||
| import com.samskivert.mustache.Template; | ||||||
| import io.swagger.v3.oas.models.OpenAPI; | ||||||
| import io.swagger.v3.oas.models.Operation; | ||||||
| import io.swagger.v3.oas.models.PathItem; | ||||||
|
|
@@ -58,6 +60,8 @@ | |||||
| import javax.lang.model.SourceVersion; | ||||||
|
|
||||||
| import java.io.File; | ||||||
| import java.io.IOException; | ||||||
| import java.io.Writer; | ||||||
| import java.time.LocalDate; | ||||||
| import java.time.ZoneId; | ||||||
| import java.time.ZonedDateTime; | ||||||
|
|
@@ -104,6 +108,7 @@ public abstract class AbstractJavaCodegen extends DefaultCodegen implements Code | |||||
| public static final String IMPLICIT_HEADERS_REGEX = "implicitHeadersRegex"; | ||||||
| public static final String JAVAX_PACKAGE = "javaxPackage"; | ||||||
| public static final String USE_JAKARTA_EE = "useJakartaEe"; | ||||||
| public static final String USE_JSPECIFY = "useJspecify"; | ||||||
| public static final String CONTAINER_DEFAULT_TO_NULL = "containerDefaultToNull"; | ||||||
| public static final String DISABLE_DISCRIMINATOR_JSON_IGNORE_PROPERTIES = "disableDiscriminatorJsonIgnoreProperties"; | ||||||
|
|
||||||
|
|
@@ -216,6 +221,11 @@ protected enum ENUM_PROPERTY_NAMING_TYPE {MACRO_CASE, legacy, original} | |||||
| */ | ||||||
| @Getter @Setter | ||||||
| protected boolean useBeanValidation = false; | ||||||
| @Getter | ||||||
| @Setter | ||||||
| protected boolean useJspecify; | ||||||
| protected JSpecifyNullableLambda jSpecifyNullableLambda; | ||||||
|
|
||||||
| private Map<String, String> schemaKeyToModelNameCache = new HashMap<>(); | ||||||
|
|
||||||
| public AbstractJavaCodegen() { | ||||||
|
|
@@ -597,6 +607,7 @@ public void processOpts() { | |||||
| convertPropertyToBooleanAndWriteBack(CAMEL_CASE_DOLLAR_SIGN, this::setCamelCaseDollarSign); | ||||||
| convertPropertyToBooleanAndWriteBack(USE_ONE_OF_INTERFACES, this::setUseOneOfInterfaces); | ||||||
| convertPropertyToStringAndWriteBack(CodegenConstants.ENUM_PROPERTY_NAMING, this::setEnumPropertyNaming); | ||||||
| convertPropertyToBooleanAndWriteBack(USE_JSPECIFY, this::setUseJspecify); | ||||||
|
|
||||||
| if (!StringUtils.isEmpty(parentGroupId) && !StringUtils.isEmpty(parentArtifactId) && !StringUtils.isEmpty(parentVersion)) { | ||||||
| additionalProperties.put("parentOverridden", true); | ||||||
|
|
@@ -841,10 +852,19 @@ private void sanitizeConfig() { | |||||
|
|
||||||
| protected void applyJavaxPackage() { | ||||||
| writePropertyBack(JAVAX_PACKAGE, "javax"); | ||||||
| writePropertyBack("nullableAnnotation", "@javax.annotation.Nullable"); | ||||||
| writePropertyBack("nonNullAnnotation", "@javax.annotation.Nonnull"); | ||||||
| } | ||||||
|
|
||||||
| protected void applyJakartaPackage() { | ||||||
| writePropertyBack(JAVAX_PACKAGE, "jakarta"); | ||||||
| writePropertyBack("nullableAnnotation", "@jakarta.annotation.Nullable"); | ||||||
| writePropertyBack("nonNullAnnotation", "@jakarta.annotation.Nonnull"); | ||||||
| } | ||||||
|
|
||||||
| protected void applyJspecify() { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to set the JAVAX_PACKAGE to jspecify?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, obsolete commit, I've removed it @Chrimle thanks for the review
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, so there's no need for something like Resolve if no concern 👍 |
||||||
| writePropertyBack("nullableAnnotation", "@org.jspecify.annotations.Nullable"); | ||||||
| writePropertyBack("nonNullAnnotation", "@NonNull"); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for fully-qualified name?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oups, left-over from an initial version. I clean the code....
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done ✅ |
||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
|
|
@@ -2653,4 +2673,82 @@ public void setEnumPropertyNaming(final String enumPropertyNamingType) { | |||||
| throw new RuntimeException(sb.toString()); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| protected ImmutableMap.Builder<String, Mustache.Lambda> addMustacheLambdas() { | ||||||
| this.jSpecifyNullableLambda = new JSpecifyNullableLambda(); | ||||||
| return super.addMustacheLambdas() | ||||||
| .put("jSpecifyDatatype",(fragment, writer) -> { | ||||||
| String dataType = fragment.execute(); | ||||||
| if (jSpecifyNullableLambda.keptNullable) { | ||||||
| jSpecifyNullableLambda.keptNullable = false; | ||||||
| int idx = dataType.lastIndexOf('.'); | ||||||
| if (idx > 0) { | ||||||
| // generate declareation like java.time.@Nullable Timestamp | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done ✅ |
||||||
| writer.write(dataType.substring(0, idx + 1)); | ||||||
| writer.write("@Nullable "); | ||||||
| writer.write(dataType.substring(idx + 1)); | ||||||
| } else { | ||||||
| writer.write("@Nullable "); | ||||||
| writer.write(dataType); | ||||||
| } | ||||||
| } else { | ||||||
| writer.write(dataType); | ||||||
| } | ||||||
| }) | ||||||
| .put("jSpecifyNullable", jSpecifyNullableLambda); | ||||||
|
|
||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * for Jspecify, remove @Nullable before the datatype. | ||||||
| */ | ||||||
| class JSpecifyNullableLambda implements Mustache.Lambda { | ||||||
| private String nullableAnnotation = "@Nullable"; | ||||||
| // remember if @Nullable is needed | ||||||
| boolean keptNullable = false; | ||||||
|
|
||||||
| public void setNullableAnnotation(String nullableAnnotation) { | ||||||
| this.nullableAnnotation = nullableAnnotation; | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public void execute(Template.Fragment fragment, Writer writer) throws IOException { | ||||||
| keptNullable = false; | ||||||
| String value = fragment.execute(); | ||||||
| if (useJspecify) { | ||||||
| if (value.startsWith(nullableAnnotation)) { | ||||||
| keptNullable = true; | ||||||
| int idx = nullableAnnotation.length(); | ||||||
| // trim left | ||||||
| while (idx < value.length() && value.charAt(idx) == ' ') { | ||||||
| idx ++; | ||||||
| } | ||||||
| value = value.substring(idx); | ||||||
| } | ||||||
| } | ||||||
| writer.write(value); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| protected void addPackagInfoSupportingFiles() { | ||||||
| if (useJspecify) { | ||||||
| supportingFiles.add(new SupportingFile("modelPackageInfo.mustache", | ||||||
| (sourceFolder + File.separator + modelPackage).replace(".", java.io.File.separator), | ||||||
| "package-info.java")); | ||||||
| supportingFiles.add(new SupportingFile("apiPackageInfo.mustache", | ||||||
| (sourceFolder + File.separator + apiPackage).replace(".", java.io.File.separator), | ||||||
| "package-info.java")); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Adds Nullable import if any parameter is nullable or optional. | ||||||
| */ | ||||||
| protected void addNullableImportForOperation(CodegenOperation codegenOperation) { | ||||||
| codegenOperation.allParams.stream() | ||||||
| .filter(CodegenParameter::notRequiredOrIsNullable) | ||||||
| .findAny() | ||||||
| .ifPresent(param -> codegenOperation.imports.add("Nullable")); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
|
|
||
| import io.swagger.v3.oas.models.Operation; | ||
| import io.swagger.v3.oas.models.media.Schema; | ||
| import io.swagger.v3.oas.models.servers.Server; | ||
| import lombok.AccessLevel; | ||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
|
|
@@ -47,7 +48,6 @@ | |
| import static com.google.common.base.CaseFormat.LOWER_CAMEL; | ||
| import static com.google.common.base.CaseFormat.UPPER_UNDERSCORE; | ||
| import static java.util.Collections.sort; | ||
| import static org.openapitools.codegen.CodegenConstants.SERIALIZATION_LIBRARY; | ||
| import static org.openapitools.codegen.CodegenConstants.X_IMPLEMENTS; | ||
| import static org.openapitools.codegen.utils.CamelizeOption.LOWERCASE_FIRST_LETTER; | ||
| import static org.openapitools.codegen.utils.StringUtils.camelize; | ||
|
|
@@ -176,6 +176,7 @@ public class JavaClientCodegen extends AbstractJavaCodegen | |
|
|
||
| @Setter protected int maxAttemptsForRetry = 1; | ||
| @Setter protected long waitTimeMillis = 10l; | ||
| private final Set<String> JSPECIFY_SUPPORTED_LIBRARIES = new TreeSet<>(Arrays.asList(RESTCLIENT, WEBCLIENT, NATIVE, RESTTEMPLATE)); | ||
|
|
||
| private static class MpRestClientVersion { | ||
| public final String rootPackage; | ||
|
|
@@ -274,6 +275,7 @@ public JavaClientCodegen() { | |
| cliOptions.add(CliOption.newBoolean(SUPPORT_VERTX_FUTURE, "Also generate api methods that return a vertx Future instead of taking a callback. Only `vertx` supports this option. Requires vertx 4 or greater.", this.supportVertxFuture)); | ||
| cliOptions.add(CliOption.newBoolean(USE_SEALED_ONE_OF_INTERFACES, "Generate the oneOf interfaces as sealed interfaces. Only supported for WebClient and RestClient.", this.useSealedOneOfInterfaces)); | ||
| cliOptions.add(CliOption.newBoolean(USE_UNARY_INTERCEPTOR, "If true it will generate ResponseInterceptors using a UnaryOperator. This can be usefull for manipulating the request before it gets passed, for example doing your own decryption", this.useUnaryInterceptor)); | ||
| cliOptions.add(CliOption.newBoolean(USE_JSPECIFY, "Use Jspecify for null checks. Ony supported for " + JSPECIFY_SUPPORTED_LIBRARIES, useJspecify)); | ||
|
|
||
| supportedLibraries.put(JERSEY2, "HTTP client: Jersey client 2.25.1. JSON processing: Jackson 2.17.1"); | ||
| supportedLibraries.put(JERSEY3, "HTTP client: Jersey client 3.1.1. JSON processing: Jackson 2.17.1"); | ||
|
|
@@ -322,6 +324,15 @@ private void initMpRestClientVersionToRootPackage() { | |
| mpRestClientVersions.put("3.0", new MpRestClientVersion("jakarta", "pom_3.0.mustache")); | ||
| } | ||
|
|
||
| @Override | ||
| public void setUseJspecify(boolean useJspecify) { | ||
| // currently only available for a limited set of libraries | ||
| if (useJspecify && !JSPECIFY_SUPPORTED_LIBRARIES.contains(library)) { | ||
| throw new IllegalArgumentException("useJspecify is only suppored in these libraries: " + JSPECIFY_SUPPORTED_LIBRARIES); | ||
| } | ||
| super.setUseJspecify(useJspecify); | ||
| } | ||
|
|
||
| @Override | ||
| public CodegenType getTag() { | ||
| return CodegenType.CLIENT; | ||
|
|
@@ -788,6 +799,9 @@ public void processOpts() { | |
| } else { | ||
| LOGGER.error("Unknown library option (-l/--library): {}", getLibrary()); | ||
| } | ||
| if (useJspecify) { | ||
| applyJspecify(); | ||
| } | ||
|
|
||
| if (usePlayWS) { | ||
| // remove unsupported auth | ||
|
|
@@ -1006,6 +1020,15 @@ public int compare(CodegenParameter one, CodegenParameter another) { | |
| return objs; | ||
| } | ||
|
|
||
| @Override | ||
| public CodegenOperation fromOperation(String path, String httpMethod, Operation operation, List<Server> servers) { | ||
| CodegenOperation op = super.fromOperation(path, httpMethod, operation, servers); | ||
| if (useJspecify) { | ||
| addNullableImportForOperation(op); | ||
| } | ||
| return op; | ||
| } | ||
|
|
||
| @Override | ||
| public String apiFilename(String templateName, String tag) { | ||
| if (isLibrary(VERTX)) { | ||
|
|
@@ -1136,6 +1159,9 @@ public CodegenModel fromModel(String name, Schema model) { | |
| if (!AnnotationLibrary.SWAGGER2.equals(getAnnotationLibrary())) { | ||
| codegenModel.imports.remove("Schema"); | ||
| } | ||
| if (useJspecify) { | ||
| codegenModel.imports.add("Nullable"); | ||
| } | ||
|
|
||
| return codegenModel; | ||
| } | ||
|
|
@@ -1345,4 +1371,22 @@ public List<VendorExtension> getSupportedVendorExtensions() { | |
| extensions.add(VendorExtension.X_WEBCLIENT_BLOCKING); | ||
| return extensions; | ||
| } | ||
|
|
||
| @Override | ||
| protected void applyJspecify() { | ||
| super.applyJspecify(); | ||
| addPackagInfoSupportingFiles(); | ||
| importMapping.put("Nullable", "org.jspecify.annotations.Nullable"); | ||
| jSpecifyNullableLambda.setNullableAnnotation("@" + additionalProperties.get(JAVAX_PACKAGE) + ".annotation.Nullable"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if I understand why javax/jakarta should be used here? Because I did not see that we set this JAVAX_PACKAGE to be jspecify... but maybe I missed something
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nullable_var_annotations.mustache uses comment added |
||
| } | ||
|
|
||
| @Override | ||
| protected void addPackagInfoSupportingFiles() { | ||
| super.addPackagInfoSupportingFiles(); | ||
| if (useJspecify) { | ||
| supportingFiles.add(new SupportingFile("invokerPackageInfo.mustache", | ||
| (sourceFolder + File.separator + invokerPackage).replace(".", java.io.File.separator), | ||
| "package-info.java")); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as usual, what about adding the new sample folder to the github workflow(s)?