Skip to content

Commit 029523d

Browse files
jpfinnecubic-dev-ai[bot]
jpfinne
and
cubic-dev-ai[bot]
authored
fix(core) handle mismatching discriminatorType in oneOf interfaces (#24812)
* issue_24769 sample * Fix #24769: improve common discriminator getter * Fix #24769: common string schema with no format * workaround for some misconfigured unit tests * Fallback to default behaviour for Kotlin * Fallback to default behaviour for Kotlin * Fallback to string if not type * Hanling of $ref to find discriminatorType * cleanup * Remove not supported by JavaFileAssert test on sealed interface * Update modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/DiscriminatorUtils.java Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * Update modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * Update modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * Minor cleanup after code review * Some variation of #19194 * Fix typo * More tests * Fix string not mapped * Use getPrimitiveType() * Enum improvement * Test more cases * Avoid NPE * Improve allOf support * Fix typo * Add example with discriminator being enums * Fix required enumType --------- Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
1 parent 4f3eb90 commit 029523d

9 files changed

Lines changed: 708 additions & 3 deletions

File tree

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

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3861,9 +3861,93 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch
38613861
* @param discriminatorPropertyName The name of the discriminator property.
38623862
*/
38633863
protected String getDiscriminatorPropertyType(Schema schema, String discriminatorPropertyName) {
3864-
return DiscriminatorUtils.getDiscriminatorPropertyType(schema, discriminatorPropertyName)
3864+
String type = DiscriminatorUtils.getDiscriminatorPropertyType(schema, discriminatorPropertyName)
38653865
.map(this::toModelName)
3866-
.orElseGet(() -> typeMapping.get("string"));
3866+
.orElse(null);
3867+
if (type != null) {
3868+
return type;
3869+
}
3870+
List<Schema> schemas = DiscriminatorUtils.getDistinctTypes(openAPI, schema, discriminatorPropertyName);
3871+
return getCommonSchemaType(schemas);
3872+
}
3873+
3874+
/**
3875+
* Get the most commons denominator schemaType for several schemas.
3876+
* <p>
3877+
* @param schemas the list of schemas to compare.
3878+
*
3879+
* @Return the comman type
3880+
*/
3881+
protected String getCommonSchemaType(List<Schema> schemas) {
3882+
switch (schemas.size()) {
3883+
case 0:
3884+
//. keep string for backward compatibility
3885+
return typeMapping.get("string");
3886+
case 1:
3887+
3888+
Schema first = schemas.get(0);
3889+
try {
3890+
if (StringUtils.isNotEmpty(first.get$ref())) {
3891+
return toModelName(ModelUtils.getSimpleRef(first.get$ref()));
3892+
}
3893+
if (ModelUtils.isEnumSchema(first)) {
3894+
// inline enum, use the least common denominator
3895+
String simpleType = typeMapping.get("enum");
3896+
3897+
return simpleType != null? simpleType: typeMapping.get("object");
3898+
}
3899+
return typeMapping.get(getPrimitiveType(first));
3900+
} catch (Exception e) {
3901+
// fallback for some unit test misconfigurations...
3902+
LOGGER.warn("Unable to model name for " + first, e);
3903+
return typeMapping.get("string");
3904+
}
3905+
default:
3906+
break;
3907+
}
3908+
3909+
// boolean allRef = schemas.stream().allMatch(s -> s.get$ref() != null);
3910+
// if (allRef) {
3911+
// Set<String> modelNames = schemas.stream()
3912+
// .map(s -> toModelName(ModelUtils.getSimpleRef(s.get$ref())))
3913+
// .filter(Objects::nonNull)
3914+
// .collect(Collectors.toSet());
3915+
// if (modelNames.size() == 1) {
3916+
// return modelNames.iterator().next();
3917+
// }
3918+
// }
3919+
schemas = schemas.stream().map(s -> ModelUtils.getReferencedSchema(openAPI, s)).collect(Collectors.toList());
3920+
return getCommonTypeMapping(schemas);
3921+
}
3922+
3923+
/**
3924+
* Find the common type between different schemas.
3925+
*
3926+
* @param schemas list of more than one schema.
3927+
* @return the common matching mapped type
3928+
*/
3929+
protected String getCommonTypeMapping(List<Schema> schemas) {
3930+
String simpleType = "object";
3931+
3932+
boolean allEnums = schemas.stream().allMatch(ModelUtils::isEnumSchema);
3933+
if (allEnums) {
3934+
// non matching enums. Use enum if if the langage can map it.
3935+
if (typeMapping.containsKey("enum")) {
3936+
simpleType = "enum";
3937+
}
3938+
return typeMapping.get(simpleType);
3939+
}
3940+
if (schemas.stream().noneMatch(ModelUtils::isEnumSchema)) {
3941+
Set<String> types = schemas.stream().map(this::getPrimitiveType).collect(Collectors.toSet());
3942+
if (types.size() == 1) {
3943+
String foundPrimitiveType = types.iterator().next();
3944+
if (typeMapping.containsKey(foundPrimitiveType)) {
3945+
// matching simple type
3946+
simpleType = foundPrimitiveType;
3947+
}
3948+
}
3949+
}
3950+
return typeMapping.get(simpleType);
38673951
}
38683952

38693953
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ public AbstractJavaCodegen() {
304304
typeMapping.put("date", "Date");
305305
typeMapping.put("file", "File");
306306
typeMapping.put("AnyType", "Object");
307+
typeMapping.put("enum", "Enum");
307308

308309
importMapping.put("BigDecimal", "java.math.BigDecimal");
309310
importMapping.put("UUID", "java.util.UUID");

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,16 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
490490
return objs;
491491
}
492492

493+
/**
494+
* Kotlin has its own implementation in postProcessAllModels
495+
*
496+
* @return
497+
*/
498+
@Override
499+
protected String getCommonSchemaType(List<Schema> schemas) {
500+
return typeMapping.get("string");
501+
}
502+
493503
@Override
494504
public ModelsMap postProcessModels(ModelsMap objs) {
495505
objs = super.postProcessModelsEnum(objs);

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.slf4j.helpers.MessageFormatter;
1111

1212
import java.util.*;
13+
import java.util.stream.Collectors;
1314

1415
import static org.openapitools.codegen.CodegenConstants.X_DISCRIMINATOR_VALUE;
1516
import static org.openapitools.codegen.utils.OnceLogger.once;
@@ -322,6 +323,132 @@ private static CodegenProperty getDiscriminatorCodegenProperty(OpenAPI openAPI,
322323
return null;
323324
}
324325

326+
/**
327+
* Get the best matching simple types for all the mapped schemas
328+
* @param openAPI The openAPI specification
329+
* @param schema The Schema that may contain the discriminator
330+
* @param discPropName The String that is the discriminator propertyName in the schema
331+
* @return the distinct list of property Schema with the requested discPropName
332+
*/
333+
public static List<Schema> getDistinctTypes(OpenAPI openAPI, Schema schema, String discPropName) {
334+
List<Schema> mappedSchemas = getMappedSchemas(openAPI, schema);
335+
List<Schema> properties = new ArrayList<>();
336+
for (Schema s: mappedSchemas) {
337+
Schema prop = findProperty(openAPI, s, discPropName, new HashSet<>());
338+
if (prop != null && properties.stream().noneMatch(p ->
339+
Objects.equals(p.getType(), prop.getType()) &&
340+
Objects.equals(p.get$ref(), prop.get$ref()) &&
341+
Objects.equals(p.getEnum(), prop.getEnum()) &&
342+
Objects.equals(p.getFormat(), prop.getFormat()))) {
343+
properties.add(prop);
344+
}
345+
}
346+
return properties;
347+
}
348+
349+
/**
350+
* return the list of dereferenced mapping schemas.
351+
* @return the schemas found or empty list if not found.
352+
*/
353+
public static List<Schema> getMappedSchemas(OpenAPI openAPI, Schema schema) {
354+
if (schema.getDiscriminator() != null) {
355+
if (schema.getDiscriminator().getMapping() != null) {
356+
return schema.getDiscriminator().getMapping().values().stream()
357+
.map(ref -> getReferencedSchema(openAPI, ref))
358+
.filter(Objects::nonNull)
359+
.collect(Collectors.toList());
360+
}
361+
if (schema.getOneOf() != null && !schema.getOneOf().isEmpty()) {
362+
// try also oneOf without discriminator mapping
363+
List<Schema> oneOfs = (List<Schema>) schema.getOneOf();
364+
return oneOfs.stream()
365+
.filter(Objects::nonNull)
366+
.map(oneOf -> oneOf.get$ref()!=null
367+
? getReferencedSchema(openAPI, oneOf.get$ref())
368+
: oneOf)
369+
.filter(Objects::nonNull)
370+
.collect(Collectors.toList());
371+
}
372+
}
373+
return Collections.emptyList();
374+
}
375+
376+
private static Schema getReferencedSchema(OpenAPI openAPI, String ref) {
377+
if (ref == null) {
378+
return null;
379+
}
380+
if (ref.indexOf('/') >= 0) {
381+
ref = ModelUtils.getSimpleRef(ref);
382+
}
383+
return ModelUtils.getSchema(openAPI, ref);
384+
}
385+
386+
/**
387+
* Recursively try to find the schema matching the propertyName.
388+
*
389+
* @return the schema found or null if not found
390+
*/
391+
public static Schema findProperty(OpenAPI openAPI, Schema schema, String propertyName, Set<Schema> visitedSchemas) {
392+
schema = ModelUtils.getReferencedSchema(openAPI, schema);
393+
if (propertyName == null || schema == null || visitedSchemas.contains(schema)) {
394+
return null;
395+
}
396+
visitedSchemas.add(schema);
397+
Map<String, Schema> properties = schema.getProperties();
398+
if (properties != null) {
399+
Schema property = properties.get(propertyName);
400+
if (property != null) {
401+
Schema found = simplifyProperty(property, visitedSchemas);
402+
return found;
403+
}
404+
}
405+
// loop into parent allOfs
406+
List<Schema> allOfs = schema.getAllOf();
407+
if (allOfs != null) {
408+
for (Schema child : allOfs) {
409+
Schema found = findProperty(openAPI, child, propertyName, visitedSchemas);
410+
if (found != null) {
411+
return simplifyProperty(found, visitedSchemas);
412+
}
413+
}
414+
}
415+
return null;
416+
}
417+
418+
/**
419+
* simplify allOf property represented as [$ref, {description: ...}].
420+
*/
421+
private static Schema simplifyProperty(Schema schema, Set<Schema> visitedSchemas) {
422+
if (!ModelUtils.isAllOf(schema)) {
423+
return schema;
424+
}
425+
if (visitedSchemas.contains(schema)) {
426+
return null;
427+
}
428+
visitedSchemas.add(schema);
429+
430+
/*
431+
* handle type+description. For Example:
432+
* petType:
433+
* $ref: '#/components/schemas/PetType'
434+
* description: DOG
435+
*/
436+
Schema found = null;
437+
int count = 0;
438+
for (Schema sc: (List<Schema>)schema.getAllOf()) {
439+
if (ModelUtils.getType(sc) != null || ModelUtils.hasRef(sc)) {
440+
found = sc;
441+
count++;
442+
}
443+
}
444+
if (count == 1) {
445+
return found;
446+
}
447+
// multiple types, too complex.
448+
return null;
449+
450+
}
451+
325452
public static class DiscriminatorData {
326453
private final Discriminator discriminator;
327454

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.openapitools.codegen.java.assertions;
22

3+
import com.github.javaparser.ParseProblemException;
34
import com.github.javaparser.StaticJavaParser;
45
import com.github.javaparser.ast.CompilationUnit;
56
import com.github.javaparser.ast.Node;
@@ -36,7 +37,7 @@ public static JavaFileAssert assertThat(final String source) {
3637
public static JavaFileAssert assertThat(final Path path) {
3738
try {
3839
return new JavaFileAssert(StaticJavaParser.parse(path));
39-
} catch (IOException e) {
40+
} catch (IOException | ParseProblemException e) {
4041
throw new RuntimeException("Exception while reading file: " + path, e);
4142
}
4243
}

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9335,4 +9335,67 @@ public void issue_24232() throws IOException {
93359335
JavaFileAssert.assertThat(files.get("Dummy.java"))
93369336
.fileContains("import org.myorg.MyCustomId;", "import org.myorg.MyCustomKey;");
93379337
}
9338+
9339+
@Test
9340+
public void issue_24769() throws IOException {
9341+
Map<String, File> files = generateFromContract(
9342+
"src/test/resources/3_0/oneOf_issue_24769.yaml", SPRING_BOOT,
9343+
Map.of(USE_SPRING_BOOT4, true)
9344+
);
9345+
9346+
JavaFileAssert.assertThat(files.get("Dog.java"))
9347+
.fileContains("public enum TypeEnum {",
9348+
"DOG(\"DOG\");");
9349+
JavaFileAssert.assertThat(files.get("Cat.java"))
9350+
.fileContains("public enum TypeEnum {",
9351+
"CAT(\"CAT\");");
9352+
JavaFileAssert.assertThat(files.get("Pet.java"))
9353+
.fileDoesNotContain("public enum TypeEnum {")
9354+
.fileContains("public Enum getType();");
9355+
9356+
JavaFileAssert.assertThat(files.get("PetInteger.java"))
9357+
.fileContains("public Integer getIntType();");
9358+
9359+
JavaFileAssert.assertThat(files.get("PetEnumRef.java"))
9360+
.fileContains("public PetEnumType getEnumRefType();");
9361+
9362+
JavaFileAssert.assertThat(files.get("PetWithParent.java"))
9363+
.fileContains("public PetEnumType getPetType();");
9364+
9365+
JavaFileAssert.assertThat(files.get("PetWithAllOf.java"))
9366+
.fileContains("public PetEnumType getTypeAllOf()");
9367+
9368+
JavaFileAssert.assertThat(files.get("PetWithEnum.java"))
9369+
.fileContains("public Enum getEnumType()");
9370+
JavaFileAssert.assertThat(files.get("CatWithEnum.java"))
9371+
.fileContains("public enum EnumTypeEnum {",
9372+
"CAT(\"CAT\");");
9373+
9374+
9375+
JavaFileAssert.assertThat(files.get("PetNoMapping.java"))
9376+
.fileContains("public Enum getType();");
9377+
9378+
}
9379+
9380+
@DataProvider(name = "oneOfDiscriminatorType")
9381+
public Object[][] oneOfDiscriminatorType() {
9382+
return new Object[][]{
9383+
{"/3_0/oneOf_issue_19194.yaml", true, "CargoInterface.java", "public CargoGeneralParameterUnit getUnit();"},
9384+
{"/3_0/oneOf_issue_19194.yaml", false, "CargoInterface.java", "public Enum getUnit();"},
9385+
{"/3_0/oneOf_issue_19194_v2.yaml", false, "CargoParent.java", "public Object getUnit()"},
9386+
{"/3_0/oneof_polymorphism_and_inheritance.yaml", false, "FooRefOrValue.java", "public String getAtType()"}
9387+
};
9388+
}
9389+
9390+
@Test(dataProvider = "oneOfDiscriminatorType")
9391+
public void oneOfDiscriminatorType(String filename, boolean resolveInlineEnum, String fileToCheck, String expectedContains) throws IOException {
9392+
Map<String, File> files = generateFromContract(
9393+
"src/test/resources" + filename, SPRING_BOOT,
9394+
Map.of(USE_SPRING_BOOT4, true), configurator->
9395+
configurator.addInlineSchemaOption("RESOLVE_INLINE_ENUMS", Boolean.toString(resolveInlineEnum))
9396+
);
9397+
JavaFileAssert.assertThat(files.get(fileToCheck))
9398+
.fileContains(expectedContains);
9399+
}
9400+
93389401
}

0 commit comments

Comments
 (0)