From 52d65ecde093353f9ca51a8ffd6cd1c8126ada19 Mon Sep 17 00:00:00 2001 From: Mattias-Sehlstedt <60173714+Mattias-Sehlstedt@users.noreply.github.com> Date: Sun, 28 Jun 2026 00:12:21 +0200 Subject: [PATCH] refactor: introduce DiscriminatorUtils to separate handling of discriminator discoverability and construction --- .../openapitools/codegen/DefaultCodegen.java | 112 +------------ .../codegen/utils/DiscriminatorUtils.java | 154 ++++++++++++++++++ 2 files changed, 162 insertions(+), 104 deletions(-) create mode 100644 modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/DiscriminatorUtils.java diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java index efd8983154e3..006bdc827c08 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java @@ -66,6 +66,7 @@ import org.openapitools.codegen.serializer.SerializerUtils; import org.openapitools.codegen.templating.MustacheEngineAdapter; import org.openapitools.codegen.templating.mustache.*; +import org.openapitools.codegen.utils.DiscriminatorUtils; import org.openapitools.codegen.utils.ExamplesUtils; import org.openapitools.codegen.utils.ModelUtils; import org.openapitools.codegen.utils.OneOfImplementorAdditionalData; @@ -318,7 +319,7 @@ apiTemplateFiles are for API outputs only (controllers/handlers). private TemplatingEngineAdapter templatingEngine = new MustacheEngineAdapter(); // flag to indicate whether to use the utils.OneOfImplementorAdditionalData related logic protected boolean useOneOfInterfaces = false; - // whether or not the oneOf imports machinery should add oneOf interfaces as imports in implementing classes + // whether the oneOf imports machinery should add oneOf interfaces as imports in implementing classes protected boolean addOneOfInterfaceImports = false; protected List addOneOfInterfaces = new ArrayList<>(); @@ -577,7 +578,7 @@ public Map postProcessAllModels(Map objs) } // if this is oneOf interface, make sure we include the necessary imports for it addImportsToOneOfInterface(modelsImports); - // + // ensure that no JsonTypeName is created when the parent interface has a discriminator mapping if (cm.discriminator != null && cm.discriminator.getMappedModels() != null && !cm.discriminator.getMappedModels().isEmpty()) { cm.discriminator.getMappedModels().stream() @@ -3453,95 +3454,7 @@ private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, * @param visitedSchemas An array list of visited schemas */ private Discriminator recursiveGetDiscriminator(Schema sc, ArrayList visitedSchemas) { - Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); - Discriminator foundDisc = refSchema.getDiscriminator(); - if (foundDisc != null) { - return foundDisc; - } - - if (this.getLegacyDiscriminatorBehavior()) { - return null; - } - - for (Schema s : visitedSchemas) { - if (s == refSchema) { - return null; - } - } - visitedSchemas.add(refSchema); - - Discriminator disc = new Discriminator(); - if (ModelUtils.isComposedSchema(refSchema)) { - Schema composedSchema = refSchema; - if (composedSchema.getAllOf() != null) { - // If our discriminator is in one of the allOf schemas break when we find it - for (Object allOf : composedSchema.getAllOf()) { - foundDisc = recursiveGetDiscriminator((Schema) allOf, visitedSchemas); - if (foundDisc != null) { - disc.setPropertyName(foundDisc.getPropertyName()); - disc.setMapping(foundDisc.getMapping()); - return disc; - } - } - } - if (ModelUtils.hasOneOf(composedSchema)) { - // All oneOf definitions must contain the discriminator - Integer hasDiscriminatorCnt = 0; - Integer hasNullTypeCnt = 0; - Set discriminatorsPropNames = new HashSet<>(); - for (Object oneOf : composedSchema.getOneOf()) { - if (ModelUtils.isNullType((Schema) oneOf)) { - // The null type does not have a discriminator. Skip. - hasNullTypeCnt++; - continue; - } - foundDisc = recursiveGetDiscriminator((Schema) oneOf, visitedSchemas); - if (foundDisc != null) { - discriminatorsPropNames.add(foundDisc.getPropertyName()); - hasDiscriminatorCnt++; - } - } - if (discriminatorsPropNames.size() > 1) { - once(LOGGER).warn("The oneOf schemas have conflicting discriminator property names. oneOf schemas must have the same property name, but found {}", String.join(", ", discriminatorsPropNames)); - } - if (foundDisc != null && (hasDiscriminatorCnt + hasNullTypeCnt) == composedSchema.getOneOf().size() && discriminatorsPropNames.size() == 1) { - disc.setPropertyName(foundDisc.getPropertyName()); - disc.setMapping(foundDisc.getMapping()); - return disc; - } - // If the scenario when oneOf has two children and one of them is the 'null' type, - // there is no need for a discriminator. - } - if (composedSchema.getAnyOf() != null && composedSchema.getAnyOf().size() != 0) { - // All anyOf definitions must contain the discriminator because a min of one must be selected - Integer hasDiscriminatorCnt = 0; - Integer hasNullTypeCnt = 0; - Set discriminatorsPropNames = new HashSet<>(); - for (Object anyOf : composedSchema.getAnyOf()) { - if (ModelUtils.isNullType((Schema) anyOf)) { - // The null type does not have a discriminator. Skip. - hasNullTypeCnt++; - continue; - } - foundDisc = recursiveGetDiscriminator((Schema) anyOf, visitedSchemas); - if (foundDisc != null) { - discriminatorsPropNames.add(foundDisc.getPropertyName()); - hasDiscriminatorCnt++; - } - } - if (discriminatorsPropNames.size() > 1) { - once(LOGGER).warn("The anyOf schemas have conflicting discriminator property names. anyOf schemas must have the same property name, but found {}", String.join(", ", discriminatorsPropNames)); - } - if (foundDisc != null && (hasDiscriminatorCnt + hasNullTypeCnt) == composedSchema.getAnyOf().size() && discriminatorsPropNames.size() == 1) { - disc.setPropertyName(foundDisc.getPropertyName()); - disc.setMapping(foundDisc.getMapping()); - return disc; - } - // If the scenario when anyOf has two children and one of them is the 'null' type, - // there is no need for a discriminator. - } - } - return null; + return DiscriminatorUtils.recursiveGetDiscriminator(openAPI, this.getLegacyDiscriminatorBehavior(), sc, visitedSchemas); } /** @@ -3767,14 +3680,7 @@ protected CodegenDiscriminator createDiscriminator(String schemaName, Schema sch * @param discriminatorName The name of the discriminator property. */ protected Schema getDiscriminatorSchema(Schema schema, String discriminatorName) { - if (schema.getProperties() == null) { - return null; - } - Schema discSchema = (Schema) schema.getProperties().get(discriminatorName); - if (ModelUtils.isAllOf(discSchema)) { - discSchema = (Schema) discSchema.getAllOf().get(0); - } - return discSchema; + return DiscriminatorUtils.getDiscriminatorSchema(schema, discriminatorName); } /** @@ -3784,9 +3690,7 @@ protected Schema getDiscriminatorSchema(Schema schema, String discriminatorName) * @param discriminatorPropertyName The name of the discriminator property. */ protected String getDiscriminatorPropertyType(Schema schema, String discriminatorPropertyName) { - return Optional.ofNullable(getDiscriminatorSchema(schema, discriminatorPropertyName)) - .map(Schema::get$ref) - .map(ModelUtils::getSimpleRef) + return DiscriminatorUtils.getDiscriminatorPropertyType(schema, discriminatorPropertyName) .map(this::toModelName) .orElseGet(() -> typeMapping.get("string")); } @@ -4522,7 +4426,7 @@ protected void updatePropertyForMap(CodegenProperty property, CodegenProperty in * Update property for map container * * @param property Codegen property - * @return True if the inner most type is enum + * @return True if the innermost type is enum */ protected Boolean isPropertyInnerMostEnum(CodegenProperty property) { CodegenProperty currentProperty = getMostInnerItems(property); @@ -8691,7 +8595,7 @@ public void addOneOfInterfaceModel(Schema cs, String type) { if (((Schema) o).get$ref() == null) { if (cm.discriminator != null && ((Schema) o).get$ref() == null) { // OpenAPI spec states that inline objects should not be considered when discriminator is used - // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#discriminatorObject + // https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#discriminator-object LOGGER.warn("Ignoring inline object in oneOf definition of {}, since discriminator is used", type); } else { LOGGER.warn("Inline models are not supported in oneOf definition right now"); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/DiscriminatorUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/DiscriminatorUtils.java new file mode 100644 index 000000000000..5ed6f5b0761c --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/DiscriminatorUtils.java @@ -0,0 +1,154 @@ +package org.openapitools.codegen.utils; + +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.media.Discriminator; +import io.swagger.v3.oas.models.media.Schema; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.*; + +import static org.openapitools.codegen.utils.OnceLogger.once; + +public class DiscriminatorUtils { + + private static final Logger LOGGER = LoggerFactory.getLogger(DiscriminatorUtils.class); + + private static final String CONFLICTING_DISCRIMINATOR_NAMES = + "The alternative schemas have conflicting discriminator property names. The schemas must have the same property name, but found {}"; + + /** + * Gets the simple ref name of the discriminator property type from the schema. + * + * @param schema The input OAS schema. + * @param discriminatorPropertyName The name of the discriminator property. + * @return referenced type name, or an empty optional if unavailable + */ + public static Optional getDiscriminatorPropertyType(Schema schema, String discriminatorPropertyName) { + return Optional.ofNullable(getDiscriminatorSchema(schema, discriminatorPropertyName)) + .map(Schema::get$ref) + .map(ModelUtils::getSimpleRef); + } + + /** + * Get the Schema for the discriminator type. Requires special handling due to siblings from OAS 3.1. + * An example of a sibling is an enum-ref that has its own description. This will lead to the enum being + * referenced as an allOf that in turn has a ref, rather than a regular ref directly to the enum. + * + * @param schema The input OAS schema. + * @param discriminatorName The name of the discriminator property. + */ + public static Schema getDiscriminatorSchema(Schema schema, String discriminatorName) { + if (schema.getProperties() == null) { + return null; + } + Schema discSchema = (Schema) schema.getProperties().get(discriminatorName); + if (ModelUtils.isAllOf(discSchema)) { + discSchema = (Schema) discSchema.getAllOf().get(0); + } + return discSchema; + } + + /** + * Recursively look in Schema sc for the discriminator and return it + * + * @param openAPI the openAPI specification + * @param legacyDiscriminatorBehavior whether legacy discriminator behavior is enabled + * @param sc The Schema that may contain the discriminator + * @param visitedSchemas an array list of visited schemas + */ + public static Discriminator recursiveGetDiscriminator( + OpenAPI openAPI, + boolean legacyDiscriminatorBehavior, + Schema sc, + ArrayList visitedSchemas) { + Schema refSchema = ModelUtils.getReferencedSchema(openAPI, sc); + Discriminator foundDisc = refSchema.getDiscriminator(); + if (foundDisc != null) { + return foundDisc; + } + + if (legacyDiscriminatorBehavior) { + return null; + } + + for (Schema s : visitedSchemas) { + if (s == refSchema) { + return null; + } + } + visitedSchemas.add(refSchema); + + Discriminator disc = new Discriminator(); + if (ModelUtils.isComposedSchema(refSchema)) { + Schema composedSchema = refSchema; + if (composedSchema.getAllOf() != null) { + // If our discriminator is in one of the allOf schemas break when we find it + for (Object allOf : composedSchema.getAllOf()) { + foundDisc = recursiveGetDiscriminator(openAPI, legacyDiscriminatorBehavior, (Schema) allOf, visitedSchemas); + if (foundDisc != null) { + disc.setPropertyName(foundDisc.getPropertyName()); + disc.setMapping(foundDisc.getMapping()); + return disc; + } + } + } + if (ModelUtils.hasOneOf(composedSchema)) { + foundDisc = getDiscriminatorFromAlternatives(openAPI, legacyDiscriminatorBehavior, composedSchema.getOneOf(), visitedSchemas); + if (foundDisc != null) { + return foundDisc; + } + } + if (ModelUtils.hasAnyOf(composedSchema)) { + return getDiscriminatorFromAlternatives(openAPI, legacyDiscriminatorBehavior, composedSchema.getAnyOf(), visitedSchemas); + } + } + return null; + } + + /** + * Check whether the alternative schemas share a discriminator, and if they do, return it + * + * @param openAPI the openAPI specification + * @param legacyDiscriminatorBehavior whether legacy discriminator behavior is enabled + * @param alternativeSchemas list of schemas that should be checked for a shared discriminator + * @param visitedSchemas an array list of visited schemas + * @return the discriminator if the alternatives correctly shares one, otherwise null + */ + private static Discriminator getDiscriminatorFromAlternatives( + OpenAPI openAPI, + boolean legacyDiscriminatorBehavior, + List alternativeSchemas, + ArrayList visitedSchemas) { + Discriminator discriminator = new Discriminator(); + Discriminator foundDisc = null; + Integer hasDiscriminatorCnt = 0; + Integer hasNullTypeCnt = 0; + Set discriminatorsPropNames = new HashSet<>(); + for (Object alternative : alternativeSchemas) { + if (ModelUtils.isNullType((Schema) alternative)) { + // The null type does not have a discriminator. Skip. + hasNullTypeCnt++; + continue; + } + foundDisc = recursiveGetDiscriminator(openAPI, legacyDiscriminatorBehavior, (Schema) alternative, visitedSchemas); + if (foundDisc != null) { + discriminatorsPropNames.add(foundDisc.getPropertyName()); + hasDiscriminatorCnt++; + } + } + if (discriminatorsPropNames.size() > 1) { + once(LOGGER).warn(CONFLICTING_DISCRIMINATOR_NAMES, String.join(", ", discriminatorsPropNames)); + } + boolean allAlternativesHaveADiscriminator = hasDiscriminatorCnt + hasNullTypeCnt == alternativeSchemas.size(); + if (foundDisc != null && allAlternativesHaveADiscriminator && discriminatorsPropNames.size() == 1) { + discriminator.setPropertyName(foundDisc.getPropertyName()); + discriminator.setMapping(foundDisc.getMapping()); + return discriminator; + } + // If the scenario when composite schema has two children and one of them is the 'null' type, + // there is no need for a discriminator. + return null; + } + +}