From eb3114488223420a1c1c23271142485e6dee3ec0 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Sat, 1 Feb 2020 08:07:25 -0800 Subject: [PATCH 01/22] initial commit for null type --- .../languages/PythonClientCodegen.java | 5 ++++- .../codegen/utils/ModelUtils.java | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java index c621ebef024d..9b5155201b32 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java @@ -705,6 +705,9 @@ private String toExampleValueRecursive(Schema schema, List included_sche for (int i=0 ; i< indentation ; i++) indentation_string += " "; String example = super.toExampleValue(schema); + if (ModelUtils.isNullSchema(schema) && null!=example) { + return "None"; + } // correct "true"s into "True"s, since super.toExampleValue uses "toString()" on Java booleans if (ModelUtils.isBooleanSchema(schema) && null!=example) { if ("false".equalsIgnoreCase(example)) example = "False"; @@ -866,7 +869,7 @@ private String toExampleValueRecursive(Schema schema, List included_sche } example +=")"; } else { - LOGGER.warn("Type " + schema.getType() + " not handled properly in toExampleValue"); + LOGGER.warn("Type '" + schema.getType() + "' not handled properly in toExampleValue"); } if (ModelUtils.isStringSchema(schema)) { 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 3872b11d8723..98cdddfa70e7 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 @@ -397,6 +397,27 @@ public static boolean isStringSchema(Schema schema) { return false; } + /** + * isNullSchema returns true if the specified schema is the 'null' type. + * + * The 'null' type is supported in OAS 3.1 and above. It is not supported + * in OAS 2.0 and OAS 3.0.x. + * + * For example, the "null" type could be used to specify that a value must + * either be null or a specified type: + * + * OptionalOrder: + * oneOf: + * - type: 'null' + * - $ref: '#/components/schemas/Order' + */ + public static boolean isNullSchema(Schema schema) { + if ("null".equals(schema.getType())) { + return true; + } + return false; + } + public static boolean isIntegerSchema(Schema schema) { if (schema instanceof IntegerSchema) { return true; From 56da6220fd72b6717a776ab8ad7f7b7712d4e798 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Sat, 1 Feb 2020 14:12:04 -0800 Subject: [PATCH 02/22] handle scenario when one element in composed schema is 'null' type --- .../codegen/utils/ModelUtils.java | 49 ++++++++++++++++--- 1 file changed, 43 insertions(+), 6 deletions(-) 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 98cdddfa70e7..bc0f0cf8f337 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 @@ -963,8 +963,28 @@ public static List getInterfaces(ComposedSchema composed) { } /** - * Get the parent model name from the schemas (allOf, anyOf, oneOf). - * If there are multiple parents, return the first one. + * Get the parent model name from the composed schema (allOf, anyOf, oneOf). + * It traverses the OAS model (possibly resolving $ref) to determine schemas + * that specify a determinator. + * If there are multiple elements in the composed schema and it is not clear + * which one should be the parent, return null. + * + * For example, given the following OAS spec, the parent of 'Dog' is Animal + * because 'Animal' specifies a discriminator. + * + * animal: + * type: object + * discriminator: + * propertyName: type + * properties: + * type: string + * + * dog: + * allOf: + * - $ref: '#/components/schemas/animal' + * - type: object + * properties: + * breed: string * * @param composedSchema schema (alias or direct reference) * @param allSchemas all schemas @@ -974,7 +994,8 @@ public static String getParentName(ComposedSchema composedSchema, Map interfaces = getInterfaces(composedSchema); List refedParentNames = new ArrayList<>(); - + int nullSchemaChildrenCount = 0; + boolean hasAmbiguousParents = false; if (interfaces != null && !interfaces.isEmpty()) { for (Schema schema : interfaces) { // get the actual schema @@ -991,17 +1012,33 @@ public static String getParentName(ComposedSchema composedSchema, Map Date: Sat, 1 Feb 2020 14:36:14 -0800 Subject: [PATCH 03/22] support 'null' type --- .../src/main/java/org/openapitools/codegen/DefaultCodegen.java | 2 ++ 1 file changed, 2 insertions(+) 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 6d704fe95092..d49c276052d9 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 @@ -1708,6 +1708,8 @@ private String getSingleSchemaType(Schema schema) { private String getPrimitiveType(Schema schema) { if (schema == null) { throw new RuntimeException("schema cannot be null in getPrimitiveType"); + } else if (ModelUtils.isNullSchema(schema)) { + return "null"; } else if (ModelUtils.isStringSchema(schema) && "number".equals(schema.getFormat())) { // special handle of type: string, format: number return "BigDecimal"; From 8ff92365fba385337193f07816632ce66fddfdc1 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Sun, 2 Feb 2020 09:19:05 -0800 Subject: [PATCH 04/22] fix null pointer exception while evaluating recommendations --- .../codegen/validations/oas/OpenApiEvaluator.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java index e28a69b4386b..9132c7d0e224 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java @@ -87,7 +87,10 @@ public ValidationResult validate(OpenAPI specification) { } } - parameters.forEach(parameter -> validationResult.consume(parameterValidations.validate(parameter))); + parameters.forEach(parameter -> { + parameter = ModelUtils.getReferencedParameter(specification, parameter); + validationResult.consume(parameterValidations.validate(parameter)); + }); return validationResult; } From 30111a53bb74a0e3953730482b882280a8d417ca Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Sun, 2 Feb 2020 10:14:35 -0800 Subject: [PATCH 05/22] add validation for null type and type array --- .../oas/OpenApiSchemaValidations.java | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java index bfb829a8010f..8bb484537622 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -2,10 +2,14 @@ import io.swagger.v3.oas.models.media.ComposedSchema; import io.swagger.v3.oas.models.media.Schema; + +import org.openapitools.codegen.utils.ModelUtils; import org.openapitools.codegen.validation.GenericValidator; import org.openapitools.codegen.validation.ValidationRule; import java.util.ArrayList; +import java.util.List; +import java.util.Locale; /** * A standalone instance for evaluating rules and recommendations related to OAS {@link Schema} @@ -21,6 +25,25 @@ class OpenApiSchemaValidations extends GenericValidator { OpenApiSchemaValidations::checkOneOfWithProperties )); } + if (ruleConfiguration.isEnableSchemaTypeRecommendation()) { + rules.add(ValidationRule.warn( + "Schema defines uses the 'null' type.", + "The 'null' type is not supported in OpenAPI 3.0.x. It is supported in OpenAPI 3.1 and above. While our tooling supports this, it may cause issues with other tools.", + OpenApiSchemaValidations::checkNullType + )); + rules.add(ValidationRule.warn( + "Schema defines uses a type array.", + "The type array is not supported in OpenAPI 3.0.x. It is supported in OpenAPI 3.1 and above. While our tooling supports this, it may cause issues with other tools.", + OpenApiSchemaValidations::checkTypeArray + )); + } + if (ruleConfiguration.isEnableNullableAttributeRecommendation()) { + rules.add(ValidationRule.warn( + "Schema uses the 'nullable' attribute.", + "The 'nullable' attribute is deprecated in OpenAPI 3.1, and may no longer be supported in future releases. Consider migrating to 'null' type.", + OpenApiSchemaValidations::checkNullableAttribute + )); + } } } @@ -54,4 +77,65 @@ private static ValidationRule.Result checkOneOfWithProperties(Schema schema) { } return result; } + + /** + * JSON Schema specifies type: 'null'. + *

+ * 'null' type is supported in OpenAPI Specification 3.1 and above. It is not supported in OpenAPI 3.0.x. + * + * @param schema An input schema, regardless of the type of schema + * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} + */ + private static ValidationRule.Result checkNullType(Schema schema) { + ValidationRule.Result result = ValidationRule.Pass.empty(); + + if (ModelUtils.isNullSchema(schema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, "Specification is version '%s' %s uses a 'null' type.", "3.0.1", schema.getName())); + return result; + } + if (schema instanceof ComposedSchema) { + final ComposedSchema composed = (ComposedSchema) schema; + List interfaces = ModelUtils.getInterfaces(composed); + if (!interfaces.isEmpty()) { + for (Schema interfaceSchema : interfaces) { + if (ModelUtils.isNullSchema(interfaceSchema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, "Specification is version '%s' %s uses a 'null' type.", "3.0.1", + interfaceSchema.getName())); + return result; + } + } + } + } + return result; + } + + /** + * JSON Schema uses a type array. + *

+ * A type array is supported in OpenAPI Specification 3.1 and above. It is not supported in OpenAPI 3.0.x. + * + * @param schema An input schema, regardless of the type of schema + * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} + */ + private static ValidationRule.Result checkTypeArray(Schema schema) { + ValidationRule.Result result = ValidationRule.Pass.empty(); + // TODO + return result; + } + + /** + * JSON Schema uses the 'nullable' attribute. + *

+ * The 'nullable' attribute is supported in OpenAPI Specification 3.0.x, but it is deprecated in OpenAPI 3.1 and above. + * + * @param schema An input schema, regardless of the type of schema + * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} + */ + private static ValidationRule.Result checkNullableAttribute(Schema schema) { + ValidationRule.Result result = ValidationRule.Pass.empty(); + // TODO + return result; + } } From 68bbe2d7ebcea8143de683ce75ca8c8a0118e61a Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Sun, 2 Feb 2020 10:15:07 -0800 Subject: [PATCH 06/22] add validation for null type and type array --- .../validations/oas/RuleConfiguration.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java index c8238ef590e4..840bd0041a7c 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java @@ -10,6 +10,8 @@ public class RuleConfiguration { private boolean enableApacheNginxUnderscoreRecommendation = defaultedBoolean(propertyPrefix + ".apache-nginx-underscore", true); private boolean enableOneOfWithPropertiesRecommendation = defaultedBoolean(propertyPrefix + ".oneof-properties-ambiguity", true); private boolean enableUnusedSchemasRecommendation = defaultedBoolean(propertyPrefix + ".unused-schemas", true); + private boolean enableSchemaTypeRecommendation = defaultedBoolean(propertyPrefix + ".schema-type", true); + private boolean enableNullableAttributeRecommendation = defaultedBoolean(propertyPrefix + ".nullable-deprecated", true); private boolean enableApiRequestUriWithBodyRecommendation = defaultedBoolean(propertyPrefix + ".anti-patterns.uri-unexpected-body", true); @@ -90,6 +92,60 @@ public void setEnableOneOfWithPropertiesRecommendation(boolean enableOneOfWithPr this.enableOneOfWithPropertiesRecommendation = enableOneOfWithPropertiesRecommendation; } + /** + * Enable or Disable the recommendation check for schemas containing type definitions, specifically + * for changes between OpenAPI 3.0.x and 3.1. + * + *

+ * For more details, see {@link RuleConfiguration#isEnableSchemaTypeRecommendation()} + * + * @param enableSchemaTypeRecommendation true to enable, false to disable + */ + public void setEnableSchemaTypeRecommendation(boolean enableSchemaTypeRecommendation) { + this.enableSchemaTypeRecommendation = enableSchemaTypeRecommendation; + } + + /** + * Gets whether the recommendation check for for schemas containing type definitions. + *

+ * In OpenAPI 3.0.x, the "type" attribute must be a string value. + * In OpenAPI 3.1, the type attribute may be: + * A string value + * The 'null' value + * An array containing primitive types and the 'null' value. + * + * @return true if enabled, false if disabled + */ + public boolean isEnableSchemaTypeRecommendation() { + return enableSchemaTypeRecommendation; + } + + /** + * Enable or Disable the recommendation check for the 'nullable' attribute. + * + *

+ * For more details, see {@link RuleConfiguration#isEnableNullableAttributeRecommendation()} + * + * @param enableNullableAttributeRecommendation true to enable, false to disable + */ + public void setEnableNullableAttributeRecommendation(boolean enableNullableAttributeRecommendation) { + this.enableNullableAttributeRecommendation = enableNullableAttributeRecommendation; + } + + /** + * Gets whether the recommendation check for for schemas containing the 'nullable' attribute. + *

+ * In OpenAPI 3.0.x, the "nullable" attribute is supported. However, because it is deprecated in 3.1 + * and above, a warning is logged to prepare for OpenAPI 3.1 recommendations. + * In OpenAPI 3.1, the 'nullable' attribute is deprecated. Instead the OpenAPI specification should + * use the 'null' type. + * + * @return true if enabled, false if disabled + */ + public boolean isEnableNullableAttributeRecommendation() { + return enableNullableAttributeRecommendation; + } + /** * Get whether recommendations are enabled. * From 20590358b70f8501abcffe50a799d965ec724aa8 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 5 Feb 2020 17:56:28 +0000 Subject: [PATCH 07/22] Add openAPI attribute to validation and recommendation --- .../codegen/utils/ModelUtils.java | 28 +++++++++++-- .../validations/oas/OpenApiEvaluator.java | 15 +++++-- .../oas/OpenApiParameterValidations.java | 5 ++- .../oas/OpenApiSchemaValidations.java | 9 ++-- .../oas/OpenApiSecuritySchemeValidations.java | 6 ++- .../validations/oas/OperationWrapper.java | 15 ++++++- .../validations/oas/ParameterWrapper.java | 41 +++++++++++++++++++ .../validations/oas/SchemaWrapper.java | 41 +++++++++++++++++++ .../oas/SecuritySchemeWrapper.java | 41 +++++++++++++++++++ .../oas/OpenApiOperationValidationsTest.java | 6 +-- .../oas/OpenApiParameterValidationsTest.java | 6 +-- .../oas/OpenApiSchemaValidationsTest.java | 6 +-- .../OpenApiSecuritySchemeValidationsTest.java | 6 +-- 13 files changed, 197 insertions(+), 28 deletions(-) create mode 100644 modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/ParameterWrapper.java create mode 100644 modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SchemaWrapper.java create mode 100644 modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SecuritySchemeWrapper.java 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 3872b11d8723..9ae3f6ed3215 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 @@ -942,8 +942,28 @@ public static List getInterfaces(ComposedSchema composed) { } /** - * Get the parent model name from the schemas (allOf, anyOf, oneOf). - * If there are multiple parents, return the first one. + * Get the parent model name from the composed schema (allOf, anyOf, oneOf). + * It traverses the OAS model (possibly resolving $ref) to determine schemas + * that specify a determinator. + * If there are multiple elements in the composed schema and it is not clear + * which one should be the parent, return null. + * + * For example, given the following OAS spec, the parent of 'Dog' is Animal + * because 'Animal' specifies a discriminator. + * + * animal: + * type: object + * discriminator: + * propertyName: type + * properties: + * type: string + * + * dog: + * allOf: + * - $ref: '#/components/schemas/animal' + * - type: object + * properties: + * breed: string * * @param composedSchema schema (alias or direct reference) * @param allSchemas all schemas @@ -953,7 +973,6 @@ public static String getParentName(ComposedSchema composedSchema, Map interfaces = getInterfaces(composedSchema); List refedParentNames = new ArrayList<>(); - if (interfaces != null && !interfaces.isEmpty()) { for (Schema schema : interfaces) { // get the actual schema @@ -980,7 +999,8 @@ public static String getParentName(ComposedSchema composedSchema, Map schemas = ModelUtils.getSchemas(specification); - schemas.forEach((key, schema) -> validationResult.consume(schemaValidations.validate(schema))); + schemas.forEach((key, schema) -> { + SchemaWrapper wrapper = new SchemaWrapper(specification, schema); + validationResult.consume(schemaValidations.validate(wrapper)); + }); List parameters = new ArrayList<>(50); @@ -68,7 +71,7 @@ public ValidationResult validate(OpenAPI specification) { parameters.addAll(op.getParameters()); } - OperationWrapper wrapper = new OperationWrapper(op, httpMethod); + OperationWrapper wrapper = new OperationWrapper(specification, op, httpMethod); validationResult.consume(operationValidations.validate(wrapper)); } }); @@ -79,7 +82,10 @@ public ValidationResult validate(OpenAPI specification) { if (components != null) { Map securitySchemes = components.getSecuritySchemes(); if (securitySchemes != null && !securitySchemes.isEmpty()) { - securitySchemes.values().forEach(securityScheme -> validationResult.consume(securitySchemeValidations.validate(securityScheme))); + securitySchemes.values().forEach(securityScheme -> { + SecuritySchemeWrapper wrapper = new SecuritySchemeWrapper(specification, securityScheme); + validationResult.consume(securitySchemeValidations.validate(wrapper)); + }); } if (components.getParameters() != null) { @@ -89,7 +95,8 @@ public ValidationResult validate(OpenAPI specification) { parameters.forEach(parameter -> { parameter = ModelUtils.getReferencedParameter(specification, parameter); - validationResult.consume(parameterValidations.validate(parameter)); + ParameterWrapper wrapper = new ParameterWrapper(specification, parameter); + validationResult.consume(parameterValidations.validate(wrapper)); }); return validationResult; diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidations.java index 0e3585585a03..d7bf3ff2e401 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidations.java @@ -12,7 +12,7 @@ /** * A standalone instance for evaluating rules and recommendations related to OAS {@link Parameter} */ -class OpenApiParameterValidations extends GenericValidator { +class OpenApiParameterValidations extends GenericValidator { OpenApiParameterValidations(RuleConfiguration ruleConfiguration) { super(new ArrayList<>()); if (ruleConfiguration.isEnableRecommendations()) { @@ -32,7 +32,8 @@ class OpenApiParameterValidations extends GenericValidator { * @param parameter Any spec doc parameter. The method will handle {@link HeaderParameter} evaluation. * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} with details "[key] contains an underscore." */ - private static ValidationRule.Result apacheNginxHeaderCheck(Parameter parameter) { + private static ValidationRule.Result apacheNginxHeaderCheck(ParameterWrapper parameterWrapper) { + Parameter parameter = parameterWrapper.getParameter(); if (parameter == null || !parameter.getIn().equals("header")) return ValidationRule.Pass.empty(); ValidationRule.Result result = ValidationRule.Pass.empty(); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java index bfb829a8010f..d1f6478366fe 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -2,6 +2,8 @@ import io.swagger.v3.oas.models.media.ComposedSchema; import io.swagger.v3.oas.models.media.Schema; + +import org.openapitools.codegen.utils.ModelUtils; import org.openapitools.codegen.validation.GenericValidator; import org.openapitools.codegen.validation.ValidationRule; @@ -10,7 +12,7 @@ /** * A standalone instance for evaluating rules and recommendations related to OAS {@link Schema} */ -class OpenApiSchemaValidations extends GenericValidator { +class OpenApiSchemaValidations extends GenericValidator { OpenApiSchemaValidations(RuleConfiguration ruleConfiguration) { super(new ArrayList<>()); if (ruleConfiguration.isEnableRecommendations()) { @@ -34,10 +36,11 @@ class OpenApiSchemaValidations extends GenericValidator { * Because of this ambiguity in the spec about what is non-standard about oneOf support, we'll warn as a recommendation that * properties on the schema defining oneOf relationships may not be intentional in the OpenAPI Specification. * - * @param schema An input schema, regardless of the type of schema + * @param schemaWrapper An input schema, regardless of the type of schema * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} */ - private static ValidationRule.Result checkOneOfWithProperties(Schema schema) { + private static ValidationRule.Result checkOneOfWithProperties(SchemaWrapper schemaWrapper) { + Schema schema = schemaWrapper.getSchema(); ValidationRule.Result result = ValidationRule.Pass.empty(); if (schema instanceof ComposedSchema) { diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidations.java index 53bc3f5224e4..c834e9522cf6 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidations.java @@ -1,6 +1,7 @@ package org.openapitools.codegen.validations.oas; import io.swagger.v3.oas.models.security.SecurityScheme; +import io.swagger.v3.oas.models.OpenAPI; import org.apache.commons.lang3.StringUtils; import org.openapitools.codegen.validation.GenericValidator; import org.openapitools.codegen.validation.ValidationRule; @@ -11,7 +12,7 @@ /** * A standalone instance for evaluating rules and recommendations related to OAS {@link SecurityScheme} */ -class OpenApiSecuritySchemeValidations extends GenericValidator { +class OpenApiSecuritySchemeValidations extends GenericValidator { OpenApiSecuritySchemeValidations(RuleConfiguration ruleConfiguration) { super(new ArrayList<>()); if (ruleConfiguration.isEnableRecommendations()) { @@ -31,7 +32,8 @@ class OpenApiSecuritySchemeValidations extends GenericValidator * @param securityScheme Security schemes are often used as header parameters (e.g. APIKEY). * @return true if the check succeeds (header does not have an underscore, e.g. 'api_key') */ - private static ValidationRule.Result apacheNginxHeaderCheck(SecurityScheme securityScheme) { + private static ValidationRule.Result apacheNginxHeaderCheck(SecuritySchemeWrapper securitySchemeWrapper) { + SecurityScheme securityScheme = securitySchemeWrapper.getSecurityScheme(); if (securityScheme == null || securityScheme.getIn() != SecurityScheme.In.HEADER) return ValidationRule.Pass.empty(); ValidationRule.Result result = ValidationRule.Pass.empty(); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OperationWrapper.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OperationWrapper.java index c2e3b249fdac..5e4cbce630bd 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OperationWrapper.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OperationWrapper.java @@ -2,12 +2,14 @@ import io.swagger.v3.oas.models.Operation; import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.OpenAPI; /** * Encapsulates an operation with its HTTP Method. In OAS, the {@link PathItem} structure contains more than what we'd * want to evaluate for operation-only checks. */ public class OperationWrapper { + OpenAPI specification; private Operation operation; private PathItem.HttpMethod httpMethod; @@ -17,7 +19,8 @@ public class OperationWrapper { * @param operation The operation instances to wrap * @param httpMethod The http method to wrap */ - OperationWrapper(Operation operation, PathItem.HttpMethod httpMethod) { + OperationWrapper(OpenAPI specification, Operation operation, PathItem.HttpMethod httpMethod) { + this.specification = specification; this.operation = operation; this.httpMethod = httpMethod; } @@ -39,4 +42,14 @@ public Operation getOperation() { public PathItem.HttpMethod getHttpMethod() { return httpMethod; } + + + /** + * Returns the OpenAPI specification. + * + * @return The the OpenAPI specification. + */ + public OpenAPI getOpenAPI() { + return specification; + } } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/ParameterWrapper.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/ParameterWrapper.java new file mode 100644 index 000000000000..7d74d5aae905 --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/ParameterWrapper.java @@ -0,0 +1,41 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.parameters.Parameter; +import io.swagger.v3.oas.models.OpenAPI; + +/** + * Encapsulates an OAS parameter. + */ +public class ParameterWrapper { + OpenAPI specification; + private Parameter parameter; + + /** + * Constructs a new instance of {@link ParameterWrapper} + * + * @param specification The OAS specification + * @param parameter The OAS parameter + */ + ParameterWrapper(OpenAPI specification, Parameter parameter) { + this.specification = specification; + this.parameter = parameter; + } + + /** + * Return the OAS parameter + * + * @return the OAS parameter + */ + public Parameter getParameter() { + return parameter; + } + + /** + * Returns the OpenAPI specification. + * + * @return The the OpenAPI specification. + */ + public OpenAPI getOpenAPI() { + return specification; + } +} diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SchemaWrapper.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SchemaWrapper.java new file mode 100644 index 000000000000..5b500035b0e2 --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SchemaWrapper.java @@ -0,0 +1,41 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.oas.models.OpenAPI; + +/** + * Encapsulates an OAS schema. + */ +public class SchemaWrapper { + OpenAPI specification; + private Schema schema; + + /** + * Constructs a new instance of {@link SchemaWrapper} + * + * @param specification The OAS specification + * @param schema The OAS schema + */ + SchemaWrapper(OpenAPI specification, Schema schema) { + this.specification = specification; + this.schema = schema; + } + + /** + * Return the OAS schema + * + * @return the OAS schema + */ + public Schema getSchema() { + return schema; + } + + /** + * Returns the OpenAPI specification. + * + * @return The the OpenAPI specification. + */ + public OpenAPI getOpenAPI() { + return specification; + } +} diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SecuritySchemeWrapper.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SecuritySchemeWrapper.java new file mode 100644 index 000000000000..c089eeba946b --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/SecuritySchemeWrapper.java @@ -0,0 +1,41 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.security.SecurityScheme; + +/** + * Encapsulates an OAS parameter. + */ +public class SecuritySchemeWrapper { + OpenAPI specification; + private SecurityScheme securityScheme; + + /** + * Constructs a new instance of {@link SecuritySchemeWrapper} + * + * @param specification The OAS specification + * @param securityScheme The OAS securityScheme + */ + SecuritySchemeWrapper(OpenAPI specification, SecurityScheme securityScheme) { + this.specification = specification; + this.securityScheme = securityScheme; + } + + /** + * Return the OAS securityScheme + * + * @return the OAS securityScheme + */ + public SecurityScheme getSecurityScheme() { + return securityScheme; + } + + /** + * Returns the OpenAPI specification. + * + * @return The the OpenAPI specification. + */ + public OpenAPI getOpenAPI() { + return specification; + } +} diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidationsTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidationsTest.java index c4d911563eca..dacd58c9b8df 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidationsTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidationsTest.java @@ -47,7 +47,7 @@ public void testGetOrHeadWithBody(PathItem.HttpMethod method, String operationId op.setRequestBody(body); } - ValidationResult result = validator.validate(new OperationWrapper(op, method)); + ValidationResult result = validator.validate(new OperationWrapper(null, op, method)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -77,7 +77,7 @@ public void testGetOrHeadWithBodyWithDisabledRecommendations(PathItem.HttpMethod op.setRequestBody(body); } - ValidationResult result = validator.validate(new OperationWrapper(op, method)); + ValidationResult result = validator.validate(new OperationWrapper(null, op, method)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -103,7 +103,7 @@ public void testGetOrHeadWithBodyWithDisabledRule(PathItem.HttpMethod method, St op.setRequestBody(body); } - ValidationResult result = validator.validate(new OperationWrapper(op, method)); + ValidationResult result = validator.validate(new OperationWrapper(null, op, method)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidationsTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidationsTest.java index a242a355e3a9..2ed46fb5bc83 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidationsTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidationsTest.java @@ -22,7 +22,7 @@ public void testApacheNginxWithDisabledRecommendations(String in, String key, bo parameter.setIn(in); parameter.setName(key); - ValidationResult result = validator.validate(parameter); + ValidationResult result = validator.validate(new ParameterWrapper(null, parameter)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -43,7 +43,7 @@ public void testApacheNginxWithDisabledRule(String in, String key, boolean match parameter.setIn(in); parameter.setName(key); - ValidationResult result = validator.validate(parameter); + ValidationResult result = validator.validate(new ParameterWrapper(null, parameter)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -64,7 +64,7 @@ public void testDefaultRecommendationApacheNginx(String in, String key, boolean parameter.setIn(in); parameter.setName(key); - ValidationResult result = validator.validate(parameter); + ValidationResult result = validator.validate(new ParameterWrapper(null, parameter)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidationsTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidationsTest.java index 2f8ad8ce5c98..b2b2070ebef1 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidationsTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidationsTest.java @@ -18,7 +18,7 @@ public void testDefaultRecommendationOneOfWithSiblingProperties(Schema schema, b config.setEnableRecommendations(true); OpenApiSchemaValidations validator = new OpenApiSchemaValidations(config); - ValidationResult result = validator.validate(schema); + ValidationResult result = validator.validate(new SchemaWrapper(null, schema)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -39,7 +39,7 @@ public void testOneOfWithSiblingPropertiesDisabledRecommendations(Schema schema, config.setEnableRecommendations(false); OpenApiSchemaValidations validator = new OpenApiSchemaValidations(config); - ValidationResult result = validator.validate(schema); + ValidationResult result = validator.validate(new SchemaWrapper(null, schema)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -56,7 +56,7 @@ public void testOneOfWithSiblingPropertiesDisabledRule(Schema schema, boolean ma config.setEnableOneOfWithPropertiesRecommendation(false); OpenApiSchemaValidations validator = new OpenApiSchemaValidations(config); - ValidationResult result = validator.validate(schema); + ValidationResult result = validator.validate(new SchemaWrapper(null, schema)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidationsTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidationsTest.java index aae32dd30126..a0df21ca1783 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidationsTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidationsTest.java @@ -20,7 +20,7 @@ public void testApacheNginxWithDisabledRecommendations(SecurityScheme.In in, Str SecurityScheme securityScheme = new SecurityScheme().in(in).name(key); - ValidationResult result = validator.validate(securityScheme); + ValidationResult result = validator.validate(new SecuritySchemeWrapper(null, securityScheme)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -39,7 +39,7 @@ public void testApacheNginxWithDisabledRule(SecurityScheme.In in, String key, bo SecurityScheme securityScheme = new SecurityScheme().in(in).name(key); - ValidationResult result = validator.validate(securityScheme); + ValidationResult result = validator.validate(new SecuritySchemeWrapper(null, securityScheme)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() @@ -58,7 +58,7 @@ public void testDefaultRecommendationApacheNginx(SecurityScheme.In in, String ke SecurityScheme securityScheme = new SecurityScheme().in(in).name(key); - ValidationResult result = validator.validate(securityScheme); + ValidationResult result = validator.validate(new SecuritySchemeWrapper(null, securityScheme)); Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() From bb911043b38772a5938a809412a82ac79411a64f Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 5 Feb 2020 10:33:06 -0800 Subject: [PATCH 08/22] add validation for nullable and null --- .../oas/OpenApiSchemaValidations.java | 90 +++++++++++-------- 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java index 6e707bde3aaa..2fdaba29c15b 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -4,6 +4,7 @@ import io.swagger.v3.oas.models.media.Schema; import org.openapitools.codegen.utils.ModelUtils; +import org.openapitools.codegen.utils.SemVer; import org.openapitools.codegen.validation.GenericValidator; import org.openapitools.codegen.validation.ValidationRule; @@ -31,11 +32,6 @@ class OpenApiSchemaValidations extends GenericValidator { "The 'null' type is not supported in OpenAPI 3.0.x. It is supported in OpenAPI 3.1 and above. While our tooling supports this, it may cause issues with other tools.", OpenApiSchemaValidations::checkNullType )); - rules.add(ValidationRule.warn( - "Schema defines uses a type array.", - "The type array is not supported in OpenAPI 3.0.x. It is supported in OpenAPI 3.1 and above. While our tooling supports this, it may cause issues with other tools.", - OpenApiSchemaValidations::checkTypeArray - )); } if (ruleConfiguration.isEnableNullableAttributeRecommendation()) { rules.add(ValidationRule.warn( @@ -87,24 +83,31 @@ private static ValidationRule.Result checkOneOfWithProperties(SchemaWrapper sche * @param schema An input schema, regardless of the type of schema * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} */ - private static ValidationRule.Result checkNullType(Schema schema) { + private static ValidationRule.Result checkNullType(SchemaWrapper schemaWrapper) { + Schema schema = schemaWrapper.getSchema(); ValidationRule.Result result = ValidationRule.Pass.empty(); - - if (ModelUtils.isNullSchema(schema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, "Specification is version '%s' %s uses a 'null' type.", "3.0.1", schema.getName())); - return result; - } - if (schema instanceof ComposedSchema) { - final ComposedSchema composed = (ComposedSchema) schema; - List interfaces = ModelUtils.getInterfaces(composed); - if (!interfaces.isEmpty()) { - for (Schema interfaceSchema : interfaces) { - if (ModelUtils.isNullSchema(interfaceSchema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, "Specification is version '%s' %s uses a 'null' type.", "3.0.1", - interfaceSchema.getName())); - return result; + SemVer version = new SemVer(schemaWrapper.getOpenAPI().getOpenapi()); + if (version.atLeast("3.0") && version.compareTo(new SemVer("3.1")) < 0) { + // OAS spec is 3.0.x + if (ModelUtils.isNullSchema(schema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", + schema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); + return result; + } + if (schema instanceof ComposedSchema) { + final ComposedSchema composed = (ComposedSchema) schema; + List interfaces = ModelUtils.getInterfaces(composed); + if (!interfaces.isEmpty()) { + for (Schema interfaceSchema : interfaces) { + if (ModelUtils.isNullSchema(interfaceSchema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", + interfaceSchema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); + return result; + } } } } @@ -112,20 +115,6 @@ private static ValidationRule.Result checkNullType(Schema schema) { return result; } - /** - * JSON Schema uses a type array. - *

- * A type array is supported in OpenAPI Specification 3.1 and above. It is not supported in OpenAPI 3.0.x. - * - * @param schema An input schema, regardless of the type of schema - * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} - */ - private static ValidationRule.Result checkTypeArray(Schema schema) { - ValidationRule.Result result = ValidationRule.Pass.empty(); - // TODO - return result; - } - /** * JSON Schema uses the 'nullable' attribute. *

@@ -134,9 +123,34 @@ private static ValidationRule.Result checkTypeArray(Schema schema) { * @param schema An input schema, regardless of the type of schema * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} */ - private static ValidationRule.Result checkNullableAttribute(Schema schema) { + private static ValidationRule.Result checkNullableAttribute(SchemaWrapper schemaWrapper) { + Schema schema = schemaWrapper.getSchema(); ValidationRule.Result result = ValidationRule.Pass.empty(); - // TODO + SemVer version = new SemVer(schemaWrapper.getOpenAPI().getOpenapi()); + if (version.atLeast("3.1")) { + if (ModelUtils.isNullable(schema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", + schemaWrapper.getOpenAPI().getOpenapi(), schema.getName())); + return result; + } + if (schema instanceof ComposedSchema) { + final ComposedSchema composed = (ComposedSchema) schema; + List interfaces = ModelUtils.getInterfaces(composed); + if (!interfaces.isEmpty()) { + for (Schema interfaceSchema : interfaces) { + if (ModelUtils.isNullable(interfaceSchema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", + schemaWrapper.getOpenAPI().getOpenapi(), interfaceSchema.getName())); + return result; + } + } + } + } + } return result; } } From 57f4a0e89e8b0a1b8b9c9eb28c4095708cc3d3a2 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 5 Feb 2020 10:38:54 -0800 Subject: [PATCH 09/22] add validation for nullable and null --- .../oas/OpenApiSchemaValidations.java | 86 ++++++++++--------- 1 file changed, 45 insertions(+), 41 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java index 2fdaba29c15b..647a76d61a70 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -86,27 +86,29 @@ private static ValidationRule.Result checkOneOfWithProperties(SchemaWrapper sche private static ValidationRule.Result checkNullType(SchemaWrapper schemaWrapper) { Schema schema = schemaWrapper.getSchema(); ValidationRule.Result result = ValidationRule.Pass.empty(); - SemVer version = new SemVer(schemaWrapper.getOpenAPI().getOpenapi()); - if (version.atLeast("3.0") && version.compareTo(new SemVer("3.1")) < 0) { - // OAS spec is 3.0.x - if (ModelUtils.isNullSchema(schema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, - "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", - schema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); - return result; - } - if (schema instanceof ComposedSchema) { - final ComposedSchema composed = (ComposedSchema) schema; - List interfaces = ModelUtils.getInterfaces(composed); - if (!interfaces.isEmpty()) { - for (Schema interfaceSchema : interfaces) { - if (ModelUtils.isNullSchema(interfaceSchema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, - "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", - interfaceSchema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); - return result; + if (schemaWrapper.getOpenAPI() != null) { + SemVer version = new SemVer(schemaWrapper.getOpenAPI().getOpenapi()); + if (version.atLeast("3.0") && version.compareTo(new SemVer("3.1")) < 0) { + // OAS spec is 3.0.x + if (ModelUtils.isNullSchema(schema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", + schema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); + return result; + } + if (schema instanceof ComposedSchema) { + final ComposedSchema composed = (ComposedSchema) schema; + List interfaces = ModelUtils.getInterfaces(composed); + if (!interfaces.isEmpty()) { + for (Schema interfaceSchema : interfaces) { + if (ModelUtils.isNullSchema(interfaceSchema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", + interfaceSchema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); + return result; + } } } } @@ -126,26 +128,28 @@ private static ValidationRule.Result checkNullType(SchemaWrapper schemaWrapper) private static ValidationRule.Result checkNullableAttribute(SchemaWrapper schemaWrapper) { Schema schema = schemaWrapper.getSchema(); ValidationRule.Result result = ValidationRule.Pass.empty(); - SemVer version = new SemVer(schemaWrapper.getOpenAPI().getOpenapi()); - if (version.atLeast("3.1")) { - if (ModelUtils.isNullable(schema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, - "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", - schemaWrapper.getOpenAPI().getOpenapi(), schema.getName())); - return result; - } - if (schema instanceof ComposedSchema) { - final ComposedSchema composed = (ComposedSchema) schema; - List interfaces = ModelUtils.getInterfaces(composed); - if (!interfaces.isEmpty()) { - for (Schema interfaceSchema : interfaces) { - if (ModelUtils.isNullable(interfaceSchema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, - "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", - schemaWrapper.getOpenAPI().getOpenapi(), interfaceSchema.getName())); - return result; + if (schemaWrapper.getOpenAPI() != null) { + SemVer version = new SemVer(schemaWrapper.getOpenAPI().getOpenapi()); + if (version.atLeast("3.1")) { + if (ModelUtils.isNullable(schema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", + schemaWrapper.getOpenAPI().getOpenapi(), schema.getName())); + return result; + } + if (schema instanceof ComposedSchema) { + final ComposedSchema composed = (ComposedSchema) schema; + List interfaces = ModelUtils.getInterfaces(composed); + if (!interfaces.isEmpty()) { + for (Schema interfaceSchema : interfaces) { + if (ModelUtils.isNullable(interfaceSchema)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, + "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", + schemaWrapper.getOpenAPI().getOpenapi(), interfaceSchema.getName())); + return result; + } } } } From 36b8d629ac042d3c398a8ea44d4dc6f58ec82863 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 5 Feb 2020 10:52:48 -0800 Subject: [PATCH 10/22] revert log statement modification --- .../org/openapitools/codegen/languages/PythonClientCodegen.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java index 9b5155201b32..82a421f2801f 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java @@ -869,7 +869,7 @@ private String toExampleValueRecursive(Schema schema, List included_sche } example +=")"; } else { - LOGGER.warn("Type '" + schema.getType() + "' not handled properly in toExampleValue"); + LOGGER.warn("Type " + schema.getType() + " not handled properly in toExampleValue"); } if (ModelUtils.isStringSchema(schema)) { From 71363d58d1a4f4f95b88e8684afa983df41ffc0a Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 5 Feb 2020 22:03:52 -0800 Subject: [PATCH 11/22] support null type --- .../codegen/CodegenParameter.java | 5 ++ .../openapitools/codegen/DefaultCodegen.java | 7 +- .../PythonClientExperimentalCodegen.java | 12 +++ .../codegen/utils/ModelUtils.java | 83 ++++++++++++++----- 4 files changed, 84 insertions(+), 23 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java index 270f4f2b5e6c..7072df85be8a 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java @@ -19,6 +19,11 @@ import java.util.*; +/** + * Describes a single operation parameter in the OAS specification. + * A unique parameter is defined by a combination of a name and location. + * Parameters may be located in a path, query, header or cookie. + */ public class CodegenParameter implements IJsonSchemaValidationProperties { public boolean isFormParam, isQueryParam, isPathParam, isHeaderParam, isCookieParam, isBodyParam, hasMore, isContainer, 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 4d745b46a0d3..c85ea8fff245 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 @@ -1627,8 +1627,8 @@ protected Schema getSchemaItems(ArraySchema schema) { } /** - * Return the name of the allOf schema - * + * Return the name of the allOf schema. + * * @param names List of names * @param composedSchema composed schema * @return name of the allOf schema @@ -1701,6 +1701,9 @@ private String getSingleSchemaType(Schema schema) { /** * Return the OAI type (e.g. integer, long, etc) corresponding to a schema. * 

$ref
is not taken into account by this method. + * + * If the schema is free-form (i.e. 'type: object' with no properties) or inline + * schema, the returned OAI type is 'object'. * * @param schema * @return type diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java index 33472480b13a..ce4743868a90 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java @@ -823,12 +823,24 @@ public String getSimpleTypeDeclaration(Schema schema) { return oasType; } + /** + * Return a string representation of the Python types for the specified schema. + * Primitive types in the OAS specification are implemented in Python using the corresponding + * Python primitive types. + * Composed types (e.g. allAll, oneOf, anyOf) are represented in Python using list of types. + * + * @param p The OAS schema. + * @param prefix + * @param suffix + * @return + */ public String getTypeString(Schema p, String prefix, String suffix) { // this is used to set dataType, which defines a python tuple of classes String fullSuffix = suffix; if (")".equals(suffix)) { fullSuffix = "," + suffix; } + p = ModelUtils.getReferencedSchema(this.openAPI, p); if (ModelUtils.isNullable(p)) { fullSuffix = ", none_type" + suffix; } 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 6ef5b00f5329..0836c87676ee 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 @@ -397,27 +397,6 @@ public static boolean isStringSchema(Schema schema) { return false; } - /** - * isNullSchema returns true if the specified schema is the 'null' type. - * - * The 'null' type is supported in OAS 3.1 and above. It is not supported - * in OAS 2.0 and OAS 3.0.x. - * - * For example, the "null" type could be used to specify that a value must - * either be null or a specified type: - * - * OptionalOrder: - * oneOf: - * - type: 'null' - * - $ref: '#/components/schemas/Order' - */ - public static boolean isNullSchema(Schema schema) { - if ("null".equals(schema.getType())) { - return true; - } - return false; - } - public static boolean isIntegerSchema(Schema schema) { if (schema instanceof IntegerSchema) { return true; @@ -1111,6 +1090,15 @@ else if (schema instanceof ComposedSchema) { return false; } + /** + * Return true if the 'nullable' attribute is set to true in the schema, i.e. if the value + * of the property can be the null value. + * + * The 'nullable' attribute was introduced in OAS 3.0. + * The 'nullable' attribute is deprecated in OAS 3.1. + * + * @param schema the OAS schema. + */ public static boolean isNullable(Schema schema) { if (schema == null) { return false; @@ -1123,7 +1111,60 @@ public static boolean isNullable(Schema schema) { if (schema.getExtensions() != null && schema.getExtensions().get("x-nullable") != null) { return Boolean.valueOf(schema.getExtensions().get("x-nullable").toString()); } + // In OAS 3.1, the recommended way to define a nullable property or object is to use oneOf. + // In the example below, the 'OptionalOrder' can have the null value because the 'null' + // type is one of the elements under 'oneOf'. + // + // OptionalOrder: + // oneOf: + // - type: 'null' + // - $ref: '#/components/schemas/Order' + // + if (schema instanceof ComposedSchema) { + return isNullableComposedSchema(((ComposedSchema) schema)); + } + return false; + } + /** + * Return true if the specified composed schema is 'oneOf', contains one or two elements, + * and at least one of the elements is the 'null' type. + * + * The 'null' type is supported in OAS 3.1 and above. + * + * @param schema the OAS composed schema. + * @return true if the composed schema is nullable. + */ + public static boolean isNullableComposedSchema(ComposedSchema schema) { + List oneOf = schema.getOneOf(); + if (oneOf != null && oneOf.size() <= 2) { + for (Schema s : oneOf) { + if (isNullSchema(s)) { + return true; + } + } + } + return false; + } + + /** + * isNullSchema returns true if the specified schema is the 'null' type. + * + * The 'null' type is supported in OAS 3.1 and above. It is not supported + * in OAS 2.0 and OAS 3.0.x. + * + * For example, the "null" type could be used to specify that a value must + * either be null or a specified type: + * + * OptionalOrder: + * oneOf: + * - type: 'null' + * - $ref: '#/components/schemas/Order' + */ + public static boolean isNullSchema(Schema schema) { + if ("null".equals(schema.getType())) { + return true; + } return false; } From 0f140cc70fed19ab5285aed4bbd39a62b10f3e82 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 5 Feb 2020 22:27:42 -0800 Subject: [PATCH 12/22] improve code comments --- .../main/java/org/openapitools/codegen/DefaultCodegen.java | 4 +++- .../openapitools/codegen/languages/PythonClientCodegen.java | 2 ++ .../codegen/languages/PythonClientExperimentalCodegen.java | 6 +++--- 3 files changed, 8 insertions(+), 4 deletions(-) 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 c85ea8fff245..2ab5db3933cd 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 @@ -1627,7 +1627,7 @@ protected Schema getSchemaItems(ArraySchema schema) { } /** - * Return the name of the allOf schema. + * Return the name of the 'allOf' composed schema. * * @param names List of names * @param composedSchema composed schema @@ -1712,6 +1712,8 @@ private String getPrimitiveType(Schema schema) { if (schema == null) { throw new RuntimeException("schema cannot be null in getPrimitiveType"); } else if (ModelUtils.isNullSchema(schema)) { + // The 'null' type is allowed in OAS 3.1 and above. It is not supported by OAS 3.0.x, + // though this tooling supports it. return "null"; } else if (ModelUtils.isStringSchema(schema) && "number".equals(schema.getFormat())) { // special handle of type: string, format: number diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java index 82a421f2801f..327bb0d8fd2a 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java @@ -706,6 +706,8 @@ private String toExampleValueRecursive(Schema schema, List included_sche String example = super.toExampleValue(schema); if (ModelUtils.isNullSchema(schema) && null!=example) { + // The 'null' type is allowed in OAS 3.1 and above. It is not supported by OAS 3.0.x, + // though this tooling supports it. return "None"; } // correct "true"s into "True"s, since super.toExampleValue uses "toString()" on Java booleans diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java index ce4743868a90..7c1963b0a532 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java @@ -830,9 +830,9 @@ public String getSimpleTypeDeclaration(Schema schema) { * Composed types (e.g. allAll, oneOf, anyOf) are represented in Python using list of types. * * @param p The OAS schema. - * @param prefix - * @param suffix - * @return + * @param prefix prepended to the returned value. + * @param suffix appended to the returned value. + * @return a string representation of the Python types */ public String getTypeString(Schema p, String prefix, String suffix) { // this is used to set dataType, which defines a python tuple of classes From 8ba134de6710757805514fa0154cbd3f5953469f Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Thu, 6 Feb 2020 07:27:57 -0800 Subject: [PATCH 13/22] improve code comments. the warning used to notify the users to use instead of defining the schema inline, but now the InlineModelResolver has been enhanced --- .../src/main/java/org/openapitools/codegen/DefaultCodegen.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 2ab5db3933cd..cbd5c02cfd36 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 @@ -1641,8 +1641,7 @@ public String toAllOfName(List names, ComposedSchema composedSchema) { } else if (names.size() == 1) { return names.get(0); } else { - LOGGER.warn("allOf with multiple schemas defined. Using only the first one: {}. " + - "To fully utilize allOf, please use $ref instead of inline schema definition", names.get(0)); + LOGGER.warn("allOf with multiple schemas defined. Using only the first one: {}", names.get(0)); return names.get(0); } } From 23c8bccdab102cfdb4ba5b76390ffde2b9702b37 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Thu, 6 Feb 2020 07:45:14 -0800 Subject: [PATCH 14/22] add code comments --- .../codegen/utils/ModelUtils.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) 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 0836c87676ee..141f4a01bf7f 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 @@ -1094,8 +1094,13 @@ else if (schema instanceof ComposedSchema) { * Return true if the 'nullable' attribute is set to true in the schema, i.e. if the value * of the property can be the null value. * + * The caller is responsible for resolving schema references before invoking isNullable. + * If the input schema is a $ref and the referenced schema has 'nullable: true', this method + * returns false (because the nullable attribute is defined in the referenced schema). + * * The 'nullable' attribute was introduced in OAS 3.0. - * The 'nullable' attribute is deprecated in OAS 3.1. + * The 'nullable' attribute is deprecated in OAS 3.1. In a OAS 3.1 document, the preferred way + * to specify nullable properties is to use the 'null' type. * * @param schema the OAS schema. */ @@ -1112,14 +1117,6 @@ public static boolean isNullable(Schema schema) { return Boolean.valueOf(schema.getExtensions().get("x-nullable").toString()); } // In OAS 3.1, the recommended way to define a nullable property or object is to use oneOf. - // In the example below, the 'OptionalOrder' can have the null value because the 'null' - // type is one of the elements under 'oneOf'. - // - // OptionalOrder: - // oneOf: - // - type: 'null' - // - $ref: '#/components/schemas/Order' - // if (schema instanceof ComposedSchema) { return isNullableComposedSchema(((ComposedSchema) schema)); } @@ -1131,6 +1128,13 @@ public static boolean isNullable(Schema schema) { * and at least one of the elements is the 'null' type. * * The 'null' type is supported in OAS 3.1 and above. + * In the example below, the 'OptionalOrder' can have the null value because the 'null' + * type is one of the elements under 'oneOf'. + * + * OptionalOrder: + * oneOf: + * - type: 'null' + * - $ref: '#/components/schemas/Order' * * @param schema the OAS composed schema. * @return true if the composed schema is nullable. From ae76186133ff4023bf23980e9c7eea6c6778d47f Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Thu, 6 Feb 2020 07:46:26 -0800 Subject: [PATCH 15/22] code formatting --- .../org/openapitools/codegen/languages/PythonClientCodegen.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java index 327bb0d8fd2a..ced47190f0c9 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java @@ -705,7 +705,7 @@ private String toExampleValueRecursive(Schema schema, List included_sche for (int i=0 ; i< indentation ; i++) indentation_string += " "; String example = super.toExampleValue(schema); - if (ModelUtils.isNullSchema(schema) && null!=example) { + if (ModelUtils.isNullSchema(schema) && null != example) { // The 'null' type is allowed in OAS 3.1 and above. It is not supported by OAS 3.0.x, // though this tooling supports it. return "None"; From 3e52b6f9fef2a82143188e39283cecc97f2ec679 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Thu, 6 Feb 2020 09:50:50 -0800 Subject: [PATCH 16/22] add code comments, refactor method --- .../org/openapitools/codegen/DefaultCodegen.java | 2 +- .../codegen/languages/PythonClientCodegen.java | 2 +- .../languages/PythonClientExperimentalCodegen.java | 1 + .../org/openapitools/codegen/utils/ModelUtils.java | 12 ++++++++---- .../validations/oas/OpenApiSchemaValidations.java | 4 ++-- 5 files changed, 13 insertions(+), 8 deletions(-) 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 cbd5c02cfd36..d7c27ef0a668 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 @@ -1710,7 +1710,7 @@ private String getSingleSchemaType(Schema schema) { private String getPrimitiveType(Schema schema) { if (schema == null) { throw new RuntimeException("schema cannot be null in getPrimitiveType"); - } else if (ModelUtils.isNullSchema(schema)) { + } else if (ModelUtils.isNullType(schema)) { // The 'null' type is allowed in OAS 3.1 and above. It is not supported by OAS 3.0.x, // though this tooling supports it. return "null"; diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java index ced47190f0c9..99a972c2ede8 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java @@ -705,7 +705,7 @@ private String toExampleValueRecursive(Schema schema, List included_sche for (int i=0 ; i< indentation ; i++) indentation_string += " "; String example = super.toExampleValue(schema); - if (ModelUtils.isNullSchema(schema) && null != example) { + if (ModelUtils.isNullType(schema) && null != example) { // The 'null' type is allowed in OAS 3.1 and above. It is not supported by OAS 3.0.x, // though this tooling supports it. return "None"; diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java index 7c1963b0a532..b27b394a4124 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java @@ -840,6 +840,7 @@ public String getTypeString(Schema p, String prefix, String suffix) { if (")".equals(suffix)) { fullSuffix = "," + suffix; } + // Resolve $ref because ModelUtils.isXYZ methods do not automatically resolve references. p = ModelUtils.getReferencedSchema(this.openAPI, p); if (ModelUtils.isNullable(p)) { fullSuffix = ", none_type" + suffix; 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 141f4a01bf7f..5a2e1698d3a8 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 @@ -996,7 +996,7 @@ public static String getParentName(ComposedSchema composedSchema, Map oneOf = schema.getOneOf(); if (oneOf != null && oneOf.size() <= 2) { for (Schema s : oneOf) { - if (isNullSchema(s)) { + if (isNullType(s)) { return true; } } @@ -1152,7 +1156,7 @@ public static boolean isNullableComposedSchema(ComposedSchema schema) { } /** - * isNullSchema returns true if the specified schema is the 'null' type. + * isNullType returns true if the input schema is the 'null' type. * * The 'null' type is supported in OAS 3.1 and above. It is not supported * in OAS 2.0 and OAS 3.0.x. @@ -1165,7 +1169,7 @@ public static boolean isNullableComposedSchema(ComposedSchema schema) { * - type: 'null' * - $ref: '#/components/schemas/Order' */ - public static boolean isNullSchema(Schema schema) { + public static boolean isNullType(Schema schema) { if ("null".equals(schema.getType())) { return true; } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java index 647a76d61a70..ec8117ac596f 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -90,7 +90,7 @@ private static ValidationRule.Result checkNullType(SchemaWrapper schemaWrapper) SemVer version = new SemVer(schemaWrapper.getOpenAPI().getOpenapi()); if (version.atLeast("3.0") && version.compareTo(new SemVer("3.1")) < 0) { // OAS spec is 3.0.x - if (ModelUtils.isNullSchema(schema)) { + if (ModelUtils.isNullType(schema)) { result = new ValidationRule.Fail(); result.setDetails(String.format(Locale.ROOT, "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", @@ -102,7 +102,7 @@ private static ValidationRule.Result checkNullType(SchemaWrapper schemaWrapper) List interfaces = ModelUtils.getInterfaces(composed); if (!interfaces.isEmpty()) { for (Schema interfaceSchema : interfaces) { - if (ModelUtils.isNullSchema(interfaceSchema)) { + if (ModelUtils.isNullType(interfaceSchema)) { result = new ValidationRule.Fail(); result.setDetails(String.format(Locale.ROOT, "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", From 3b61fb94e123b5530f000434e8f1c5c484f78b60 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Fri, 7 Feb 2020 08:01:16 -0800 Subject: [PATCH 17/22] fix call to isNullable to handle and 'null' type --- .../codegen/languages/PythonClientExperimentalCodegen.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java index b27b394a4124..36efb1f3fe46 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java @@ -841,8 +841,7 @@ public String getTypeString(Schema p, String prefix, String suffix) { fullSuffix = "," + suffix; } // Resolve $ref because ModelUtils.isXYZ methods do not automatically resolve references. - p = ModelUtils.getReferencedSchema(this.openAPI, p); - if (ModelUtils.isNullable(p)) { + if (ModelUtils.isNullable(ModelUtils.getReferencedSchema(this.openAPI, p))) { fullSuffix = ", none_type" + suffix; } if (ModelUtils.isFreeFormObject(p) && ModelUtils.getAdditionalProperties(p) == null) { From 68ad45de926ada493a4d4d1077bb191cbd99c6c5 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Sun, 9 Feb 2020 08:58:26 -0800 Subject: [PATCH 18/22] add OAS document for test purpose --- .../src/test/resources/3_1/null-types.yaml | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 modules/openapi-generator/src/test/resources/3_1/null-types.yaml diff --git a/modules/openapi-generator/src/test/resources/3_1/null-types.yaml b/modules/openapi-generator/src/test/resources/3_1/null-types.yaml new file mode 100644 index 000000000000..f1c143cd487b --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_1/null-types.yaml @@ -0,0 +1,146 @@ +openapi: 3.1.0 +info: + version: 1.0.0 + title: Example + license: + name: MIT +servers: + - url: http://api.example.xyz/v1 +paths: + /person/display/{personId}: + get: + parameters: + - name: personId + in: path + required: true + description: The id of the person to retrieve + schema: + type: string + operationId: list + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: "#/components/schemas/Person" +components: + schemas: + # The purpose of this OAS document is to test the OpenAPITool validator and generator against + # valid OAS syntax. It is not written to achieve good model design. + Person: + type: object + discriminator: + propertyName: $_type + mapping: + a: '#/components/schemas/Adult' + c: Child + properties: + $_type: + type: string + lastName: + type: string + firstName: + type: string + maidenName: + # A property which can be the 'null' value or a string value. + # This allows for a concise expression of use cases such as a function that might + # return either a string of a certain length or a null value: + # The 'maxLength' assertion only constrain values within a certain primitive + # type. When the type of the instance is not of the type targeted by + # the keyword, the instance is considered to conform to the assertion. + # See https://tools.ietf.org/html/draft-handrews-json-schema-02#section-7.6.1 + type: [ string, 'null'] + maxLength: 255 + address: + # An object defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + type: object + properties: + street: + type: string + city: + type: string + secondHomeAddress: + # The value of this property may be the null value because the 'Nullable.Address' + # schema has set the 'nullable' attribute to true. + $ref: "#/components/schemas/Nullable.Address" + skills: + # An array defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + type: array + items: + type: object + properties: + name: + type: string + nullableSkills: + # An array defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + # The value can be 'null'. + type: [array, 'null'] + items: + type: object + properties: + name: + type: string + nullableSkillsVariant2: + # An array defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + # The value can be 'null'. + oneOf: + - type: 'null' + - type: array + items: + type: object + properties: + name: + type: string + Adult: + description: A representation of an adult + allOf: + - $ref: '#/components/schemas/Person' + - type: object + properties: + children: + # The value of this property may not be null. + # Some language generators are lenient by default (either intentionally or by accident) + # and would allow the 'null' value, but from a compliance perspective, the null value + # is not allowed. + type: array + items: + $ref: "#/components/schemas/Child" + nullableChildren: + # Use 'type array' syntax to indicate the value of the property may be the 'null' value or an array. + # Type arrays are supported in OAS 3.1 and above. + type: [array, 'null'] + items: + $ref: "#/components/schemas/Child" + nullableChildrenVariant2: + # Use 'oneOf' syntax to indicate the value of the property may be the 'null' value or an array. + # The 'null' type is supported in OAS 3.1 and above. + oneOf: + - type: 'null' + - type: array + items: + $ref: "#/components/schemas/Child" + Child: + description: A representation of a child + allOf: + - type: object + properties: + age: + type: integer + format: int32 + - $ref: '#/components/schemas/Person' + Nullable.Address: + description: A representation of an address whose value can be null. + type: object + # Note the 'nullable' attribute has been deprecated in OAS 3.1, hence it should be + # avoided in favor of using the 'null' type. + nullable: true + properties: + street: + type: string + city: + type: string From 4d6e09d6a021361eeefb567ea4cd3e5a188f0278 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Sun, 9 Feb 2020 21:13:29 -0800 Subject: [PATCH 19/22] add unit tests for null type --- .../codegen/InlineModelResolver.java | 1 + .../oas/OpenApiSchemaTypeTest.java | 43 +++++ .../3_1/null-types-with-type-array.yaml | 149 ++++++++++++++++++ .../src/test/resources/3_1/null-types.yaml | 31 +--- 4 files changed, 199 insertions(+), 25 deletions(-) create mode 100644 modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java create mode 100644 modules/openapi-generator/src/test/resources/3_1/null-types-with-type-array.yaml 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 e5e519d32c61..a388fa899eaf 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 @@ -157,6 +157,7 @@ private void flattenRequestBody(OpenAPI openAPI, String pathname, Operation oper ObjectSchema op = (ObjectSchema) inner; if (op.getProperties() != null && op.getProperties().size() > 0) { flattenProperties(op.getProperties(), pathname); + // Generate a unique model name based on the title. String modelName = resolveModelName(op.getTitle(), null); Schema innerModel = modelFromProperty(op, modelName); String existing = matchGenerated(innerModel); diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java new file mode 100644 index 000000000000..fa01cc88dcde --- /dev/null +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java @@ -0,0 +1,43 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.media.*; +import io.swagger.v3.oas.models.OpenAPI; +import org.openapitools.codegen.TestUtils; +import org.openapitools.codegen.validation.Invalid; +import org.openapitools.codegen.validation.ValidationResult; +import org.testng.Assert; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +public class OpenApiSchemaTypeTest { + @Test(dataProvider = "oas31RecommendationExpectations", description = "Warn when a 'null' type is present in OAS 3.0 document") + public void testOas30DocumentWithNullType(final OpenAPI openAPI) { + /* + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(true); + OpenApiEvaluator validator = new OpenApiEvaluator(config); + ValidationResult result = validator.validate(openAPI); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> "Schema defines properties alongside oneOf." .equals(invalid.getRule().getDescription())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + */ + } + + @Test(dataProvider = "oas31RecommendationExpectations", description = "Don't warn when a 'null' type is present in OAS 3.0 document") + public void testOas31DocumentWithNullType(Schema schema, boolean matches) { + } + + @DataProvider(name = "oas31RecommendationExpectations") + public Object[][] oas31RecommendationExpectations() { + return new Object[][]{ + //{TestUtils.parseSpec("src/test/resources/3_1/null-types.yaml")}, + }; + } +} \ No newline at end of file diff --git a/modules/openapi-generator/src/test/resources/3_1/null-types-with-type-array.yaml b/modules/openapi-generator/src/test/resources/3_1/null-types-with-type-array.yaml new file mode 100644 index 000000000000..b01e611e37ad --- /dev/null +++ b/modules/openapi-generator/src/test/resources/3_1/null-types-with-type-array.yaml @@ -0,0 +1,149 @@ +# OAS document that uses 3.1 features: +# 'null' type +# type array +openapi: 3.1.0 +info: + version: 1.0.0 + title: Example + license: + name: MIT +servers: + - url: http://api.example.xyz/v1 +paths: + /person/display/{personId}: + get: + parameters: + - name: personId + in: path + required: true + description: The id of the person to retrieve + schema: + type: string + operationId: list + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: "#/components/schemas/Person" +components: + schemas: + # The purpose of this OAS document is to test the OpenAPITool validator and generator against + # valid OAS syntax. It is not written to achieve good model design. + Person: + type: object + discriminator: + propertyName: $_type + mapping: + a: '#/components/schemas/Adult' + c: Child + properties: + $_type: + type: string + lastName: + type: string + firstName: + type: string + maidenName: + # A property which can be the 'null' value or a string value. + # This allows for a concise expression of use cases such as a function that might + # return either a string of a certain length or a null value: + # The 'maxLength' assertion only constrain values within a certain primitive + # type. When the type of the instance is not of the type targeted by + # the keyword, the instance is considered to conform to the assertion. + # See https://tools.ietf.org/html/draft-handrews-json-schema-02#section-7.6.1 + type: [ string, 'null'] + maxLength: 255 + address: + # An object defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + type: object + properties: + street: + type: string + city: + type: string + secondHomeAddress: + # The value of this property may be the null value because the 'Nullable.Address' + # schema has set the 'nullable' attribute to true. + $ref: "#/components/schemas/Nullable.Address" + skills: + # An array defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + type: array + items: + type: object + properties: + name: + type: string + nullableSkills: + # An array defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + # The value can be 'null'. + type: [array, 'null'] + items: + type: object + properties: + name: + type: string + nullableSkillsVariant2: + # An array defined inline instead of by reference. codegen and language generators + # behave differently when a property is defined inline versus by reference. + # The value can be 'null'. + oneOf: + - type: 'null' + - type: array + items: + type: object + properties: + name: + type: string + Adult: + description: A representation of an adult + allOf: + - $ref: '#/components/schemas/Person' + - type: object + properties: + children: + # The value of this property may not be null. + # Some language generators are lenient by default (either intentionally or by accident) + # and would allow the 'null' value, but from a compliance perspective, the null value + # is not allowed. + type: array + items: + $ref: "#/components/schemas/Child" + nullableChildren: + # Use 'type array' syntax to indicate the value of the property may be the 'null' value or an array. + # Type arrays are supported in OAS 3.1 and above. + type: [array, 'null'] + items: + $ref: "#/components/schemas/Child" + nullableChildrenVariant2: + # Use 'oneOf' syntax to indicate the value of the property may be the 'null' value or an array. + # The 'null' type is supported in OAS 3.1 and above. + oneOf: + - type: 'null' + - type: array + items: + $ref: "#/components/schemas/Child" + Child: + description: A representation of a child + allOf: + - type: object + properties: + age: + type: integer + format: int32 + - $ref: '#/components/schemas/Person' + Nullable.Address: + description: A representation of an address whose value can be null. + type: object + # Note the 'nullable' attribute has been deprecated in OAS 3.1, hence it should be + # avoided in favor of using the 'null' type. + nullable: true + properties: + street: + type: string + city: + type: string diff --git a/modules/openapi-generator/src/test/resources/3_1/null-types.yaml b/modules/openapi-generator/src/test/resources/3_1/null-types.yaml index f1c143cd487b..1d7df45ab540 100644 --- a/modules/openapi-generator/src/test/resources/3_1/null-types.yaml +++ b/modules/openapi-generator/src/test/resources/3_1/null-types.yaml @@ -1,4 +1,5 @@ -openapi: 3.1.0 +# OAS document that uses the 'null' type, which is a 3.1 feature. +openapi: 3.0.2 # This should be 3.1, but the parser does not accept that version yet. info: version: 1.0.0 title: Example @@ -44,14 +45,10 @@ components: type: string maidenName: # A property which can be the 'null' value or a string value. - # This allows for a concise expression of use cases such as a function that might - # return either a string of a certain length or a null value: - # The 'maxLength' assertion only constrain values within a certain primitive - # type. When the type of the instance is not of the type targeted by - # the keyword, the instance is considered to conform to the assertion. - # See https://tools.ietf.org/html/draft-handrews-json-schema-02#section-7.6.1 - type: [ string, 'null'] - maxLength: 255 + oneOf: + - type: string + maxLength: 255 + - type: 'null' address: # An object defined inline instead of by reference. codegen and language generators # behave differently when a property is defined inline versus by reference. @@ -74,16 +71,6 @@ components: properties: name: type: string - nullableSkills: - # An array defined inline instead of by reference. codegen and language generators - # behave differently when a property is defined inline versus by reference. - # The value can be 'null'. - type: [array, 'null'] - items: - type: object - properties: - name: - type: string nullableSkillsVariant2: # An array defined inline instead of by reference. codegen and language generators # behave differently when a property is defined inline versus by reference. @@ -110,12 +97,6 @@ components: type: array items: $ref: "#/components/schemas/Child" - nullableChildren: - # Use 'type array' syntax to indicate the value of the property may be the 'null' value or an array. - # Type arrays are supported in OAS 3.1 and above. - type: [array, 'null'] - items: - $ref: "#/components/schemas/Child" nullableChildrenVariant2: # Use 'oneOf' syntax to indicate the value of the property may be the 'null' value or an array. # The 'null' type is supported in OAS 3.1 and above. From 364d3552aa8b7dab6f9f497f902b1a422db643c7 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Tue, 11 Feb 2020 08:27:27 -0800 Subject: [PATCH 20/22] add 'null' type validation --- .../codegen/utils/ModelUtils.java | 40 ++++++++++++++ .../validations/oas/OpenApiEvaluator.java | 6 ++- .../oas/OpenApiSchemaValidations.java | 54 ++++++------------- .../oas/OpenApiSchemaTypeTest.java | 20 +++---- 4 files changed, 71 insertions(+), 49 deletions(-) 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 5a2e1698d3a8..1172b0f57a78 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 @@ -264,6 +264,18 @@ private static void visitContent(OpenAPI openAPI, Content content, OpenAPISchema } } + /** + * Invoke the specified visitor function for every schema that matches mimeType in the OpenAPI document. + * + * To avoid infinite recursion, referenced schemas are visited only once. When a referenced schema is visited, + * it is added to visitedSchemas. + * + * @param openAPI the OpenAPI document that contains schema objects. + * @param schema the root schema object to be visited. + * @param mimeType the mime type. TODO: does not seem to be used in a meaningful way. + * @param visitedSchemas the list of referenced schemas that have been visited. + * @param visitor the visitor function which is invoked for every visited schema. + */ private static void visitSchema(OpenAPI openAPI, Schema schema, String mimeType, List visitedSchemas, OpenAPISchemaVisitor visitor) { visitor.visit(schema, mimeType); if (schema.get$ref() != null) { @@ -648,6 +660,14 @@ public static Schema getSchema(OpenAPI openAPI, String name) { return getSchemas(openAPI).get(name); } + /** + * Return a Map of the schemas defined under /components/schemas in the OAS document. + * The returned Map only includes the direct children of /components/schemas in the OAS document; the Map + * does not include inlined schemas. + * + * @param openAPI the OpenAPI document. + * @return a map of schemas in the OAS document. + */ public static Map getSchemas(OpenAPI openAPI) { if (openAPI != null && openAPI.getComponents() != null && openAPI.getComponents().getSchemas() != null) { return openAPI.getComponents().getSchemas(); @@ -655,6 +675,26 @@ public static Map getSchemas(OpenAPI openAPI) { return Collections.emptyMap(); } + /** + * Return the list of all schemas in the 'components/schemas' section of an openAPI specification, + * including inlined schemas and children of composed schemas. + * + * @param openAPI OpenAPI document + * @return a list of schemas + */ + public static List getAllSchemas(OpenAPI openAPI) { + List allSchemas = new ArrayList(); + List refSchemas = new ArrayList(); + getSchemas(openAPI).forEach((key, schema) -> { + // Invoke visitSchema to recursively visit all schema objects, included inlined and composed schemas. + // Use the OpenAPISchemaVisitor visitor function + visitSchema(openAPI, schema, null, refSchemas, (s, mimetype) -> { + allSchemas.add(s); + }); + }); + return allSchemas; + } + /** * If a RequestBody contains a reference to an other RequestBody with '$ref', returns the referenced RequestBody if it is found or the actual RequestBody in the other cases. * diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java index 29e4e9dcf730..5cbe4c9eb894 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java @@ -49,8 +49,10 @@ public ValidationResult validate(OpenAPI specification) { ModelUtils.getUnusedSchemas(specification).forEach(schemaName -> validationResult.addResult(Validated.invalid(unusedSchema, "Unused model: " + schemaName))); } - Map schemas = ModelUtils.getSchemas(specification); - schemas.forEach((key, schema) -> { + // Get list of all schemas under /components/schemas, including nested schemas defined inline and composed schema. + // The validators must be able to validate every schema defined in the OAS document. + List schemas = ModelUtils.getAllSchemas(specification); + schemas.forEach(schema -> { SchemaWrapper wrapper = new SchemaWrapper(specification, schema); validationResult.consume(schemaValidations.validate(wrapper)); }); diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java index ec8117ac596f..4347f9ef4384 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -28,7 +28,7 @@ class OpenApiSchemaValidations extends GenericValidator { } if (ruleConfiguration.isEnableSchemaTypeRecommendation()) { rules.add(ValidationRule.warn( - "Schema defines uses the 'null' type.", + "Schema uses the 'null' type but OAS document is version 3.0.", "The 'null' type is not supported in OpenAPI 3.0.x. It is supported in OpenAPI 3.1 and above. While our tooling supports this, it may cause issues with other tools.", OpenApiSchemaValidations::checkNullType )); @@ -36,7 +36,7 @@ class OpenApiSchemaValidations extends GenericValidator { if (ruleConfiguration.isEnableNullableAttributeRecommendation()) { rules.add(ValidationRule.warn( "Schema uses the 'nullable' attribute.", - "The 'nullable' attribute is deprecated in OpenAPI 3.1, and may no longer be supported in future releases. Consider migrating to 'null' type.", + "The 'nullable' attribute is deprecated in OpenAPI 3.1, and may no longer be supported in future releases. Consider migrating to the 'null' type.", OpenApiSchemaValidations::checkNullableAttribute )); } @@ -79,8 +79,10 @@ private static ValidationRule.Result checkOneOfWithProperties(SchemaWrapper sche * JSON Schema specifies type: 'null'. *

* 'null' type is supported in OpenAPI Specification 3.1 and above. It is not supported in OpenAPI 3.0.x. + * Note: the validator invokes checkNullType() for every top-level schema in the OAS document. The method + * is not called for nested schemas that are defined inline. * - * @param schema An input schema, regardless of the type of schema + * @param schema An input schema, regardless of the type of schema. * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} */ private static ValidationRule.Result checkNullType(SchemaWrapper schemaWrapper) { @@ -92,26 +94,15 @@ private static ValidationRule.Result checkNullType(SchemaWrapper schemaWrapper) // OAS spec is 3.0.x if (ModelUtils.isNullType(schema)) { result = new ValidationRule.Fail(); + String name = schema.getName(); + if (name == null) { + name = schema.getTitle(); + } result.setDetails(String.format(Locale.ROOT, - "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", - schema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); + "Schema '%s' uses a 'null' type, which is specified in OAS 3.1 and above, but OAS document is version %s", + name, schemaWrapper.getOpenAPI().getOpenapi())); return result; } - if (schema instanceof ComposedSchema) { - final ComposedSchema composed = (ComposedSchema) schema; - List interfaces = ModelUtils.getInterfaces(composed); - if (!interfaces.isEmpty()) { - for (Schema interfaceSchema : interfaces) { - if (ModelUtils.isNullType(interfaceSchema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, - "%s uses a 'null' type, which is specified in OAS 3.1 and above, but OAS version is %s", - interfaceSchema.getName(), schemaWrapper.getOpenAPI().getOpenapi())); - return result; - } - } - } - } } } return result; @@ -133,26 +124,15 @@ private static ValidationRule.Result checkNullableAttribute(SchemaWrapper schema if (version.atLeast("3.1")) { if (ModelUtils.isNullable(schema)) { result = new ValidationRule.Fail(); + String name = schema.getName(); + if (name == null) { + name = schema.getTitle(); + } result.setDetails(String.format(Locale.ROOT, - "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", - schemaWrapper.getOpenAPI().getOpenapi(), schema.getName())); + "OAS document is version '%s'. Schema '%s' uses 'nullable' attribute, which has been deprecated in OAS 3.1.", + schemaWrapper.getOpenAPI().getOpenapi(), name)); return result; } - if (schema instanceof ComposedSchema) { - final ComposedSchema composed = (ComposedSchema) schema; - List interfaces = ModelUtils.getInterfaces(composed); - if (!interfaces.isEmpty()) { - for (Schema interfaceSchema : interfaces) { - if (ModelUtils.isNullable(interfaceSchema)) { - result = new ValidationRule.Fail(); - result.setDetails(String.format(Locale.ROOT, - "OAS specification is version '%s'. %s is 'nullable', which has been deprecated.", - schemaWrapper.getOpenAPI().getOpenapi(), interfaceSchema.getName())); - return result; - } - } - } - } } } return result; diff --git a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java index fa01cc88dcde..ac8e207fb62e 100644 --- a/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaTypeTest.java @@ -8,14 +8,14 @@ import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; + import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; public class OpenApiSchemaTypeTest { - @Test(dataProvider = "oas31RecommendationExpectations", description = "Warn when a 'null' type is present in OAS 3.0 document") - public void testOas30DocumentWithNullType(final OpenAPI openAPI) { - /* + @Test(dataProvider = "oas31RecommendationExpectations", description = "Warn when 3.1 features are present in a OAS 3.0 document") + public void testOas30DocumentWithNullType(final OpenAPI openAPI, boolean matches) { RuleConfiguration config = new RuleConfiguration(); config.setEnableRecommendations(true); OpenApiEvaluator validator = new OpenApiEvaluator(config); @@ -23,21 +23,21 @@ public void testOas30DocumentWithNullType(final OpenAPI openAPI) { Assert.assertNotNull(result.getWarnings()); List warnings = result.getWarnings().stream() - .filter(invalid -> "Schema defines properties alongside oneOf." .equals(invalid.getRule().getDescription())) + .filter(invalid -> "Schema uses the 'null' type but OAS document is version 3.0." .equals(invalid.getRule().getDescription())) .collect(Collectors.toList()); Assert.assertNotNull(warnings); - */ - } - - @Test(dataProvider = "oas31RecommendationExpectations", description = "Don't warn when a 'null' type is present in OAS 3.0 document") - public void testOas31DocumentWithNullType(Schema schema, boolean matches) { + if (matches) { + Assert.assertEquals(warnings.size() >= 1, true, "Expected to match recommendation."); + } else { + Assert.assertEquals(warnings.size(), 0, "Expected not to match recommendation."); + } } @DataProvider(name = "oas31RecommendationExpectations") public Object[][] oas31RecommendationExpectations() { return new Object[][]{ - //{TestUtils.parseSpec("src/test/resources/3_1/null-types.yaml")}, + {TestUtils.parseSpec("src/test/resources/3_1/null-types.yaml"), true} }; } } \ No newline at end of file From e825bbce86fefc962995549c4ccf7941aedccb3b Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Tue, 11 Feb 2020 09:47:55 -0800 Subject: [PATCH 21/22] fix javadoc issues --- .../main/java/org/openapitools/codegen/utils/ModelUtils.java | 4 ++++ 1 file changed, 4 insertions(+) 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 865b32dbdd2c..27aa602df085 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 @@ -1153,6 +1153,7 @@ else if (schema instanceof ComposedSchema) { * to specify nullable properties is to use the 'null' type. * * @param schema the OAS schema. + * @return true if the schema is nullable. */ public static boolean isNullable(Schema schema) { if (schema == null) { @@ -1214,6 +1215,9 @@ public static boolean isNullableComposedSchema(ComposedSchema schema) { * oneOf: * - type: 'null' * - $ref: '#/components/schemas/Order' + * + * @param schema the OpenAPI schema + * @return true if the schema is the 'null' type */ public static boolean isNullType(Schema schema) { if ("null".equals(schema.getType())) { From 023cd47d9ad7e0b460c34be46e2a5990264a80c9 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 12 Feb 2020 08:10:25 -0800 Subject: [PATCH 22/22] Add validation rule for the supported values of the 'type' attribute --- .../oas/OpenApiSchemaValidations.java | 41 ++++++++++++++++++- .../validations/oas/RuleConfiguration.java | 25 +++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java index 4347f9ef4384..9251349391f3 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -11,6 +11,9 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.Set; +import java.util.HashSet; +import java.util.Arrays; /** * A standalone instance for evaluating rules and recommendations related to OAS {@link Schema} @@ -40,6 +43,13 @@ class OpenApiSchemaValidations extends GenericValidator { OpenApiSchemaValidations::checkNullableAttribute )); } + if (ruleConfiguration.isEnableInvalidTypeRecommendation()) { + rules.add(ValidationRule.warn( + "Schema uses an invalid value for the 'type' attribute.", + "The 'type' attribute must be one of 'null', 'boolean', 'object', 'array', 'number', 'string', or 'integer'.", + OpenApiSchemaValidations::checkInvalidType + )); + } } } @@ -51,7 +61,7 @@ class OpenApiSchemaValidations extends GenericValidator { *

* Where the only examples of oneOf in OpenAPI Specification are used to define either/or type structures rather than validations. * Because of this ambiguity in the spec about what is non-standard about oneOf support, we'll warn as a recommendation that - * properties on the schema defining oneOf relationships may not be intentional in the OpenAPI Specification. +ne * properties on the schema defining oneOf relationships may not be intentional in the OpenAPI Specification. * * @param schemaWrapper An input schema, regardless of the type of schema * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} @@ -137,4 +147,33 @@ private static ValidationRule.Result checkNullableAttribute(SchemaWrapper schema } return result; } + + // The set of valid OAS values for the 'type' attribute. + private static Set validTypes = new HashSet( + Arrays.asList("null", "boolean", "object", "array", "number", "string", "integer")); + + /** + * Validate the OAS document uses supported values for the 'type' attribute. + *

+ * The type must be one of the following values: null, boolean, object, array, number, string, integer. + * + * @param schema An input schema, regardless of the type of schema + * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} + */ + private static ValidationRule.Result checkInvalidType(SchemaWrapper schemaWrapper) { + Schema schema = schemaWrapper.getSchema(); + ValidationRule.Result result = ValidationRule.Pass.empty(); + if (schema.getType() != null && !validTypes.contains(schema.getType())) { + result = new ValidationRule.Fail(); + String name = schema.getName(); + if (name == null) { + name = schema.getTitle(); + } + result.setDetails(String.format(Locale.ROOT, + "Schema '%s' uses the '%s' type, which is not a valid type.", + name, schema.getType())); + return result; + } + return result; + } } diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java index 840bd0041a7c..0ad2213ded7d 100644 --- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java @@ -12,6 +12,7 @@ public class RuleConfiguration { private boolean enableUnusedSchemasRecommendation = defaultedBoolean(propertyPrefix + ".unused-schemas", true); private boolean enableSchemaTypeRecommendation = defaultedBoolean(propertyPrefix + ".schema-type", true); private boolean enableNullableAttributeRecommendation = defaultedBoolean(propertyPrefix + ".nullable-deprecated", true); + private boolean enableInvalidTypeRecommendation = defaultedBoolean(propertyPrefix + ".invalid-type", true); private boolean enableApiRequestUriWithBodyRecommendation = defaultedBoolean(propertyPrefix + ".anti-patterns.uri-unexpected-body", true); @@ -146,8 +147,32 @@ public boolean isEnableNullableAttributeRecommendation() { return enableNullableAttributeRecommendation; } + /** + * Enable or Disable the recommendation check for the 'type' attribute. + * + *

+ * For more details, see {@link RuleConfiguration#isEnableInvalidTypeRecommendation()} + * + * @param enableInvalidTypeRecommendation true to enable, false to disable + */ + public void setEnableInvalidTypeRecommendation(boolean enableInvalidTypeRecommendation) { + this.enableInvalidTypeRecommendation = enableInvalidTypeRecommendation; + } + + /** + * Gets whether the recommendation check for for schemas containing invalid values for the 'type' attribute. + *

+ * + * @return true if enabled, false if disabled + */ + public boolean isEnableInvalidTypeRecommendation() { + return enableInvalidTypeRecommendation; + } + /** * Get whether recommendations are enabled. + *

+ * The 'type' attribute must be one of 'null', 'boolean', 'object', 'array', 'number', 'string', or 'integer' * * @return true if enabled, false if disabled */