From 20590358b70f8501abcffe50a799d965ec724aa8 Mon Sep 17 00:00:00 2001 From: "Sebastien Rosset (serosset)" Date: Wed, 5 Feb 2020 17:56:28 +0000 Subject: [PATCH] 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()