[Java] better default value handling by wing328 · Pull Request #14130 · OpenAPITools/openapi-generator

wing328 · 2022-11-27T10:33:51Z

support default value for array of enum string
use nullable attribute to determine the default value of container (e.g. null vs new ArrayList<>())
add an option containerDefaultToNull (false by default) to allow fallback to previous behaviour
add tests

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
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*.
For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

cc @bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10)

.../openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java

...ore/java/apache-httpclient/src/test/java/org/openapitools/client/model/DefaultValueTest.java

wing328 · 2022-11-28T16:06:36Z

@leonard84 thanks for the feedback. Will definitely add tests before marking this PR ready for review.

…efault

wing328 · 2022-11-30T09:57:53Z

@leonard84 I've pushed some updates and added some tests. Note that there's new project (echo API client) with the newly added tests for default values of array property: https://github.com/OpenAPITools/openapi-generator/pull/14130/files#diff-43b8a464f4da55b9453d495bd235c5091b9a0de7a0d4ab0cfbc9fc66386cbe00

In order for the API tests to pass, you will need to run the following locally:

git clone https://github.com/wing328/http-echo-server -b openapi-generator-test-server
cd http-echo-server
npm install && npm start

Please kindly review and let me know if you've further feedback.

.../openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java

…en/languages/AbstractJavaCodegen.java Co-authored-by: Leonard Brünings <lord_damokles@gmx.net>

…efault

leonard84

I couldn't find any tests, that check the behavior of the new default values with regards to serialization/deserialization, either as json or query parameters.

I would expect at least the following scenarios:

serialization/deserialization with no explicit values present
serialization/deserialization with non-default values

leonard84 · 2023-01-18T14:07:42Z

...s/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java


        // dateLibrary <> java8
-        Assert.assertNull(defaultValue);
+        if (defaultValue.contains("HKT")) {


❌ This will only work when the local ZoneId.systemDefault() happens to be HKT which I imagine is not the case for every developer. I'd suggest to test with a named ZoneId, so that it is portable.

❗ Furthermore, I'd try to avoid conditionals in tests as much as possible. This condition can be false when it should actually be true and nothing would be failing.

leonard84 · 2023-01-18T14:09:54Z

...s/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java


        String defaultValue = codegen.toDefaultValue(schema);
-        Assert.assertEquals(defaultValue, "new HashMap<>()", "Expected string-string map aliased model to default to new HashMap<String, String>()");
+        Assert.assertEquals(defaultValue, "null", "Expected string-string map aliased model to default to null since nullable is not set to true");


❓ why is is set to null, when nullable is false?

That's the old toDefaultValue(Schema schema) not used by java generators any more, which use toDefaultValue(CodegenProperty cp, Schema schema)

leonard84 · 2023-01-18T14:20:35Z

...tstore/java/apache-httpclient/src/main/java/org/openapitools/client/model/NullableClass.java


  public static final String JSON_PROPERTY_ARRAY_ITEMS_NULLABLE = "array_items_nullable";
-  private List<Object> arrayItemsNullable = null;
+  private List<Object> arrayItemsNullable = new ArrayList<>();


❓ If the field is nullable shouldn't it be null here?

the array item is nullable, not the array itself:

array_items_nullable: type: array items: type: object nullable: true

wing328 added 3 commits November 25, 2022 00:40

add test for array default value

e4af067

update null return

0ef34fc

minor fixes

ff31150

leonard84 suggested changes Nov 28, 2022

View reviewed changes

.../openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java Outdated Show resolved Hide resolved

...ore/java/apache-httpclient/src/test/java/org/openapitools/client/model/DefaultValueTest.java Outdated Show resolved Hide resolved

wing328 added 5 commits November 30, 2022 14:57

Merge remote-tracking branch 'origin/master' into java-native-array-d…

fa73c8d

…efault

move default value tests to echo api spec

79920df

add new files

defbf68

remove unused files

1786742

fix enum array default, add tests

2e1fed8

better array init

599190a

leonard84 reviewed Jan 9, 2023

View reviewed changes

wing328 and others added 15 commits January 13, 2023 23:16

Update modules/openapi-generator/src/main/java/org/openapitools/codeg…

6f97d72

…en/languages/AbstractJavaCodegen.java Co-authored-by: Leonard Brünings <lord_damokles@gmx.net>

Update modules/openapi-generator/src/main/java/org/openapitools/codeg…

3c09089

…en/languages/AbstractJavaCodegen.java Co-authored-by: Leonard Brünings <lord_damokles@gmx.net>

Merge remote-tracking branch 'origin/master' into java-native-array-d…

284daf3

…efault

Merge remote-tracking branch 'origin/master' into java-native-array-d…

293506f

…efault

revert the fix

fd98c10

improve default value handling

62f7943

update native samples, add tests

428452f

update samples

f2ef203

fix tests

f790197

use conditional test for timezone

dee4035

add tests to apache http client echo api

b9b93fd

add option to default container to null

02b10f7

fix map default value

d4e8e37

minor refactoring

305a79c

update samples

f294172

wing328 added Client: Java Server: Java labels Jan 17, 2023

wing328 changed the title ~~[Java] support default value for array of enum string~~ [Java] better default value handling Jan 17, 2023

wing328 added this to the 6.3.0 milestone Jan 17, 2023

wing328 added the Enhancement: General label Jan 17, 2023

wing328 added 2 commits January 17, 2023 17:04

fix javadoc

4ec0a34

fix pom.xml

492c621

wing328 marked this pull request as ready for review January 17, 2023 10:58

leonard84 reviewed Jan 18, 2023

View reviewed changes

wing328 modified the milestones: 6.3.0, 6.3.1 Jan 20, 2023

wing328 added 5 commits January 29, 2023 22:14

add tests in java native echo client

d322ed0

add java apache client echo tests

6309db8

fix test

0392e0b

Merge remote-tracking branch 'origin' into java-native-array-default

1ad332a

fix test

c606f27

wing328 added the Breaking change (with fallback) label Jan 30, 2023

leonard84 approved these changes Jan 30, 2023

View reviewed changes

wing328 merged commit fd45b74 into master Jan 30, 2023

wing328 deleted the java-native-array-default branch January 30, 2023 12:19

wing328 modified the milestones: 6.4.0, 6.3.0 Feb 1, 2023

kzander91 mentioned this pull request Feb 1, 2023

[BUG] [JAVA] [webclient] addXYZ Builder Method in Pojo produces NullPointerException for nullable array properties #14583

Closed

5 tasks

anthonyCarigny mentioned this pull request Feb 6, 2023

[BUG][Java] the client generation in java instantiate Set as ArrayList #14631

Open

5 tasks

This was referenced Feb 14, 2023

[BUG][JAVA] Error in generation of defaults for arrays for specific number types #14690

Closed

[BUG] default values for references enums are not generated correctly #14689

Closed

Perdjesk mentioned this pull request Aug 3, 2023

[Java] Array default value requires import of java.util.Arrays #16246

Merged

6 tasks

twbecker mentioned this pull request Oct 31, 2023

[BUG][JAVA] Since 6.5.0 Generated Lists no longer default to empty List but null #15891

Closed

6 tasks

Uh oh!

Conversation

wing328 commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

Uh oh!

Uh oh!

wing328 commented Nov 28, 2022

Uh oh!

wing328 commented Nov 30, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leonard84 left a comment

Choose a reason for hiding this comment

Uh oh!

leonard84 Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

leonard84 Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

wing328 Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

leonard84 Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

leonard84 Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

wing328 Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

leonard84 Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wing328 commented Nov 27, 2022 •

edited

Loading