Skip to content

Commit 54fe232

Browse files
marceljkfanqiewanzi
marceljk
and
fanqiewanzi
authored
[BUG] [JAVA] validateJsonElement fails for required nullable fields (#22912)
* [Java] add missing nullable judgement when required property is true * [Java] add okhttp template test and regenerate sample * [Java] add tests when field is nullable and required * [Java] regenerate samples to fix pipeline error * [Java] add JSONTest fro RequiredNullableBody class * run generate-samples after rebase * review feedback * review feedback * fix test * update hash of test file --------- Co-authored-by: weirdo0314 <2019215183@stu.cqupt.edu.cn>
1 parent 459f359 commit 54fe232

21 files changed

Lines changed: 1527 additions & 30 deletions

File tree

‎bin/utils/test_file_list.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/ClientTest.java"
1111
sha256: 325fdd5d7e2c97790c0fb44f712ab7b2ba022d7e1a5b0056f47b07f342682b6d
1212
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/JSONTest.java"
13-
sha256: 67941355a0a27ed9ff9318b1caa103e78b81b9aff61b594b18be5cd2bb9f6591
13+
sha256: b1b1d31e0df17f0b68cf2747a4a53879f12acb1bf2860e45385c679c1efe9894
1414
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/api/PetApiTest.java"
1515
sha256: 8b1b8f2a2ad00ccb090873a94a5f73e328b98317d2ec715f53bd7a1accb2a023
1616
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/model/PetTest.java"

‎modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -385,16 +385,16 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
385385
{{#isArray}}
386386
{{#items.isModel}}
387387
{{#required}}
388-
// ensure the json data is an array
389-
if (!jsonObj.get("{{{baseName}}}").isJsonArray()) {
390-
throw new IllegalArgumentException(String.format(java.util.Locale.ROOT, "Expected the field `{{{baseName}}}` to be an array in the JSON string but got `%s`", jsonObj.get("{{{baseName}}}").toString()));
388+
if (jsonObj.get("{{{baseName}}}") != null{{#isNullable}} && !jsonObj.get("{{{baseName}}}").isJsonNull(){{/isNullable}}) {
389+
if (!jsonObj.get("{{{baseName}}}").isJsonArray()) {
390+
throw new IllegalArgumentException(String.format(java.util.Locale.ROOT, "Expected the field `{{{baseName}}}` to be an array in the JSON string but got `%s`", jsonObj.get("{{{baseName}}}").toString()));
391+
}
392+
JsonArray jsonArray{{name}} = jsonObj.getAsJsonArray("{{{baseName}}}");
393+
// validate the required field `{{{baseName}}}` (array)
394+
for (int i = 0; i < jsonArray{{name}}.size(); i++) {
395+
{{{items.dataType}}}.validateJsonElement(jsonArray{{name}}.get(i));
396+
}
391397
}
392-
393-
JsonArray jsonArray{{name}} = jsonObj.getAsJsonArray("{{{baseName}}}");
394-
// validate the required field `{{{baseName}}}` (array)
395-
for (int i = 0; i < jsonArray{{name}}.size(); i++) {
396-
{{{items.dataType}}}.validateJsonElement(jsonArray{{name}}.get(i));
397-
};
398398
{{/required}}
399399
{{^required}}
400400
if (jsonObj.get("{{{baseName}}}") != null && !jsonObj.get("{{{baseName}}}").isJsonNull()) {
@@ -424,7 +424,7 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
424424
// ensure the required json array is present
425425
if (jsonObj.get("{{{baseName}}}") == null) {
426426
throw new IllegalArgumentException("Expected the field `linkedContent` to be an array in the JSON string but got `null`");
427-
} else if (!jsonObj.get("{{{baseName}}}").isJsonArray()) {
427+
} else if (!jsonObj.get("{{{baseName}}}").isJsonArray(){{#isNullable}} && !jsonObj.get("{{baseName}}").isJsonNull(){{/isNullable}}) {
428428
throw new IllegalArgumentException(String.format(java.util.Locale.ROOT, "Expected the field `{{{baseName}}}` to be an array in the JSON string but got `%s`", jsonObj.get("{{{baseName}}}").toString()));
429429
}
430430
{{/required}}
@@ -438,8 +438,14 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
438438
{{/isString}}
439439
{{#isModel}}
440440
{{#required}}
441+
{{#isNullable}}
442+
if (jsonObj.get("{{{baseName}}}") != null && !jsonObj.get("{{{baseName}}}").isJsonNull()) {
443+
{{/isNullable}}
441444
// validate the required field `{{{baseName}}}`
442445
{{{dataType}}}.validateJsonElement(jsonObj.get("{{{baseName}}}"));
446+
{{#isNullable}}
447+
}
448+
{{/isNullable}}
443449
{{/required}}
444450
{{^required}}
445451
// validate the optional field `{{{baseName}}}`
@@ -450,8 +456,14 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
450456
{{/isModel}}
451457
{{#isEnum}}
452458
{{#required}}
459+
{{#isNullable}}
460+
if (jsonObj.get("{{{baseName}}}") != null && !jsonObj.get("{{{baseName}}}").isJsonNull()) {
461+
{{/isNullable}}
453462
// validate the required field `{{{baseName}}}`
454463
{{{datatypeWithEnum}}}.validateJsonElement(jsonObj.get("{{{baseName}}}"));
464+
{{#isNullable}}
465+
}
466+
{{/isNullable}}
455467
{{/required}}
456468
{{^required}}
457469
// validate the optional field `{{{baseName}}}`
@@ -462,8 +474,14 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
462474
{{/isEnum}}
463475
{{#isEnumRef}}
464476
{{#required}}
477+
{{#isNullable}}
478+
if (jsonObj.get("{{{baseName}}}") != null && !jsonObj.get("{{{baseName}}}").isJsonNull()) {
479+
{{/isNullable}}
465480
// validate the required field `{{{baseName}}}`
466481
{{{dataType}}}.validateJsonElement(jsonObj.get("{{{baseName}}}"));
482+
{{#isNullable}}
483+
}
484+
{{/isNullable}}
467485
{{/required}}
468486
{{^required}}
469487
// validate the optional field `{{{baseName}}}`

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3769,6 +3769,35 @@ public void testClassesAreValidJavaOkHttpGson() {
37693769
);
37703770
}
37713771

3772+
@Test
3773+
public void testRequiredAndNullableAreBothTrue() throws IOException {
3774+
File output = Files.createTempDirectory("test").toFile();
3775+
output.deleteOnExit();
3776+
3777+
final CodegenConfigurator configurator = new CodegenConfigurator()
3778+
.setGeneratorName("java")
3779+
.setLibrary(JavaClientCodegen.OKHTTP_GSON)
3780+
.setInputSpec("src/test/resources/bugs/issue_18516.yaml")
3781+
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));
3782+
3783+
3784+
DefaultGenerator generator = new DefaultGenerator();
3785+
List<File> files = generator.opts(configurator.toClientOptInput()).generate();
3786+
files.forEach(File::deleteOnExit);
3787+
3788+
validateJavaSourceFiles(files);
3789+
3790+
Path modelFile = Paths.get(output + "/src/main/java/org/openapitools/client/model/SomeObject.java");
3791+
TestUtils.assertFileContains(
3792+
modelFile,
3793+
"} else if (!jsonObj.get(\"ids\").isJsonArray() && !jsonObj.get(\"ids\").isJsonNull()) {",
3794+
"if (jsonObj.get(\"users\") != null && !jsonObj.get(\"users\").isJsonNull()) {",
3795+
"if (!jsonObj.get(\"users\").isJsonArray()) {",
3796+
"if (jsonObj.get(\"user\") != null && !jsonObj.get(\"user\").isJsonNull()) {",
3797+
"if (jsonObj.get(\"role\") != null && !jsonObj.get(\"role\").isJsonNull()) {",
3798+
"if (jsonObj.get(\"custom\") != null && !jsonObj.get(\"custom\").isJsonNull()) {");
3799+
}
3800+
37723801
@Test(description = "Issue #21051")
37733802
public void givenComplexObjectHasDefaultValueWhenGenerateThenDefaultAssignmentsAreValid() throws Exception {
37743803
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();

‎modules/openapi-generator/src/test/resources/3_0/java/petstore-with-fake-endpoints-models-for-testing-okhttp-gson.yaml‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,6 +1369,19 @@ paths:
13691369
$ref: '#/components/schemas/Variable'
13701370
'400':
13711371
description: Invalid Value
1372+
/fake/required-nullable-body:
1373+
get:
1374+
tags:
1375+
- fake
1376+
summary: fields in the response body, required and nullable are both true
1377+
description: ''
1378+
responses:
1379+
'200':
1380+
description: success
1381+
content:
1382+
application/json:
1383+
schema:
1384+
$ref: '#/components/schemas/RequiredNullableBody'
13721385
servers:
13731386
- url: 'http://{server}.swagger.io:{port}/v2'
13741387
description: petstore server
@@ -2768,6 +2781,38 @@ components:
27682781
$ref: '#/components/schemas/ArrayOneOf'
27692782
anyof_prop:
27702783
$ref: '#/components/schemas/ArrayAnyOf'
2784+
NullableEnum:
2785+
type: string
2786+
nullable: true
2787+
enum:
2788+
- custom
2789+
RequiredNullableBody:
2790+
allOf:
2791+
- $ref: '#/components/schemas/NullableClass'
2792+
- type: object
2793+
required:
2794+
- custom_ref_enum
2795+
- custom_enum
2796+
- integer_prop
2797+
- number_prop
2798+
- boolean_prop
2799+
- string_prop
2800+
- date_prop
2801+
- datetime_prop
2802+
- array_nullable_prop
2803+
- array_and_items_nullable_prop
2804+
- array_items_nullable
2805+
- object_nullable_prop
2806+
- object_and_items_nullable_prop
2807+
- object_items_nullable
2808+
properties:
2809+
custom_ref_enum:
2810+
$ref: "#/components/schemas/NullableEnum"
2811+
custom_enum:
2812+
type: string
2813+
nullable: true
2814+
enum:
2815+
- custom
27712816
NestedArrayWithDefaultValues:
27722817
type: object
27732818
properties:

‎modules/openapi-generator/src/test/resources/bugs/issue_18516.yaml‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
openapi: 3.0.3
2+
info:
3+
title: test
4+
description: Test API
5+
version: 1.0.1
6+
7+
paths:
8+
/test:
9+
get:
10+
responses:
11+
200:
12+
description: Valid response
13+
content:
14+
application/json:
15+
schema:
16+
$ref: "#/components/schemas/SomeObject"
17+
18+
components:
19+
schemas:
20+
SomeObject:
21+
type: object
22+
required:
23+
- ids
24+
- users
25+
- user
26+
- role
27+
- custom
28+
properties:
29+
ids:
30+
type: array
31+
nullable: true
32+
items:
33+
type: integer
34+
users:
35+
type: array
36+
nullable: true
37+
items:
38+
type: object
39+
properties:
40+
id:
41+
type: string
42+
user:
43+
type: object
44+
nullable: true
45+
properties:
46+
id:
47+
type: string
48+
role:
49+
type: string
50+
nullable: true
51+
enum:
52+
- admin
53+
- tenant
54+
custom:
55+
$ref: "#/components/schemas/customEnum"
56+
customEnum:
57+
type: string
58+
nullable: true
59+
enum:
60+
- custom
61+

‎samples/client/petstore/java/okhttp-gson-nullable-required/src/main/java/org/openapitools/client/model/PetWithRequiredNullableCases1.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
424424
// ensure the required json array is present
425425
if (jsonObj.get("photoUrls") == null) {
426426
throw new IllegalArgumentException("Expected the field `linkedContent` to be an array in the JSON string but got `null`");
427-
} else if (!jsonObj.get("photoUrls").isJsonArray()) {
427+
} else if (!jsonObj.get("photoUrls").isJsonArray() && !jsonObj.get("photoUrls").isJsonNull()) {
428428
throw new IllegalArgumentException(String.format(java.util.Locale.ROOT, "Expected the field `photoUrls` to be an array in the JSON string but got `%s`", jsonObj.get("photoUrls").toString()));
429429
}
430430
if (jsonObj.get("tags") != null && !jsonObj.get("tags").isJsonNull()) {

‎samples/client/petstore/java/okhttp-gson/.openapi-generator/FILES‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ docs/NewPet.md
7979
docs/NewPetCategoryInlineAllof.md
8080
docs/NewPetCategoryInlineAllofAllOfCategoryTag.md
8181
docs/NullableClass.md
82+
docs/NullableEnum.md
8283
docs/NullableShape.md
8384
docs/NumberOnly.md
8485
docs/ObjectWithDeprecatedFields.md
@@ -100,6 +101,7 @@ docs/PropertyNameCollision.md
100101
docs/Quadrilateral.md
101102
docs/QuadrilateralInterface.md
102103
docs/ReadOnlyFirst.md
104+
docs/RequiredNullableBody.md
103105
docs/Scalar.md
104106
docs/ScalarAnyOf.md
105107
docs/ScaleneTriangle.md
@@ -228,6 +230,7 @@ src/main/java/org/openapitools/client/model/NewPet.java
228230
src/main/java/org/openapitools/client/model/NewPetCategoryInlineAllof.java
229231
src/main/java/org/openapitools/client/model/NewPetCategoryInlineAllofAllOfCategoryTag.java
230232
src/main/java/org/openapitools/client/model/NullableClass.java
233+
src/main/java/org/openapitools/client/model/NullableEnum.java
231234
src/main/java/org/openapitools/client/model/NullableShape.java
232235
src/main/java/org/openapitools/client/model/NumberOnly.java
233236
src/main/java/org/openapitools/client/model/ObjectWithDeprecatedFields.java
@@ -248,6 +251,7 @@ src/main/java/org/openapitools/client/model/PropertyNameCollision.java
248251
src/main/java/org/openapitools/client/model/Quadrilateral.java
249252
src/main/java/org/openapitools/client/model/QuadrilateralInterface.java
250253
src/main/java/org/openapitools/client/model/ReadOnlyFirst.java
254+
src/main/java/org/openapitools/client/model/RequiredNullableBody.java
251255
src/main/java/org/openapitools/client/model/Scalar.java
252256
src/main/java/org/openapitools/client/model/ScalarAnyOf.java
253257
src/main/java/org/openapitools/client/model/ScaleneTriangle.java

‎samples/client/petstore/java/okhttp-gson/README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ Class | Method | HTTP request | Description
127127
*FakeApi* | [**fakeOuterNumberSerialize**](docs/FakeApi.md#fakeOuterNumberSerialize) | **POST** /fake/outer/number |
128128
*FakeApi* | [**fakeOuterStringSerialize**](docs/FakeApi.md#fakeOuterStringSerialize) | **POST** /fake/outer/string |
129129
*FakeApi* | [**fakeRefParameter**](docs/FakeApi.md#fakeRefParameter) | **POST** /fake/pet/{petId}/reference/parameter | fake reference parameter
130+
*FakeApi* | [**fakeRequiredNullableBodyGet**](docs/FakeApi.md#fakeRequiredNullableBodyGet) | **GET** /fake/required-nullable-body | fields in the response body, required and nullable are both true
130131
*FakeApi* | [**fakeUploadRefRequestBodies**](docs/FakeApi.md#fakeUploadRefRequestBodies) | **POST** /fake/pet/{petId}/uploadImage | fake reference parameter
131132
*FakeApi* | [**getFakeArrayofenums**](docs/FakeApi.md#getFakeArrayofenums) | **GET** /fake/array-of-enums | Array of Enums
132133
*FakeApi* | [**getFakeHealth**](docs/FakeApi.md#getFakeHealth) | **GET** /fake/health | Health check endpoint
@@ -240,6 +241,7 @@ Class | Method | HTTP request | Description
240241
- [NewPetCategoryInlineAllof](docs/NewPetCategoryInlineAllof.md)
241242
- [NewPetCategoryInlineAllofAllOfCategoryTag](docs/NewPetCategoryInlineAllofAllOfCategoryTag.md)
242243
- [NullableClass](docs/NullableClass.md)
244+
- [NullableEnum](docs/NullableEnum.md)
243245
- [NullableShape](docs/NullableShape.md)
244246
- [NumberOnly](docs/NumberOnly.md)
245247
- [ObjectWithDeprecatedFields](docs/ObjectWithDeprecatedFields.md)
@@ -260,6 +262,7 @@ Class | Method | HTTP request | Description
260262
- [Quadrilateral](docs/Quadrilateral.md)
261263
- [QuadrilateralInterface](docs/QuadrilateralInterface.md)
262264
- [ReadOnlyFirst](docs/ReadOnlyFirst.md)
265+
- [RequiredNullableBody](docs/RequiredNullableBody.md)
263266
- [Scalar](docs/Scalar.md)
264267
- [ScalarAnyOf](docs/ScalarAnyOf.md)
265268
- [ScaleneTriangle](docs/ScaleneTriangle.md)

‎samples/client/petstore/java/okhttp-gson/api/openapi.yaml‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,6 +1494,21 @@ paths:
14941494
x-internal: true
14951495
x-accepts:
14961496
- application/json
1497+
/fake/required-nullable-body:
1498+
get:
1499+
description: ""
1500+
responses:
1501+
"200":
1502+
content:
1503+
application/json:
1504+
schema:
1505+
$ref: "#/components/schemas/RequiredNullableBody"
1506+
description: success
1507+
summary: "fields in the response body, required and nullable are both true"
1508+
tags:
1509+
- fake
1510+
x-accepts:
1511+
- application/json
14971512
components:
14981513
parameters:
14991514
pet_id:
@@ -2870,6 +2885,62 @@ components:
28702885
anyof_prop:
28712886
$ref: "#/components/schemas/ArrayAnyOf"
28722887
type: object
2888+
NullableEnum:
2889+
enum:
2890+
- custom
2891+
nullable: true
2892+
type: string
2893+
RequiredNullableBody:
2894+
allOf:
2895+
- $ref: "#/components/schemas/NullableClass"
2896+
- properties:
2897+
custom_ref_enum:
2898+
$ref: "#/components/schemas/NullableEnum"
2899+
custom_enum:
2900+
enum:
2901+
- custom
2902+
nullable: true
2903+
type: string
2904+
required:
2905+
- array_and_items_nullable_prop
2906+
- array_items_nullable
2907+
- array_nullable_prop
2908+
- boolean_prop
2909+
- custom_enum
2910+
- custom_ref_enum
2911+
- date_prop
2912+
- datetime_prop
2913+
- integer_prop
2914+
- number_prop
2915+
- object_and_items_nullable_prop
2916+
- object_items_nullable
2917+
- object_nullable_prop
2918+
- string_prop
2919+
type: object
2920+
example:
2921+
number_prop: 6.027456183070403
2922+
datetime_prop: 2000-01-23T04:56:07.000+00:00
2923+
custom_ref_enum: custom
2924+
boolean_prop: true
2925+
string_prop: string_prop
2926+
array_nullable_prop:
2927+
- "{}"
2928+
- "{}"
2929+
custom_enum: custom
2930+
integer_prop: 0
2931+
array_and_items_nullable_prop:
2932+
- "{}"
2933+
- "{}"
2934+
object_items_nullable:
2935+
key: "{}"
2936+
object_nullable_prop:
2937+
key: "{}"
2938+
object_and_items_nullable_prop:
2939+
key: "{}"
2940+
date_prop: 2000-01-23
2941+
array_items_nullable:
2942+
- "{}"
2943+
- "{}"
28732944
NestedArrayWithDefaultValues:
28742945
properties:
28752946
nestedArray:

0 commit comments

Comments
 (0)