Skip to content
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.

Sign up for GitHub

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

[Java] Add a new additional property to configure Jackson's failOnUnknownProperties #19271

Merged
merged 5 commits into from
Aug 29, 2024
Merged

[Java] Add a new additional property to configure Jackson's failOnUnknownProperties #19271

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6f8f318
[Java] Add a new additional property to configure Jackson's `failOnUn…
fistons Jul 30, 2024
1cf8590
Move `FAIL_ON_UNKNOWN_PROPERTIES` property to JavaClientCodegen
fistons Aug 4, 2024
38a15fb
Template `FAIL_ON_UNKNOWN_PROPERTIES` for other libraries beside retr…
fistons Aug 5, 2024
a20b4bf
Default `failOnUnknownProperties` to false for all Java Client libraries
fistons Aug 6, 2024
36f76f2
Fix integration tests
fistons Aug 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/generators/java-microprofile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
|enumUnknownDefaultCase|If the server adds new enum cases, that are unknown by an old spec/client, the client will fail to parse the network response.With this option enabled, each enum will have a new case, 'unknown_default_open_api', so that when the server sends an enum case that is not known by the client/spec, they can safely fallback to this case.|<dl><dt>**false**</dt><dd>No changes to the enum's are made, this is the default option.</dd><dt>**true**</dt><dd>With this option enabled, each enum will have a new case, 'unknown_default_open_api', so that when the enum case sent by the server is not known by the client/spec, can safely be decoded to this case.</dd></dl>|false|
|errorObjectType|Error Object type. (This option is for okhttp-gson only)| |null|
|failOnUnknownProperties|Fail Jackson de-serialization on unknown properties| |true|
|generateBuilders|Whether to generate builders for models| |false|
|generateClientAsBean|For resttemplate, configure whether to create `ApiClient.java` and Apis clients as bean (with `@Component` annotation).| |false|
|generateConstructorWithAllArgs|whether to generate a constructor for all arguments| |false|
Expand Down
1 change: 1 addition & 0 deletions docs/generators/java.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|ensureUniqueParams|Whether to ensure parameter names are unique in an operation (rename parameters that are not).| |true|
|enumUnknownDefaultCase|If the server adds new enum cases, that are unknown by an old spec/client, the client will fail to parse the network response.With this option enabled, each enum will have a new case, 'unknown_default_open_api', so that when the server sends an enum case that is not known by the client/spec, they can safely fallback to this case.|<dl><dt>**false**</dt><dd>No changes to the enum's are made, this is the default option.</dd><dt>**true**</dt><dd>With this option enabled, each enum will have a new case, 'unknown_default_open_api', so that when the enum case sent by the server is not known by the client/spec, can safely be decoded to this case.</dd></dl>|false|
|errorObjectType|Error Object type. (This option is for okhttp-gson only)| |null|
|failOnUnknownProperties|Fail Jackson de-serialization on unknown properties| |true|
|generateBuilders|Whether to generate builders for models| |false|
|generateClientAsBean|For resttemplate, configure whether to create `ApiClient.java` and Apis clients as bean (with `@Component` annotation).| |false|
|generateConstructorWithAllArgs|whether to generate a constructor for all arguments| |false|
Expand Down
2 changes: 2 additions & 0 deletions ...enapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ public abstract class AbstractJavaCodegen extends DefaultCodegen implements Code
@Setter protected List<String> additionalEnumTypeAnnotations = new LinkedList<>();
@Getter @Setter
protected boolean openApiNullable = true;
@Getter @Setter
protected boolean failOnUnknownProperties = true;
@Setter protected String outputTestFolder = "";
protected DocumentationProvider documentationProvider;
protected AnnotationLibrary annotationLibrary;
Expand Down
3 changes: 3 additions & 0 deletions ...openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public class JavaClientCodegen extends AbstractJavaCodegen
public static final String MICROPROFILE_KUMULUZEE = "kumuluzee";
public static final String WEBCLIENT_BLOCKING_OPERATIONS = "webclientBlockingOperations";
public static final String USE_ENUM_CASE_INSENSITIVE = "useEnumCaseInsensitive";
public static final String FAIL_ON_UNKNOWN_PROPERTIES = "failOnUnknownProperties";

