Skip to content

Commit 1eee603

Browse files
Mattias-SehlstedtMattias-Sehlstedt
authored
Fixes so that a oneOf schema with a single sub-schema is simplified (#21043)
* Fixes so that a oneOf schema with a single sub-schema is simplified when SIMPLIFY_ONEOF_ANYOF is set to true * Adjusts oneOf_array test to ensure that it is generated as an interface instead of being simplified * Update ruby samples so that they no longer refer to a model that is now gone due to the schema being simplified
1 parent 02204d0 commit 1eee603

File tree

21 files changed

+198
-409
lines changed

21 files changed

+198
-409
lines changed

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.function.Function;
3939
import java.util.stream.Collectors;
4040

41+
import static org.openapitools.codegen.utils.ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema;
4142
import static org.openapitools.codegen.utils.StringUtils.getUniqueString;
4243

4344
public class OpenAPINormalizer {
@@ -1266,17 +1267,7 @@ protected Schema processSimplifyOneOf(Schema schema) {
12661267
}
12671268
}
12681269

1269-
if (oneOfSchemas.removeIf(oneOf -> ModelUtils.isNullTypeSchema(openAPI, oneOf))) {
1270-
schema.setNullable(true);
1271-
1272-
// if only one element left, simplify to just the element (schema)
1273-
if (oneOfSchemas.size() == 1) {
1274-
if (Boolean.TRUE.equals(schema.getNullable())) { // retain nullable setting
1275-
((Schema) oneOfSchemas.get(0)).setNullable(true);
1276-
}
1277-
return (Schema) oneOfSchemas.get(0);
1278-
}
1279-
}
1270+
schema = simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, schema, oneOfSchemas);
12801271

12811272
if (ModelUtils.isIntegerSchema(schema) || ModelUtils.isNumberSchema(schema) || ModelUtils.isStringSchema(schema)) {
12821273
// TODO convert oneOf const to enum
@@ -1403,17 +1394,7 @@ protected Schema processSimplifyAnyOf(Schema schema) {
14031394
}
14041395
}
14051396

1406-
if (anyOfSchemas.removeIf(anyOf -> ModelUtils.isNullTypeSchema(openAPI, anyOf))) {
1407-
schema.setNullable(true);
1408-
}
1409-
1410-
// if only one element left, simplify to just the element (schema)
1411-
if (anyOfSchemas.size() == 1) {
1412-
if (Boolean.TRUE.equals(schema.getNullable())) { // retain nullable setting
1413-
((Schema) anyOfSchemas.get(0)).setNullable(true);
1414-
}
1415-
return (Schema) anyOfSchemas.get(0);
1416-
}
1397+
schema = simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, schema, anyOfSchemas);
14171398
}
14181399

14191400
return schema;

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2219,6 +2219,30 @@ public static Schema cloneSchema(Schema schema, boolean openapi31) {
22192219
}
22202220
}
22212221

