From 527c118210b33c1d2c9acde7344619879b7639cb Mon Sep 17 00:00:00 2001 From: Sebastien Rosset Date: Sun, 16 Feb 2020 17:23:00 -0800 Subject: [PATCH] [codegen] Improve java code comments and argument documentation; Fix issue with ComposedSchema that has undeclared properties (#5316) * improve documentation * Add use case of composed schema with additional properties --- docs/usage.md | 7 +- .../codegen/CodegenConstants.java | 5 +- .../codegen/DefaultGenerator.java | 5 +- .../codegen/InlineModelResolver.java | 34 +++++++++ .../codegen/utils/ModelUtils.java | 74 ++++++++++++++++++- 5 files changed, 121 insertions(+), 4 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index bb3fade3f6..c142b55559 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -317,7 +317,12 @@ OPTIONS generator to use (see list command for list) --generate-alias-as-model - Generate alias to map, array as models + Generate model implementation for aliases to map and array schemas. + An 'alias' is an array, map, or list which is defined inline in a + OpenAPI document and becomes a model in the generated code. + A 'map' schema is an object that can have undeclared properties, + i.e. the 'additionalproperties' attribute is set on that object. + An 'array' schema is a list of sub schemas in a OAS document. --git-repo-id Git repo ID, e.g. openapi-generator. diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java index 98ee85bfde..d13d82ae64 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConstants.java @@ -326,7 +326,10 @@ public class CodegenConstants { public static final String OPEN_API_SPEC_NAME = "openAPISpecName"; public static final String GENERATE_ALIAS_AS_MODEL = "generateAliasAsModel"; - public static final String GENERATE_ALIAS_AS_MODEL_DESC = "Generate alias to map, array as models"; + public static final String GENERATE_ALIAS_AS_MODEL_DESC = "Generate model implementation for aliases to map and array schemas. " + + "An 'alias' is an array, map, or list which is defined inline in a OpenAPI document and becomes a model in the generated code. " + + "A 'map' schema is an object that can have undeclared properties, i.e. the 'additionalproperties' attribute is set on that object. " + + "An 'array' schema is a list of sub schemas in a OAS document"; public static final String USE_COMPARE_NET_OBJECTS = "useCompareNetObjects"; public static final String USE_COMPARE_NET_OBJECTS_DESC = "Use KellermanSoftware.CompareNetObjects for deep recursive object comparison. WARNING: this option incurs potential performance impact."; diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java index 774b5159b9..d2f9a078b5 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java @@ -450,7 +450,10 @@ public class DefaultGenerator extends AbstractGenerator implements Generator { LOGGER.info("Model " + name + " not generated since it's a free-form object"); continue; } else if (ModelUtils.isMapSchema(schema)) { // check to see if it's a "map" model - if (!ModelUtils.isGenerateAliasAsModel() && (schema.getProperties() == null || schema.getProperties().isEmpty())) { + // A composed schema (allOf, oneOf, anyOf) is considered a Map schema if the additionalproperties attribute is set + // for that composed schema. However, in the case of a composed schema, the properties are defined or referenced + // in the inner schemas, and the outer schema does not have properties. + if (!ModelUtils.isGenerateAliasAsModel() && !ModelUtils.isComposedSchema(schema) && (schema.getProperties() == null || schema.getProperties().isEmpty())) { // schema without property, i.e. alias to map LOGGER.info("Model " + name + " not generated since it's an alias to map (without property) and `generateAliasAsModel` is set to false (default)"); continue; diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java index a388fa899e..2faf670a93 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java @@ -328,6 +328,33 @@ public class InlineModelResolver { } } + /** + * Flattens properties of inline object schemas that belong to a composed schema into a + * single flat list of properties. This is useful to generate a single or multiple + * inheritance model. + * + * In the example below, codegen may generate a 'Dog' class that extends from the + * generated 'Animal' class. 'Dog' has additional properties 'name', 'age' and 'breed' that + * are flattened as a single list of properties. + * + * Dog: + * allOf: + * - $ref: '#/components/schemas/Animal' + * - type: object + * properties: + * name: + * type: string + * age: + * type: string + * - type: object + * properties: + * breed: + * type: string + * + * @param openAPI the OpenAPI document + * @param key a unique name ofr the composed schema. + * @param children the list of nested schemas within a composed schema (allOf, anyOf, oneOf). + */ private void flattenComposedChildren(OpenAPI openAPI, String key, List children) { if (children == null || children.isEmpty()) { return; @@ -338,6 +365,13 @@ public class InlineModelResolver { if (component instanceof ObjectSchema) { ObjectSchema op = (ObjectSchema) component; if (op.get$ref() == null && op.getProperties() != null && op.getProperties().size() > 0) { + // Note: the call to op.getTitle() can lead to totally unexpected results and should not be + // used to generate the innerModelName. + // If the value of the 'title' attribute happens to match a schema defined elsewhere in + // the specification, 'innerModelName' will be the same as that other schema. + // The 'title' attribute is supposed to be for human consumption, not for code generation. + // OAS authors should not be expected to set a 'title' value that will control the + // code generation logic. String innerModelName = resolveModelName(op.getTitle(), key); Schema innerModel = modelFromProperty(op, innerModelName); String existing = matchGenerated(innerModel); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java index 9ed6afb54b..512d3de9a2 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java @@ -353,6 +353,27 @@ public class ModelUtils { return ref; } + /** + * Return true if the specified schema is an object with a fixed number of properties. + * + * A ObjectSchema differs from an MapSchema in the following way: + * - An ObjectSchema is not extensible, i.e. it has a fixed number of properties. + * - A MapSchema is an object that can be extended with an arbitrary set of properties. + * The payload may include dynamic properties. + * + * For example, an OpenAPI schema is considered an ObjectSchema in the following scenarios: + * + * type: object + * additionalProperties: false + * properties: + * name: + * type: string + * address: + * type: string + * + * @param schema the OAS schema + * @return true if the specified schema is an Object schema. + */ public static boolean isObjectSchema(Schema schema) { if (schema instanceof ObjectSchema) { return true; @@ -370,6 +391,13 @@ public class ModelUtils { return false; } + /** + * Return true if the specified schema is composed, i.e. if it uses + * 'oneOf', 'anyOf' or 'allOf'. + * + * @param schema the OAS schema + * @return true if the specified schema is a Composed schema. + */ public static boolean isComposedSchema(Schema schema) { if (schema instanceof ComposedSchema) { return true; @@ -377,6 +405,39 @@ public class ModelUtils { return false; } + /** + * Return true if the specified 'schema' is an object that can be extended with additional properties. + * Additional properties means a Schema should support all explicitly defined properties plus any + * undeclared properties. + * + * A MapSchema differs from an ObjectSchema in the following way: + * - An ObjectSchema is not extensible, i.e. it has a fixed number of properties. + * - A MapSchema is an object that can be extended with an arbitrary set of properties. + * The payload may include dynamic properties. + * + * Note that isMapSchema returns true for a composed schema (allOf, anyOf, oneOf) that also defines + * additionalproperties. + * + * For example, an OpenAPI schema is considered a MapSchema in the following scenarios: + * + * type: object + * additionalProperties: true + * + * type: object + * additionalProperties: + * type: object + * properties: + * code: + * type: integer + * + * allOf: + * - $ref: '#/components/schemas/Class1' + * - $ref: '#/components/schemas/Class2' + * additionalProperties: true + * + * @param schema the OAS schema + * @return true if the specified schema is a Map schema. + */ public static boolean isMapSchema(Schema schema) { if (schema instanceof MapSchema) { return true; @@ -397,6 +458,12 @@ public class ModelUtils { return false; } + /** + * Return true if the specified schema is an array of items. + * + * @param schema the OAS schema + * @return true if the specified schema is an Array schema. + */ public static boolean isArraySchema(Schema schema) { return (schema instanceof ArraySchema); } @@ -591,7 +658,12 @@ public class ModelUtils { } /** - * Check to see if the schema is a free form object + * Check to see if the schema is a free form object. + * + * A free form object is an object (i.e. 'type: object' in a OAS document) that: + * 1) Does not define properties, and + * 2) Is not a composed schema (no anyOf, oneOf, allOf), and + * 3) additionalproperties is not defined, or additionalproperties: true, or additionalproperties: {}. * * @param schema potentially containing a '$ref' * @return true if it's a free-form object