public static final String SERIALIZATION_LIBRARY_GSON = "gson";
public static final String SERIALIZATION_LIBRARY_JACKSON = "jackson";
Expand Down Expand Up @@ -240,6 +241,7 @@ public JavaClientCodegen() {
cliOptions.add(CliOption.newBoolean(GENERATE_CLIENT_AS_BEAN, "For resttemplate, configure whether to create `ApiClient.java` and Apis clients as bean (with `@Component` annotation).", this.generateClientAsBean));
cliOptions.add(CliOption.newBoolean(SUPPORT_URL_QUERY, "Generate toUrlQueryString in POJO (default to true). Available on `native`, `apache-httpclient` libraries."));
cliOptions.add(CliOption.newBoolean(USE_ENUM_CASE_INSENSITIVE, "Use `equalsIgnoreCase` when String for enum comparison", useEnumCaseInsensitive));
cliOptions.add(CliOption.newBoolean(FAIL_ON_UNKNOWN_PROPERTIES, "Fail Jackson de-serialization on unknown properties", this.failOnUnknownProperties));

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");
Expand Down Expand Up @@ -390,6 +392,7 @@ public void processOpts() {
convertPropertyToStringAndWriteBack(GRADLE_PROPERTIES, this::setGradleProperties);
convertPropertyToStringAndWriteBack(ERROR_OBJECT_TYPE, this::setErrorObjectType);
convertPropertyToBooleanAndWriteBack(WEBCLIENT_BLOCKING_OPERATIONS, op -> webclientBlockingOperations=op);
convertPropertyToBooleanAndWriteBack(FAIL_ON_UNKNOWN_PROPERTIES, this::setFailOnUnknownProperties);

// add URL query deepObject support to native, apache-httpclient by default
if (!additionalProperties.containsKey(SUPPORT_URL_QUERY)) {
Expand Down
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/resources/Java/libraries/retrofit2/JSON_jackson.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class JSON implements ContextResolver<ObjectMapper> {
mapper = JsonMapper.builder()
.serializationInclusion(JsonInclude.Include.NON_NULL)
.configure(MapperFeature.ALLOW_COERCION_OF_SCALARS, false)
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, true)
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, {{failOnUnknownProperties}})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, could you please make this change for other libraries besides retrofit2 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, yes, I can do this, the only issue is that some libraries are configured to fail on unknown properties, some aren't.

I see that apache-httpclient, feign, google-api-client, restassured, restclient, resteasy, vertx and webclient currently have DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES set to false, whereas jersey2, jersey3, native and retrofit2 have DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES set to true

Is there an elegant way to tell JavaClientCodegen to default the failOnUnknownProperties variable to true or false, depending of the chosen library?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

My preferred approach would be to use this opportunity to unify the behaviour of the different libraries. Since the majority uses false as default, let's make false the default value of failOnUnknownProperties for all libraries.
This would also make this PR a "breaking change with fallback", i.e. it should become part of OpenAPI Generator 7.8.0, not of 7.7.1. But I think wing328 will take care of this aspect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's done!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Some tests in samples/openapi3/client/petstore/java/jersey2-java8/ are failing now. Apparently due to the custom deserialization logic in this sample, which relies on FAIL_ON_UNKNOWN_PROPERTIES being true. Can you please fix this by simply adding in bin/configs/java-jersey2-8.yaml the following line somewhere under additionalProperties:?

  failOnUnknownProperties: true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Thanks for the hint, I had trouble finding where it came from

.configure(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE, true)
.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.enable(SerializationFeature.WRITE_ENUMS_USING_TO_STRING)
Expand Down
16 changes: 16 additions & 0 deletions .../openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.openapitools.codegen.config.CodegenConfigurator;
import org.openapitools.codegen.java.assertions.JavaFileAssert;
import org.openapitools.codegen.languages.AbstractJavaCodegen;
import org.openapitools.codegen.languages.JavaCXFClientCodegen;
import org.openapitools.codegen.languages.JavaClientCodegen;
import org.openapitools.codegen.languages.features.BeanValidationFeatures;
import org.openapitools.codegen.languages.features.CXFServerFeatures;
Expand Down Expand Up @@ -244,6 +245,21 @@ public void testSettersForConfigValues() throws Exception {
Assertions.assertEquals(codegen.getSerializationLibrary(), JavaClientCodegen.SERIALIZATION_LIBRARY_GSON); // the library JavaClientCodegen.OKHTTP_GSON only supports GSON
}

@Test
public void testFailOnUnknownPropertiesAdditionalProperty() {
final JavaClientCodegen codegen = new JavaClientCodegen();

codegen.processOpts();

ConfigAssert configAssert = new ConfigAssert(codegen.additionalProperties());
configAssert.assertValue(JavaClientCodegen.FAIL_ON_UNKNOWN_PROPERTIES, codegen::isFailOnUnknownProperties, Boolean.TRUE);

codegen.additionalProperties().put(JavaClientCodegen.FAIL_ON_UNKNOWN_PROPERTIES, false);
codegen.processOpts();

configAssert.assertValue(JavaClientCodegen.FAIL_ON_UNKNOWN_PROPERTIES, codegen::isFailOnUnknownProperties, Boolean.FALSE);
}

@Test
public void testAdditionalPropertiesPutForConfigValues() throws Exception {
final JavaClientCodegen codegen = new JavaClientCodegen();
Expand Down