2222+
/**
2223+
* Simplifies the schema by removing the oneOfAnyOf if the oneOfAnyOf only contains a single non-null sub-schema
2224+
*
2225+
* @param openAPI OpenAPI
2226+
* @param schema Schema
2227+
* @param subSchemas The oneOf or AnyOf schemas
2228+
* @return The simplified schema
2229+
*/
2230+
public static Schema simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(OpenAPI openAPI, Schema schema, List<Schema> subSchemas) {
2231+
if (subSchemas.removeIf(subSchema -> isNullTypeSchema(openAPI, subSchema))) {
2232+
schema.setNullable(true);
2233+
}
2234+
2235+
// if only one element left, simplify to just the element (schema)
2236+
if (subSchemas.size() == 1) {
2237+
if (Boolean.TRUE.equals(schema.getNullable())) { // retain nullable setting
2238+
subSchemas.get(0).setNullable(true);
2239+
}
2240+
return subSchemas.get(0);
2241+
}
2242+
2243+
return schema;
2244+
}
2245+
22222246
/**
22232247
* Check if the schema is of type 'null' or schema itself is pointing to null
22242248
* <p>

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,12 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf() {
168168
Schema schema15 = openAPI.getComponents().getSchemas().get("AnyOfAnyTypeWithRef");
169169
assertEquals(schema15.getAnyOf().size(), 6);
170170

171+
Schema schema17 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
172+
assertEquals(((Schema) schema17.getProperties().get("number")).getOneOf().size(), 1);
173+
174+
Schema schema19 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
175+
assertEquals(schema19.getAnyOf().size(), 1);
176+
171177
Map<String, String> options = new HashMap<>();
172178
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
173179
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
@@ -209,6 +215,30 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf() {
209215
Schema schema16 = openAPI.getComponents().getSchemas().get("AnyOfAnyTypeWithRef");
210216
assertEquals(schema16.getAnyOf(), null);
211217
assertEquals(schema16.getType(), null);
218+
219+
Schema schema18 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
220+
assertEquals(((Schema) schema18.getProperties().get("number")).get$ref(), "#/components/schemas/Number");
221+
222+
Schema schema20 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
223+
assertEquals(schema20.getAnyOf(), null);
224+
assertEquals(schema20.getType(), "string");
225+
assertEquals(schema20.getEnum().size(), 2);
226+
}
227+
228+
@Test
229+
public void testOpenAPINormalizerSimplifyOneOfWithSingleRef() {
230+
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml");
231+
232+
Schema oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
233+
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).getOneOf().size(), 1);
234+
235+
Map<String, String> options = new HashMap<>();
236+
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
237+
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
238+
openAPINormalizer.normalize();
239+
240+
oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
241+
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).get$ref(), "#/components/schemas/Number");
212242
}
213243

214244
@Test
@@ -830,6 +860,12 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() {
830860
Schema schema17 = openAPI.getComponents().getSchemas().get("OneOfNullAndRef3");
831861
assertEquals(schema17.getOneOf().size(), 2);
832862

863+
Schema schema19 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
864+
assertEquals(((Schema) schema19.getProperties().get("number")).getOneOf().size(), 1);
865+
866+
Schema schema21 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
867+
assertEquals(schema21.getAnyOf().size(), 1);
868+
833869
Map<String, String> options = new HashMap<>();
834870
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
835871
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
@@ -876,6 +912,30 @@ public void testOpenAPINormalizerSimplifyOneOfAnyOf31Spec() {
876912
// original oneOf removed and simplified to just $ref (oneOf sub-schema) instead
877913
assertEquals(schema18.getOneOf(), null);
878914
assertEquals(schema18.get$ref(), "#/components/schemas/Parent");
915+
916+
Schema schema20 = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
917+
assertEquals(((Schema) schema20.getProperties().get("number")).get$ref(), "#/components/schemas/Number");
918+
919+
Schema schema22 = openAPI.getComponents().getSchemas().get("SingleAnyOfTest");
920+
assertEquals(schema22.getAnyOf(), null);
921+
assertEquals(schema22.getTypes(), Set.of("string"));
922+
assertEquals(schema22.getEnum().size(), 2);
923+
}
924+
925+
@Test
926+
public void testOpenAPINormalizerSimplifyOneOfWithSingleRef31Spec() {
927+
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/simplifyOneOfAnyOf_test.yaml");
928+
929+
Schema oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
930+
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).getOneOf().size(), 1);
931+
932+
Map<String, String> options = new HashMap<>();
933+
options.put("SIMPLIFY_ONEOF_ANYOF", "true");
934+
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
935+
openAPINormalizer.normalize();
936+
937+
oneOfWithSingleRef = openAPI.getComponents().getSchemas().get("ParentWithOneOfProperty");
938+
assertEquals(((Schema) oneOfWithSingleRef.getProperties().get("number")).get$ref(), "#/components/schemas/Number");
879939
}
880940

881941
@Test

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2272,7 +2272,7 @@ public static Object[][] sealedScenarios() {
22722272
"SchemaA.java", "public final class SchemaA extends RepresentationModel<SchemaA> implements PostRequest {",
22732273
"PostRequest.java", "public sealed interface PostRequest permits SchemaA {")},
22742274
{"oneOf_array.yaml", Map.of(
2275-
"MyExampleGet200Response.java", "public interface MyExampleGet200Response")},
2275+
"MyExampleGet200Response.java", "public sealed interface MyExampleGet200Response")},
22762276
{"oneOf_duplicateArray.yaml", Map.of(
22772277
"Example.java", "public interface Example {")},
22782278
{"oneOf_nonPrimitive.yaml", Map.of(

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

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@
3232
import java.util.List;
3333
import java.util.Map;
3434

35-
import static org.testng.Assert.assertFalse;
36-
import static org.testng.Assert.assertTrue;
35+
import static org.testng.Assert.*;
3736

3837
public class ModelUtilsTest {
3938

@@ -476,6 +475,54 @@ public void testGetSchemaItemsWith31Spec() {
476475
Assert.assertNotNull(ModelUtils.getSchemaItems((Schema) arrayWithPrefixItems.getProperties().get("without_items")));
477476
}
478477

478+
@Test
479+
public void simplyOneOfAnyOfWithOnlyOneNonNullSubSchema() {
480+
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/simplifyOneOfAnyOf_test.yaml");
481+
Schema schema;
482+
List<Schema> subSchemas;
483+
484+
Schema anyOfWithSeveralSubSchemasButSingleNonNull = ModelUtils.getSchema(openAPI, "AnyOfTest");
485+
subSchemas = anyOfWithSeveralSubSchemasButSingleNonNull.getAnyOf();
486+
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, anyOfWithSeveralSubSchemasButSingleNonNull, subSchemas);
487+
assertNull(schema.getOneOf());
488+
assertNull(schema.getAnyOf());
489+
assertTrue(schema.getNullable());
490+
assertEquals("string", schema.getType());
491+
492+
Schema anyOfWithSingleNonNullSubSchema = ModelUtils.getSchema(openAPI, "Parent");
493+
subSchemas = ((Schema) anyOfWithSingleNonNullSubSchema.getProperties().get("number")).getAnyOf();
494+
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, anyOfWithSingleNonNullSubSchema, subSchemas);
495+
assertNull(schema.getOneOf());
496+
assertNull(schema.getAnyOf());
497+
assertNull(schema.getNullable());
498+
assertEquals(schema.get$ref(), "#/components/schemas/Number");
499+
500+
Schema oneOfWithSeveralSubSchemasButSingleNonNull = ModelUtils.getSchema(openAPI, "OneOfTest");
501+
subSchemas = oneOfWithSeveralSubSchemasButSingleNonNull.getOneOf();
502+
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, oneOfWithSeveralSubSchemasButSingleNonNull, subSchemas);
503+
assertNull(schema.getOneOf());
504+
assertNull(schema.getAnyOf());
505+
assertTrue(schema.getNullable());
506+
assertEquals("integer", schema.getType());
507+
508+
Schema oneOfWithSingleNonNullSubSchema = ModelUtils.getSchema(openAPI, "ParentWithOneOfProperty");
509+
subSchemas = ((Schema) oneOfWithSingleNonNullSubSchema.getProperties().get("number")).getOneOf();
510+
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, oneOfWithSingleNonNullSubSchema, subSchemas);
511+
assertNull(schema.getOneOf());
512+
assertNull(schema.getAnyOf());
513+
assertNull(schema.getNullable());
514+
assertEquals(schema.get$ref(), "#/components/schemas/Number");
515+
516+
Schema oneOfWithSeveralSubSchemas = ModelUtils.getSchema(openAPI, "ParentWithPluralOneOfProperty");
517+
subSchemas = ((Schema) oneOfWithSeveralSubSchemas.getProperties().get("number")).getOneOf();
518+
schema = ModelUtils.simplyOneOfAnyOfWithOnlyOneNonNullSubSchema(openAPI, oneOfWithSeveralSubSchemas, subSchemas);
519+
assertNull(schema.getOneOf());
520+
assertNotNull(oneOfWithSeveralSubSchemas.getProperties().get("number"));
521+
assertNull(schema.getAnyOf());
522+
assertNull(schema.getNullable());
523+
assertEquals(((Schema) oneOfWithSeveralSubSchemas.getProperties().get("number")).getOneOf().size(), 2);
524+
}
525+
479526
@Test
480527
public void isNullTypeSchemaTest() {
481528
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/null_schema_test.yaml");

‎modules/openapi-generator/src/test/resources/3_0/oneOf_array.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ paths:
1515
- type: array
1616
items:
1717
"$ref": "#/components/schemas/OneOf1"
18+
- type: object
19+
"$ref": "#/components/schemas/OneOf1"
1820
components:
1921
schemas:
2022
OneOf1:

‎samples/client/others/java/okhttp-gson-oneOf-array/api/openapi.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,5 @@ components:
3030
- items:
3131
$ref: '#/components/schemas/OneOf1'
3232
type: array
33+
- $ref: '#/components/schemas/OneOf1'
3334

‎samples/client/others/java/okhttp-gson-oneOf-array/docs/MyExampleGet200Response.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
| Name | Type | Description | Notes |
99
|------------ | ------------- | ------------- | -------------|
10+
|**message1** | **String** | | [optional] |
1011

1112

1213

‎samples/client/others/java/okhttp-gson-oneOf-array/src/main/java/org/openapitools/client/model/MyExampleGet200Response.java

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@
1414
package org.openapitools.client.model;
1515

1616
import java.util.Objects;
17+
import com.google.gson.TypeAdapter;
18+
import com.google.gson.annotations.JsonAdapter;
19+
import com.google.gson.annotations.SerializedName;
20+
import com.google.gson.stream.JsonReader;
21+
import com.google.gson.stream.JsonWriter;
22+
import java.io.IOException;
23+
import java.util.Arrays;
1724
import java.util.List;
1825
import org.openapitools.client.model.OneOf1;
1926
import javax.validation.constraints.*;
@@ -69,6 +76,7 @@ public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
6976

7077
final Type typeInstanceListOneOf1 = new TypeToken<List<@Valid OneOf1>>(){}.getType();
7178
final TypeAdapter<List<@Valid OneOf1>> adapterListOneOf1 = (TypeAdapter<List<@Valid OneOf1>>) gson.getDelegateAdapter(this, TypeToken.get(typeInstanceListOneOf1));
79+
final TypeAdapter<OneOf1> adapterOneOf1 = gson.getDelegateAdapter(this, TypeToken.get(OneOf1.class));
7280

7381
return (TypeAdapter<T>) new TypeAdapter<MyExampleGet200Response>() {
7482
@Override
@@ -87,7 +95,13 @@ public void write(JsonWriter out, MyExampleGet200Response value) throws IOExcept
8795
return;
8896
}
8997
}
90-
throw new IOException("Failed to serialize as the type doesn't match oneOf schemas: List<@Valid OneOf1>");
98+
// check if the actual instance is of the type `OneOf1`
99+
if (value.getActualInstance() instanceof OneOf1) {
100+
JsonElement element = adapterOneOf1.toJsonTree((OneOf1)value.getActualInstance());
101+
elementAdapter.write(out, element);
102+
return;
103+
}
104+
throw new IOException("Failed to serialize as the type doesn't match oneOf schemas: List<@Valid OneOf1>, OneOf1");
91105
}
92106

93107
@Override
@@ -119,6 +133,18 @@ public MyExampleGet200Response read(JsonReader in) throws IOException {
119133
errorMessages.add(String.format("Deserialization for List<@Valid OneOf1> failed with `%s`.", e.getMessage()));
120134
log.log(Level.FINER, "Input data does not match schema 'List<@Valid OneOf1>'", e);
121135
}
136+
// deserialize OneOf1
137+
try {
138+
// validate the JSON object to see if any exception is thrown
139+
OneOf1.validateJsonElement(jsonElement);
140+
actualAdapter = adapterOneOf1;
141+
match++;
142+
log.log(Level.FINER, "Input data matches schema 'OneOf1'");
143+
} catch (Exception e) {
144+
// deserialization failed, continue
145+
errorMessages.add(String.format("Deserialization for OneOf1 failed with `%s`.", e.getMessage()));
146+
log.log(Level.FINER, "Input data does not match schema 'OneOf1'", e);
147+
}
122148

123149
if (match == 1) {
124150
MyExampleGet200Response ret = new MyExampleGet200Response();
@@ -146,6 +172,7 @@ public MyExampleGet200Response(Object o) {
146172

147173
static {
148174
schemas.put("List<@Valid OneOf1>", List.class);
175+
schemas.put("OneOf1", OneOf1.class);
149176
}
150177

151178
@Override
@@ -156,7 +183,7 @@ public Map<String, Class<?>> getSchemas() {
156183
/**
157184
* Set the instance that matches the oneOf child schema, check
158185
* the instance parameter is valid against the oneOf child schemas:
159-
* List<@Valid OneOf1>
186+
* List<@Valid OneOf1>, OneOf1
160187
*
161188
* It could be an instance of the 'oneOf' schemas.
162189
*/
@@ -170,14 +197,19 @@ public void setActualInstance(Object instance) {
170197
}
171198
}
172199

173-
throw new RuntimeException("Invalid instance type. Must be List<@Valid OneOf1>");
200+
if (instance instanceof OneOf1) {
201+
super.setActualInstance(instance);
202+
return;
203+
}
204+
205+
throw new RuntimeException("Invalid instance type. Must be List<@Valid OneOf1>, OneOf1");
174206
}
175207

176208
/**
177209
* Get the actual instance, which can be the following:
178-
* List<@Valid OneOf1>
210+
* List<@Valid OneOf1>, OneOf1
179211
*
180-
* @return The actual instance (List<@Valid OneOf1>)
212+
* @return The actual instance (List<@Valid OneOf1>, OneOf1)
181213
*/
182214
@SuppressWarnings("unchecked")
183215
@Override
@@ -196,6 +228,17 @@ public Object getActualInstance() {
196228
return (List<@Valid OneOf1>)super.getActualInstance();
197229
}
198230

231+
/**
232+
* Get the actual instance of `OneOf1`. If the actual instance is not `OneOf1`,
233+
* the ClassCastException will be thrown.
234+
*
235+
* @return The actual instance of `OneOf1`
236+
* @throws ClassCastException if the instance is not `OneOf1`
237+
*/
238+
public OneOf1 getOneOf1() throws ClassCastException {
239+
return (OneOf1)super.getActualInstance();
240+
}
241+
199242
/**
200243
* Validates the JSON Element and throws an exception if issues found
201244
*
@@ -221,8 +264,16 @@ public static void validateJsonElement(JsonElement jsonElement) throws IOExcepti
221264
errorMessages.add(String.format("Deserialization for List<@Valid OneOf1> failed with `%s`.", e.getMessage()));
222265
// continue to the next one
223266
}
267+
// validate the json string with OneOf1
268+
try {
269+
OneOf1.validateJsonElement(jsonElement);
270+
validCount++;
271+
} catch (Exception e) {
272+
errorMessages.add(String.format("Deserialization for OneOf1 failed with `%s`.", e.getMessage()));
273+
// continue to the next one
274+
}
224275
if (validCount != 1) {
225-
throw new IOException(String.format("The JSON string is invalid for MyExampleGet200Response with oneOf schemas: List<@Valid OneOf1>. %d class(es) match the result, expected 1. Detailed failure message for oneOf schemas: %s. JSON: %s", validCount, errorMessages, jsonElement.toString()));
276+
throw new IOException(String.format("The JSON string is invalid for MyExampleGet200Response with oneOf schemas: List<@Valid OneOf1>, OneOf1. %d class(es) match the result, expected 1. Detailed failure message for oneOf schemas: %s. JSON: %s", validCount, errorMessages, jsonElement.toString()));
226277
}
227278
}
228279

0 commit comments

Comments
 (0)