diff --git a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java index 153475f5032..a790464c781 100644 --- a/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java +++ b/modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java @@ -22,14 +22,14 @@ import io.airlift.airline.Option; import io.swagger.parser.OpenAPIParser; import io.swagger.v3.oas.models.OpenAPI; -import io.swagger.v3.oas.models.media.ComposedSchema; -import io.swagger.v3.oas.models.media.Schema; import io.swagger.v3.parser.core.models.SwaggerParseResult; -import org.openapitools.codegen.utils.ModelUtils; +import org.apache.commons.lang3.text.WordUtils; +import org.openapitools.codegen.validation.ValidationResult; +import org.openapitools.codegen.validations.oas.OpenApiEvaluator; +import org.openapitools.codegen.validations.oas.RuleConfiguration; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; @Command(name = "validate", description = "Validate specification") @@ -54,42 +54,28 @@ public class Validate implements Runnable { StringBuilder sb = new StringBuilder(); OpenAPI specification = result.getOpenAPI(); - if (Boolean.TRUE.equals(recommend)) { - if (specification != null) { - // Add information about unused models to the warnings set. - List unusedModels = ModelUtils.getUnusedSchemas(specification); - if (unusedModels != null) { - unusedModels.forEach(name -> warnings.add("Unused model: " + name)); - } + RuleConfiguration ruleConfiguration = new RuleConfiguration(); + ruleConfiguration.setEnableRecommendations(recommend != null ? recommend : false); - // check for loosely defined oneOf extension requirements. - // This is a recommendation because the 3.0.x spec is not clear enough on usage of oneOf. - // see https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.9.2.1.3 and the OAS section on 'Composition and Inheritance'. - Map schemas = ModelUtils.getSchemas(specification); - schemas.forEach((key, schema) -> { - if (schema instanceof ComposedSchema) { - final ComposedSchema composed = (ComposedSchema) schema; - if (composed.getOneOf() != null && composed.getOneOf().size() > 0) { - if (composed.getProperties() != null && composed.getProperties().size() >= 1 && composed.getProperties().get("discriminator") == null) { - warnings.add("Schema (oneOf) should not contain properties: " + key); - } - } - } - }); - } - } + OpenApiEvaluator evaluator = new OpenApiEvaluator(ruleConfiguration); + ValidationResult validationResult = evaluator.validate(specification); + + // TODO: We could also provide description here along with getMessage. getMessage is either a "generic" message or specific (e.g. Model 'Cat' has issues). + // This would require that we parse the messageList coming from swagger-parser into a better structure. + validationResult.getWarnings().forEach(invalid -> warnings.add(invalid.getMessage())); + validationResult.getErrors().forEach(invalid -> errors.add(invalid.getMessage())); if (!errors.isEmpty()) { sb.append("Errors:").append(System.lineSeparator()); errors.forEach(msg -> - sb.append("\t-").append(msg).append(System.lineSeparator()) + sb.append("\t- ").append(WordUtils.wrap(msg, 90).replace(System.lineSeparator(), System.lineSeparator() + "\t ")).append(System.lineSeparator()) ); } if (!warnings.isEmpty()) { sb.append("Warnings: ").append(System.lineSeparator()); warnings.forEach(msg -> - sb.append("\t-").append(msg).append(System.lineSeparator()) + sb.append("\t- ").append(WordUtils.wrap(msg, 90).replace(System.lineSeparator(), System.lineSeparator() + "\t ")).append(System.lineSeparator()) ); } diff --git a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/GenericValidator.java b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/GenericValidator.java index c095bc5abd5..00e4e00e463 100644 --- a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/GenericValidator.java +++ b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/GenericValidator.java @@ -23,9 +23,8 @@ import java.util.List; * * @param The type of object being evaluated. */ -@SuppressWarnings({"WeakerAccess"}) public class GenericValidator implements Validator { - private List rules; + protected List rules; /** * Constructs a new instance of {@link GenericValidator}. @@ -48,11 +47,11 @@ public class GenericValidator implements Validator { ValidationResult result = new ValidationResult(); if (rules != null) { rules.forEach(it -> { - boolean passes = it.evaluate(input); - if (passes) { + ValidationRule.Result attempt = it.evaluate(input); + if (attempt.passed()) { result.addResult(Validated.valid(it)); } else { - result.addResult(Validated.invalid(it, it.getFailureMessage())); + result.addResult(Validated.invalid(it, it.getFailureMessage(), attempt.getDetails())); } }); } diff --git a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Invalid.java b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Invalid.java index ad59a628536..131e7a73465 100644 --- a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Invalid.java +++ b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Invalid.java @@ -23,6 +23,7 @@ package org.openapitools.codegen.validation; public final class Invalid extends Validated { private String message; private ValidationRule rule; + private String details; /** * Constructs a new {@link Invalid} instance. @@ -35,13 +36,29 @@ public final class Invalid extends Validated { this.message = message; } + /** + * Constructs a new {@link Invalid} instance. + * + * @param rule The rule which was evaluated and resulted in this state. + * @param message The message to be displayed for this invalid state. + * @param details Additional contextual details related to the invalid state. + */ + public Invalid(ValidationRule rule, String message, String details) { + this(rule, message); + this.details = details; + } + + public String getDetails() { + return details; + } + @Override - String getMessage() { + public String getMessage() { return message; } @Override - ValidationRule getRule() { + public ValidationRule getRule() { return rule; } diff --git a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Validated.java b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Validated.java index 2df3b9c3ae3..05f9df833e4 100644 --- a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Validated.java +++ b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Validated.java @@ -54,6 +54,18 @@ public abstract class Validated { public static Validated invalid(ValidationRule rule, String message) { return new Invalid(rule, message); } + /** + * Creates an instance of an {@link Invalid} validation state. + * + * @param rule The rule which was evaluated. + * @param message The message to display to a user. + * @param details Additional contextual details related to the invalid state. + * + * @return A {@link Validated} instance representing an invalid state according to the rule. + */ + public static Validated invalid(ValidationRule rule, String message, String details) { + return new Invalid(rule, message, details); + } /** * Creates an instance of an {@link Valid} validation state. diff --git a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationResult.java b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationResult.java index 374c3544544..628a11a225c 100644 --- a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationResult.java +++ b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationResult.java @@ -24,7 +24,6 @@ import java.util.stream.Collectors; /** * Encapsulates details about the result of a validation test. */ -@SuppressWarnings("WeakerAccess") public final class ValidationResult { private final List validations; @@ -96,9 +95,16 @@ public final class ValidationResult { public void addResult(Validated validated) { synchronized (validations) { ValidationRule rule = validated.getRule(); - if (rule != null && !rule.equals(ValidationRule.empty())) { + if (rule != null && !rule.equals(ValidationRule.empty()) && !validations.contains(validated)) { validations.add(validated); } } } + + public ValidationResult consume(ValidationResult other) { + synchronized (validations) { + validations.addAll(other.validations); + } + return this; + } } diff --git a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationRule.java b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationRule.java index e220e0482cf..8d5818b2887 100644 --- a/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationRule.java +++ b/modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationRule.java @@ -26,7 +26,7 @@ public class ValidationRule { private Severity severity; private String description; private String failureMessage; - private Function test; + private Function test; /** * Constructs a new instance of {@link ValidationRule} @@ -37,7 +37,7 @@ public class ValidationRule { * @param test The test condition to be applied as a part of this rule, when this function returns true, * the evaluated instance will be considered "valid" according to this rule. */ - ValidationRule(Severity severity, String description, String failureMessage, Function test) { + ValidationRule(Severity severity, String description, String failureMessage, Function test) { this.severity = severity; this.description = description; this.failureMessage = failureMessage; @@ -60,7 +60,7 @@ public class ValidationRule { * * @return true if the object state is valid according to this rule, otherwise false. */ - public boolean evaluate(Object input) { + public Result evaluate(Object input) { return test.apply(input); } @@ -90,7 +90,7 @@ public class ValidationRule { * @return An "empty" rule. */ static ValidationRule empty() { - return new ValidationRule(Severity.ERROR, "empty", "failure message", (i) -> false); + return new ValidationRule(Severity.ERROR, "empty", "failure message", (i) -> Fail.empty() ); } /** @@ -106,8 +106,8 @@ public class ValidationRule { * @return A new instance of a {@link ValidationRule} */ @SuppressWarnings("unchecked") - public static ValidationRule create(Severity severity, String description, String failureMessage, Function fn) { - return new ValidationRule(severity, description, failureMessage, (Function) fn); + public static ValidationRule create(Severity severity, String description, String failureMessage, Function fn) { + return new ValidationRule(severity, description, failureMessage, (Function) fn); } /** @@ -121,8 +121,8 @@ public class ValidationRule { * @return A new instance of a {@link ValidationRule} */ @SuppressWarnings("unchecked") - public static ValidationRule error(String failureMessage, Function fn) { - return new ValidationRule(Severity.ERROR, null, failureMessage, (Function) fn); + public static ValidationRule error(String failureMessage, Function fn) { + return new ValidationRule(Severity.ERROR, null, failureMessage, (Function) fn); } /** @@ -137,8 +137,8 @@ public class ValidationRule { * @return A new instance of a {@link ValidationRule} */ @SuppressWarnings("unchecked") - public static ValidationRule warn(String description, String failureMessage, Function fn) { - return new ValidationRule(Severity.WARNING, description, failureMessage, (Function) fn); + public static ValidationRule warn(String description, String failureMessage, Function fn) { + return new ValidationRule(Severity.WARNING, description, failureMessage, (Function) fn); } @Override @@ -149,4 +149,69 @@ public class ValidationRule { ", failureMessage='" + failureMessage + '\'' + '}'; } + + public static abstract class Result { + protected String details = null; + protected Throwable throwable = null; + + public String getDetails() { + return details; + } + + public void setDetails(String details) { + assert this.details == null; + this.details = details; + } + + public abstract boolean passed(); + public final boolean failed() { return !passed(); } + + public Throwable getThrowable() { + return throwable; + } + + public boolean thrown() { return this.throwable == null; } + } + + public static final class Pass extends Result { + public static Result empty() { return new Pass(); } + + public Pass() { + super(); + } + + public Pass(String details) { + this(); + this.details = details; + } + + @Override + public boolean passed() { + return true; + } + } + + public static final class Fail extends Result { + public static Result empty() { return new Fail(); } + + public Fail() { + super(); + } + + public Fail(String details) { + this(); + this.details = details; + } + + public Fail(String details, Throwable throwable) { + this(); + this.throwable = throwable; + this.details = details; + } + + @Override + public boolean passed() { + return false; + } + } } diff --git a/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/GenericValidatorTest.java b/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/GenericValidatorTest.java index d37dc827c84..3fa8874ff19 100644 --- a/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/GenericValidatorTest.java +++ b/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/GenericValidatorTest.java @@ -25,7 +25,7 @@ import java.util.List; import java.util.Optional; public class GenericValidatorTest { - class Person { + static class Person { private int age; private String name; @@ -35,33 +35,33 @@ public class GenericValidatorTest { } } - private static boolean isValidAge(Person person) { - return person.age > 0; + private static ValidationRule.Result checkAge(Person person) { + return person.age > 0 ? ValidationRule.Pass.empty() : ValidationRule.Fail.empty(); } - private static boolean isAdult(Person person) { - return person.age > 18; + private static ValidationRule.Result checkAdult(Person person) { + return person.age > 18 ? ValidationRule.Pass.empty() : ValidationRule.Fail.empty(); } - private static boolean isNameSet(Person person) { - return person.name != null && person.name.length() > 0; + private static ValidationRule.Result checkName(Person person) { + return (person.name != null && person.name.length() > 0) ? ValidationRule.Pass.empty() : ValidationRule.Fail.empty(); } - private static boolean isNameValid(Person person) { + private static ValidationRule.Result checkNamePattern(Person person) { String pattern = "^[A-Z][a-z]*$"; - return person.name.matches(pattern); + return person.name.matches(pattern) ? ValidationRule.Pass.empty() : ValidationRule.Fail.empty(); } - private static boolean isNameNormalLength(Person person) { - return person.name.length() < 10; + private static ValidationRule.Result checkNameNormalLength(Person person) { + return person.name.length() < 10? ValidationRule.Pass.empty() : ValidationRule.Fail.empty(); } private List validationRules = Arrays.asList( - ValidationRule.error("Age must be positive and more than zero", GenericValidatorTest::isValidAge), - ValidationRule.error("Only adults (18 years old and older)", GenericValidatorTest::isAdult), - ValidationRule.error("Name isn't set!", GenericValidatorTest::isNameSet), - ValidationRule.error("Name isn't formatted correct", GenericValidatorTest::isNameValid), - ValidationRule.warn("Name too long?", "Name may be too long.", GenericValidatorTest::isNameNormalLength) + ValidationRule.error("Age must be positive and more than zero", GenericValidatorTest::checkAge), + ValidationRule.error("Only adults (18 years old and older)", GenericValidatorTest::checkAdult), + ValidationRule.error("Name isn't set!", GenericValidatorTest::checkName), + ValidationRule.error("Name isn't formatted correct", GenericValidatorTest::checkNamePattern), + ValidationRule.warn("Name too long?", "Name may be too long.", GenericValidatorTest::checkNameNormalLength) ); @Test diff --git a/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/ValidationRuleTest.java b/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/ValidationRuleTest.java index ce0fa165ce7..b626085c8b3 100644 --- a/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/ValidationRuleTest.java +++ b/modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/ValidationRuleTest.java @@ -33,13 +33,13 @@ public class ValidationRuleTest { } } - private static boolean checkName(Sample input) { - return input.getName() != null && input.getName().length() > 7; + private static ValidationRule.Result checkName(Sample input) { + return (input.getName() != null && input.getName().length() > 7) ? ValidationRule.Pass.empty() : ValidationRule.Fail.empty(); } - private static boolean checkPattern(Sample input) { + private static ValidationRule.Result checkPattern(Sample input) { String pattern = "^[A-Z][a-z]*$"; - return input.getName() != null && input.getName().matches(pattern); + return (input.getName() != null && input.getName().matches(pattern)) ? ValidationRule.Pass.empty() : ValidationRule.Fail.empty(); } @Test @@ -49,10 +49,10 @@ public class ValidationRuleTest { Sample seven = new Sample("1234567"); Sample eight = new Sample("12345678"); ValidationRule result = ValidationRule.error("test", ValidationRuleTest::checkName); - assertFalse(result.evaluate(nil)); - assertFalse(result.evaluate(six)); - assertFalse(result.evaluate(seven)); - assertTrue(result.evaluate(eight)); + assertFalse(result.evaluate(nil).passed()); + assertFalse(result.evaluate(six).passed()); + assertFalse(result.evaluate(seven).passed()); + assertTrue(result.evaluate(eight).passed()); } @Test @@ -61,8 +61,8 @@ public class ValidationRuleTest { Sample lowercase = new Sample("jim"); Sample titlecase = new Sample("Jim"); ValidationRule result = ValidationRule.error("test", i -> checkPattern((Sample)i)); - assertFalse(result.evaluate(nil)); - assertFalse(result.evaluate(lowercase)); - assertTrue(result.evaluate(titlecase)); + assertFalse(result.evaluate(nil).passed()); + assertFalse(result.evaluate(lowercase).passed()); + assertTrue(result.evaluate(titlecase).passed()); } } 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 new file mode 100644 index 00000000000..e28a69b4386 --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiEvaluator.java @@ -0,0 +1,94 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.Components; +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.Paths; +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.oas.models.parameters.Parameter; +import io.swagger.v3.oas.models.security.SecurityScheme; +import org.openapitools.codegen.utils.ModelUtils; +import org.openapitools.codegen.validation.*; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +/** + * A validator which evaluates an OpenAPI 3.x specification document + */ +public class OpenApiEvaluator implements Validator { + private RuleConfiguration ruleConfiguration; + + /** + * Constructs a new instance of {@link OpenApiEvaluator} with applied rules. + * + * @param ruleConfiguration The set of rules to be applied to evaluation. + */ + public OpenApiEvaluator(RuleConfiguration ruleConfiguration) { + this.ruleConfiguration = ruleConfiguration; + } + + /** + * Validates input, resulting in a instance of {@link ValidationResult} which provides details on all validations performed (success, error, warning). + * + * @param specification The {@link OpenAPI} object instance to be validated. + * @return A {@link ValidationResult} which details the success, error, and warning validation results. + */ + @Override + public ValidationResult validate(OpenAPI specification) { + ValidationResult validationResult = new ValidationResult(); + if (specification == null) return validationResult; + + OpenApiParameterValidations parameterValidations = new OpenApiParameterValidations(ruleConfiguration); + OpenApiSecuritySchemeValidations securitySchemeValidations = new OpenApiSecuritySchemeValidations(ruleConfiguration); + OpenApiSchemaValidations schemaValidations = new OpenApiSchemaValidations(ruleConfiguration); + OpenApiOperationValidations operationValidations = new OpenApiOperationValidations(ruleConfiguration); + + if (ruleConfiguration.isEnableUnusedSchemasRecommendation()) { + ValidationRule unusedSchema = ValidationRule.create(Severity.WARNING, "Unused schema", "A schema was determined to be unused.", s -> ValidationRule.Pass.empty()); + ModelUtils.getUnusedSchemas(specification).forEach(schemaName -> validationResult.addResult(Validated.invalid(unusedSchema, "Unused model: " + schemaName))); + } + + Map schemas = ModelUtils.getSchemas(specification); + schemas.forEach((key, schema) -> validationResult.consume(schemaValidations.validate(schema))); + + List parameters = new ArrayList<>(50); + + Paths paths = specification.getPaths(); + if (paths != null) { + paths.forEach((key, pathItem) -> { + // parameters defined "globally" + List pathParameters = pathItem.getParameters(); + if (pathParameters != null) parameters.addAll(pathItem.getParameters()); + + pathItem.readOperationsMap().forEach((httpMethod, op) -> { + if (op != null) { + // parameters on each operation method + if (op.getParameters() != null) { + parameters.addAll(op.getParameters()); + } + + OperationWrapper wrapper = new OperationWrapper(op, httpMethod); + validationResult.consume(operationValidations.validate(wrapper)); + } + }); + }); + } + + Components components = specification.getComponents(); + if (components != null) { + Map securitySchemes = components.getSecuritySchemes(); + if (securitySchemes != null && !securitySchemes.isEmpty()) { + securitySchemes.values().forEach(securityScheme -> validationResult.consume(securitySchemeValidations.validate(securityScheme))); + } + + if (components.getParameters() != null) { + parameters.addAll(components.getParameters().values()); + } + } + + parameters.forEach(parameter -> validationResult.consume(parameterValidations.validate(parameter))); + + return validationResult; + } +} diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidations.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidations.java new file mode 100644 index 00000000000..9b1de83e524 --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidations.java @@ -0,0 +1,75 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.parameters.RequestBody; +import org.apache.commons.lang3.StringUtils; +import org.openapitools.codegen.validation.GenericValidator; +import org.openapitools.codegen.validation.ValidationRule; + +import java.util.ArrayList; +import java.util.Locale; + +/** + * A standalone instance for evaluating rule and recommendations related to OAS {@link io.swagger.v3.oas.models.Operation} + */ +class OpenApiOperationValidations extends GenericValidator { + OpenApiOperationValidations(RuleConfiguration ruleConfiguration) { + super(new ArrayList<>()); + if (ruleConfiguration.isEnableRecommendations()) { + if (ruleConfiguration.isEnableApiRequestUriWithBodyRecommendation()) { + rules.add(ValidationRule.warn( + "API GET/HEAD defined with request body", + "While technically allowed, GET/HEAD with request body may indicate programming error, and is considered an anti-pattern.", + OpenApiOperationValidations::checkAntipatternGetOrHeadWithBody + )); + } + } + } + + /** + * Determines whether a GET or HEAD operation is configured to expect a body. + *

+ * RFC7231 describes this behavior as: + *

+ * A payload within a GET request message has no defined semantics; + * sending a payload body on a GET request might cause some existing + * implementations to reject the request. + *

+ * See https://tools.ietf.org/html/rfc7231#section-4.3.1 + *

+ * Because there are no defined semantics, and because some client and server implementations + * may silently ignore the entire body (see https://xhr.spec.whatwg.org/#the-send()-method) or + * throw an error (see https://fetch.spec.whatwg.org/#ref-for-dfn-throw%E2%91%A1%E2%91%A1), + * we maintain that the existence of a body for this operation is most likely programmer error and raise awareness. + * + * @param wrapper Wraps an operation with accompanying HTTP Method + * @return {@link ValidationRule.Pass} if the check succeeds, otherwise {@link ValidationRule.Fail} + */ + private static ValidationRule.Result checkAntipatternGetOrHeadWithBody(OperationWrapper wrapper) { + if (wrapper == null) { + return ValidationRule.Pass.empty(); + } + + ValidationRule.Result result = ValidationRule.Pass.empty(); + + if (wrapper.getHttpMethod() == PathItem.HttpMethod.GET || wrapper.getHttpMethod() == PathItem.HttpMethod.HEAD) { + RequestBody body = wrapper.getOperation().getRequestBody(); + + if (body != null) { + if (StringUtils.isNotEmpty(body.get$ref()) || (body.getContent() != null && body.getContent().size() > 0)) { + result = new ValidationRule.Fail(); + result.setDetails(String.format( + Locale.ROOT, + "%s %s contains a request body and is considered an anti-pattern.", + wrapper.getHttpMethod().name(), + wrapper.getOperation().getOperationId()) + ); + } + } + + } + + return result; + } + +} 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 new file mode 100644 index 00000000000..0e3585585a0 --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidations.java @@ -0,0 +1,47 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.parameters.HeaderParameter; +import io.swagger.v3.oas.models.parameters.Parameter; +import org.apache.commons.lang3.StringUtils; +import org.openapitools.codegen.validation.GenericValidator; +import org.openapitools.codegen.validation.ValidationRule; + +import java.util.ArrayList; +import java.util.Locale; + +/** + * A standalone instance for evaluating rules and recommendations related to OAS {@link Parameter} + */ +class OpenApiParameterValidations extends GenericValidator { + OpenApiParameterValidations(RuleConfiguration ruleConfiguration) { + super(new ArrayList<>()); + if (ruleConfiguration.isEnableRecommendations()) { + if (ruleConfiguration.isEnableApacheNginxUnderscoreRecommendation()) { + rules.add(ValidationRule.warn( + ValidationConstants.ApacheNginxUnderscoreDescription, + ValidationConstants.ApacheNginxUnderscoreFailureMessage, + OpenApiParameterValidations::apacheNginxHeaderCheck + )); + } + } + } + + /** + * Apache and Nginx default to legacy CGI behavior in which header with underscore are ignored. Raise this for awareness to the user. + * + * @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) { + if (parameter == null || !parameter.getIn().equals("header")) return ValidationRule.Pass.empty(); + ValidationRule.Result result = ValidationRule.Pass.empty(); + + String headerName = parameter.getName(); + if (StringUtils.isNotEmpty(headerName) && StringUtils.contains(headerName, '_')) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, "%s contains an underscore.", headerName)); + } + + return result; + } +} 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 new file mode 100644 index 00000000000..bfb829a8010 --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidations.java @@ -0,0 +1,57 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.media.ComposedSchema; +import io.swagger.v3.oas.models.media.Schema; +import org.openapitools.codegen.validation.GenericValidator; +import org.openapitools.codegen.validation.ValidationRule; + +import java.util.ArrayList; + +/** + * A standalone instance for evaluating rules and recommendations related to OAS {@link Schema} + */ +class OpenApiSchemaValidations extends GenericValidator { + OpenApiSchemaValidations(RuleConfiguration ruleConfiguration) { + super(new ArrayList<>()); + if (ruleConfiguration.isEnableRecommendations()) { + if (ruleConfiguration.isEnableOneOfWithPropertiesRecommendation()) { + rules.add(ValidationRule.warn( + "Schema defines properties alongside oneOf.", + "Schemas defining properties and oneOf are not clearly defined in the OpenAPI Specification. While our tooling supports this, it may cause issues with other tools.", + OpenApiSchemaValidations::checkOneOfWithProperties + )); + } + } + } + + /** + * JSON Schema defines oneOf as a validation property which can be applied to any schema. + *

+ * OpenAPI Specification is a variant of JSON Schema for which oneOf is defined as: + * "Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema." + *

+ * 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. + * + * @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 checkOneOfWithProperties(Schema schema) { + ValidationRule.Result result = ValidationRule.Pass.empty(); + + if (schema instanceof ComposedSchema) { + final ComposedSchema composed = (ComposedSchema) schema; + // check for loosely defined oneOf extension requirements. + // This is a recommendation because the 3.0.x spec is not clear enough on usage of oneOf. + // see https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.9.2.1.3 and the OAS section on 'Composition and Inheritance'. + if (composed.getOneOf() != null && composed.getOneOf().size() > 0) { + if (composed.getProperties() != null && composed.getProperties().size() >= 1 && composed.getProperties().get("discriminator") == null) { + // not necessarily "invalid" here, but we trigger the recommendation which requires the method to return false. + result = ValidationRule.Fail.empty(); + } + } + } + return result; + } +} 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 new file mode 100644 index 00000000000..53bc3f5224e --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidations.java @@ -0,0 +1,47 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.security.SecurityScheme; +import org.apache.commons.lang3.StringUtils; +import org.openapitools.codegen.validation.GenericValidator; +import org.openapitools.codegen.validation.ValidationRule; + +import java.util.ArrayList; +import java.util.Locale; + +/** + * A standalone instance for evaluating rules and recommendations related to OAS {@link SecurityScheme} + */ +class OpenApiSecuritySchemeValidations extends GenericValidator { + OpenApiSecuritySchemeValidations(RuleConfiguration ruleConfiguration) { + super(new ArrayList<>()); + if (ruleConfiguration.isEnableRecommendations()) { + if (ruleConfiguration.isEnableApacheNginxUnderscoreRecommendation()) { + rules.add(ValidationRule.warn( + ValidationConstants.ApacheNginxUnderscoreDescription, + ValidationConstants.ApacheNginxUnderscoreFailureMessage, + OpenApiSecuritySchemeValidations::apacheNginxHeaderCheck + )); + } + } + } + + /** + * Apache and Nginx default to legacy CGI behavior in which header with underscore are ignored. Raise this for awareness to the user. + * + * @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) { + if (securityScheme == null || securityScheme.getIn() != SecurityScheme.In.HEADER) + return ValidationRule.Pass.empty(); + ValidationRule.Result result = ValidationRule.Pass.empty(); + + String key = securityScheme.getName(); + if (StringUtils.contains(key, '_')) { + result = new ValidationRule.Fail(); + result.setDetails(String.format(Locale.ROOT, "%s contains an underscore.", key)); + } + + return result; + } +} 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 new file mode 100644 index 00000000000..c2e3b249fda --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/OperationWrapper.java @@ -0,0 +1,42 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.Operation; +import io.swagger.v3.oas.models.PathItem; + +/** + * 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 { + private Operation operation; + private PathItem.HttpMethod httpMethod; + + /** + * Constructs a new instance of {@link OperationWrapper} + * + * @param operation The operation instances to wrap + * @param httpMethod The http method to wrap + */ + OperationWrapper(Operation operation, PathItem.HttpMethod httpMethod) { + this.operation = operation; + this.httpMethod = httpMethod; + } + + /** + * Gets the operation associated with the http method + * + * @return An operation instance + */ + public Operation getOperation() { + return operation; + } + + /** + * Gets the http method associated with the operation + * + * @return The http method + */ + public PathItem.HttpMethod getHttpMethod() { + return httpMethod; + } +} 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 new file mode 100644 index 00000000000..c8238ef590e --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/RuleConfiguration.java @@ -0,0 +1,137 @@ +package org.openapitools.codegen.validations.oas; + +/** + * Allows for configuration of validation rules which will be applied to a specification. + */ +@SuppressWarnings({"WeakerAccess", "unused"}) +public class RuleConfiguration { + private static String propertyPrefix = "openapi.generator.rule"; + private boolean enableRecommendations = defaultedBoolean(propertyPrefix + ".recommendations", true); + 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 enableApiRequestUriWithBodyRecommendation = defaultedBoolean(propertyPrefix + ".anti-patterns.uri-unexpected-body", true); + + @SuppressWarnings("SameParameterValue") + private static boolean defaultedBoolean(String key, boolean defaultValue) { + String property = System.getProperty(key); + if (property == null) return defaultValue; + return Boolean.parseBoolean(property); + } + + /** + * Gets whether we will raise awareness that header parameters with underscore may be ignored in Apache or Nginx by default. + * For more details, see https://stackoverflow.com/a/22856867/151445. + * + * @return true if enabled, false if disabled + */ + public boolean isEnableApacheNginxUnderscoreRecommendation() { + return enableApacheNginxUnderscoreRecommendation; + } + + /** + * Enable or Disable the recommendation check for Apache/Nginx potentially ignoring header with underscore by default. + *

+ * For more details, see {@link RuleConfiguration#isEnableApacheNginxUnderscoreRecommendation()} + * + * @param enableApacheNginxUnderscoreRecommendation true to enable, false to disable + */ + public void setEnableApacheNginxUnderscoreRecommendation(boolean enableApacheNginxUnderscoreRecommendation) { + this.enableApacheNginxUnderscoreRecommendation = enableApacheNginxUnderscoreRecommendation; + } + + /** + * Gets whether we will raise awareness a GET or HEAD operation is defined with body. + * + * @return true if enabled, false if disabled + */ + public boolean isEnableApiRequestUriWithBodyRecommendation() { + return enableApiRequestUriWithBodyRecommendation; + } + + /** + * Enable or Disable the recommendation check for GET or HEAD operations with bodies. + *

+ * For more details, see {@link RuleConfiguration#isEnableApiRequestUriWithBodyRecommendation()} + * + * @param enableApiRequestUriWithBodyRecommendation true to enable, false to disable + */ + public void setEnableApiRequestUriWithBodyRecommendation(boolean enableApiRequestUriWithBodyRecommendation) { + this.enableApiRequestUriWithBodyRecommendation = enableApiRequestUriWithBodyRecommendation; + } + + /** + * Gets whether the recommendation check for oneOf with sibling properties exists. + *

+ * JSON Schema defines oneOf as a validation property which can be applied to any schema. + *

+ * OpenAPI Specification is a variant of JSON Schema for which oneOf is defined as: + * "Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema." + *

+ * 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. + * + * @return true if enabled, false if disabled + */ + public boolean isEnableOneOfWithPropertiesRecommendation() { + return enableOneOfWithPropertiesRecommendation; + } + + /** + * Enable or Disable the recommendation check for schemas containing properties and oneOf definitions. + *

+ * For more details, see {@link RuleConfiguration#isEnableOneOfWithPropertiesRecommendation()} + * + * @param enableOneOfWithPropertiesRecommendation true to enable, false to disable + */ + public void setEnableOneOfWithPropertiesRecommendation(boolean enableOneOfWithPropertiesRecommendation) { + this.enableOneOfWithPropertiesRecommendation = enableOneOfWithPropertiesRecommendation; + } + + /** + * Get whether recommendations are enabled. + * + * @return true if enabled, false if disabled + */ + public boolean isEnableRecommendations() { + return enableRecommendations; + } + + /** + * Enable or Disable recommendations. Recommendations are either informational or warning level type validations + * which are raised to communicate issues to the user which they may not be aware of, or for which support in the + * tooling/spec may not be clearly defined. + * + * @param enableRecommendations true to enable, false to disable + */ + public void setEnableRecommendations(boolean enableRecommendations) { + this.enableRecommendations = enableRecommendations; + } + + /** + * Gets whether the recommendation to check for unused schemas is enabled. + *

+ * While the tooling may or may not support generation of models representing unused schemas, we take the stance that + * a schema which is defined but not referenced in an operation or by some schema bound to an operation may be a good + * indicator of a programming error. We surface this information to the user in case the orphaned schema(s) are not + * intentional. + * + * @return true if enabled, false if disabled + */ + public boolean isEnableUnusedSchemasRecommendation() { + return enableUnusedSchemasRecommendation; + } + + /** + * Enable or Disable the recommendation check for unused schemas. + *

+ * For more details, see {@link RuleConfiguration#isEnableUnusedSchemasRecommendation()} + * + * @param enableUnusedSchemasRecommendation true to enable, false to disable + */ + public void setEnableUnusedSchemasRecommendation(boolean enableUnusedSchemasRecommendation) { + this.enableUnusedSchemasRecommendation = enableUnusedSchemasRecommendation; + } +} diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/ValidationConstants.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/ValidationConstants.java new file mode 100644 index 00000000000..9571f1bedcd --- /dev/null +++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/validations/oas/ValidationConstants.java @@ -0,0 +1,6 @@ +package org.openapitools.codegen.validations.oas; + +final class ValidationConstants { + static String ApacheNginxUnderscoreDescription = "Apache and Nginx may fail on headers keys with underscore!"; + static String ApacheNginxUnderscoreFailureMessage = "Apache and Nginx webservers may fail due to legacy CGI constraints enabled by default in which header keys with underscore are disallowed. See https://stackoverflow.com/a/22856867/151445."; +} 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 new file mode 100644 index 00000000000..c4d911563ec --- /dev/null +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiOperationValidationsTest.java @@ -0,0 +1,116 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.Operation; +import io.swagger.v3.oas.models.PathItem; +import io.swagger.v3.oas.models.media.Content; +import io.swagger.v3.oas.models.media.MediaType; +import io.swagger.v3.oas.models.parameters.RequestBody; +import org.apache.commons.lang3.StringUtils; +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.List; +import java.util.stream.Collectors; + +public class OpenApiOperationValidationsTest { + @DataProvider(name = "getOrHeadWithBodyExpectations") + public Object[][] getOrHeadWithBodyExpectations() { + return new Object[][]{ + /* method */ /* operationId */ /* ref */ /* content */ /* triggers warning */ + {PathItem.HttpMethod.GET, "opWithRef", "#/components/schemas/Animal", null, true}, + {PathItem.HttpMethod.GET, "opWithContent", null, new Content().addMediaType("a", new MediaType()), true}, + {PathItem.HttpMethod.GET, "opWithoutRefOrContent", null, null, false}, + {PathItem.HttpMethod.HEAD, "opWithRef", "#/components/schemas/Animal", null, true}, + {PathItem.HttpMethod.HEAD, "opWithContent", null, new Content().addMediaType("a", new MediaType()), true}, + {PathItem.HttpMethod.HEAD, "opWithoutRefOrContent", null, null, false}, + {PathItem.HttpMethod.POST, "opWithRef", "#/components/schemas/Animal", null, false}, + {PathItem.HttpMethod.POST, "opWithContent", null, new Content().addMediaType("a", new MediaType()), false}, + {PathItem.HttpMethod.POST, "opWithoutRefOrContent", null, null, false} + }; + } + + @Test(dataProvider = "getOrHeadWithBodyExpectations") + public void testGetOrHeadWithBody(PathItem.HttpMethod method, String operationId, String ref, Content content, boolean shouldTriggerFailure) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(true); + OpenApiOperationValidations validator = new OpenApiOperationValidations(config); + + Operation op = new Operation().operationId(operationId); + RequestBody body = new RequestBody(); + if (StringUtils.isNotEmpty(ref) || content != null) { + body.$ref(ref); + body.content(content); + + op.setRequestBody(body); + } + + ValidationResult result = validator.validate(new OperationWrapper(op, method)); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> "API GET/HEAD defined with request body".equals(invalid.getRule().getDescription())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + if (shouldTriggerFailure) { + Assert.assertEquals(warnings.size(), 1, "Expected warnings to include recommendation."); + } else { + Assert.assertEquals(warnings.size(), 0, "Expected warnings not to include recommendation."); + } + } + + @Test(dataProvider = "getOrHeadWithBodyExpectations") + public void testGetOrHeadWithBodyWithDisabledRecommendations(PathItem.HttpMethod method, String operationId, String ref, Content content, boolean shouldTriggerFailure) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(false); + OpenApiOperationValidations validator = new OpenApiOperationValidations(config); + + Operation op = new Operation().operationId(operationId); + RequestBody body = new RequestBody(); + if (StringUtils.isNotEmpty(ref) || content != null) { + body.$ref(ref); + body.content(content); + + op.setRequestBody(body); + } + + ValidationResult result = validator.validate(new OperationWrapper(op, method)); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> "API GET/HEAD defined with request body".equals(invalid.getRule().getDescription())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + Assert.assertEquals(warnings.size(), 0, "Expected warnings not to include recommendation."); + } + + @Test(dataProvider = "getOrHeadWithBodyExpectations") + public void testGetOrHeadWithBodyWithDisabledRule(PathItem.HttpMethod method, String operationId, String ref, Content content, boolean shouldTriggerFailure) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableApiRequestUriWithBodyRecommendation(false); + OpenApiOperationValidations validator = new OpenApiOperationValidations(config); + + Operation op = new Operation().operationId(operationId); + RequestBody body = new RequestBody(); + if (StringUtils.isNotEmpty(ref) || content != null) { + body.$ref(ref); + body.content(content); + + op.setRequestBody(body); + } + + ValidationResult result = validator.validate(new OperationWrapper(op, method)); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> "API GET/HEAD defined with request body".equals(invalid.getRule().getDescription())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + Assert.assertEquals(warnings.size(), 0, "Expected warnings not to include recommendation."); + } +} 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 new file mode 100644 index 00000000000..a242a355e3a --- /dev/null +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiParameterValidationsTest.java @@ -0,0 +1,93 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.parameters.Parameter; +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.List; +import java.util.stream.Collectors; + +public class OpenApiParameterValidationsTest { + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "disable apache nginx via turning off recommendations") + public void testApacheNginxWithDisabledRecommendations(String in, String key, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(false); + OpenApiParameterValidations validator = new OpenApiParameterValidations(config); + + Parameter parameter = new Parameter(); + parameter.setIn(in); + parameter.setName(key); + + ValidationResult result = validator.validate(parameter); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> ValidationConstants.ApacheNginxUnderscoreFailureMessage.equals(invalid.getMessage())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + Assert.assertEquals(warnings.size(), 0, "Expected recommendations to be disabled completely."); + } + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "disable apache nginx via turning off recommendations") + public void testApacheNginxWithDisabledRule(String in, String key, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableApacheNginxUnderscoreRecommendation(false); + OpenApiParameterValidations validator = new OpenApiParameterValidations(config); + + Parameter parameter = new Parameter(); + parameter.setIn(in); + parameter.setName(key); + + ValidationResult result = validator.validate(parameter); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> ValidationConstants.ApacheNginxUnderscoreFailureMessage.equals(invalid.getMessage())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + Assert.assertEquals(warnings.size(), 0, "Expected rule to be disabled."); + } + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "default apache nginx recommendation") + public void testDefaultRecommendationApacheNginx(String in, String key, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(true); + OpenApiParameterValidations validator = new OpenApiParameterValidations(config); + + Parameter parameter = new Parameter(); + parameter.setIn(in); + parameter.setName(key); + + ValidationResult result = validator.validate(parameter); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> ValidationConstants.ApacheNginxUnderscoreFailureMessage.equals(invalid.getMessage())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + if (matches) { + Assert.assertEquals(warnings.size(), 1, "Expected " + key + " to match recommendation."); + } else { + Assert.assertEquals(warnings.size(), 0, "Expected " + key + " not to match recommendation."); + } + } + + @DataProvider(name = "apacheNginxRecommendationExpectations") + public Object[][] apacheNginxRecommendationExpectations() { + return new Object[][]{ + {"header", "api_key", true}, + {"header", "apikey", false}, + {"cookie", "api_key", false}, + {"cookie", "apikey", false}, + {"query", "api_key", false}, + {"query", "apikey", false} + }; + } +} 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 new file mode 100644 index 00000000000..2f8ad8ce5c9 --- /dev/null +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSchemaValidationsTest.java @@ -0,0 +1,128 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.media.*; +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 OpenApiSchemaValidationsTest { + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "default oneOf with sibling properties recommendation") + public void testDefaultRecommendationOneOfWithSiblingProperties(Schema schema, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(true); + OpenApiSchemaValidations validator = new OpenApiSchemaValidations(config); + + ValidationResult result = validator.validate(schema); + 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); + if (matches) { + Assert.assertEquals(warnings.size(), 1, "Expected to match recommendation."); + } else { + Assert.assertEquals(warnings.size(), 0, "Expected not to match recommendation."); + } + } + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "disable oneOf with sibling properties recommendation via turning off recommendations") + public void testOneOfWithSiblingPropertiesDisabledRecommendations(Schema schema, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(false); + OpenApiSchemaValidations validator = new OpenApiSchemaValidations(config); + + ValidationResult result = validator.validate(schema); + 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); + Assert.assertEquals(warnings.size(), 0, "Expected recommendations to be disabled completely."); + } + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "disable oneOf with sibling properties recommendation via turning off rule") + public void testOneOfWithSiblingPropertiesDisabledRule(Schema schema, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableOneOfWithPropertiesRecommendation(false); + OpenApiSchemaValidations validator = new OpenApiSchemaValidations(config); + + ValidationResult result = validator.validate(schema); + 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); + Assert.assertEquals(warnings.size(), 0, "Expected rule to be disabled."); + } + + @DataProvider(name = "apacheNginxRecommendationExpectations") + public Object[][] apacheNginxRecommendationExpectations() { + return new Object[][]{ + {getOneOfSample(true), true}, + {getOneOfSample(false), false}, + {getAllOfSample(true), false}, + {getAllOfSample(false), false}, + {getAnyOfSample(true), false}, + {getAnyOfSample(false), false}, + {new StringSchema(), false}, + {new MapSchema(), false}, + {new ArraySchema(), false}, + {new ObjectSchema(), false} + }; + } + + private ComposedSchema getOneOfSample(boolean withProperties) { + ComposedSchema schema = new ComposedSchema().oneOf(Arrays.asList( + new StringSchema(), + new IntegerSchema().format("int64")) + ); + + if (withProperties) { + schema.addProperties("other", new ArraySchema() + .items(new Schema().$ref("#/definitions/Other"))); + } + + return schema; + } + + private ComposedSchema getAllOfSample(boolean withProperties) { + // This doesn't matter if it's realistic; it's a structural check + ComposedSchema schema = new ComposedSchema().allOf(Arrays.asList( + new StringSchema(), + new IntegerSchema().format("int64")) + ); + + if (withProperties) { + schema.addProperties("other", new ArraySchema() + .items(new Schema().$ref("#/definitions/Other"))); + } + + return schema; + } + + private ComposedSchema getAnyOfSample(boolean withProperties) { + ComposedSchema schema = new ComposedSchema().anyOf(Arrays.asList( + new StringSchema(), + new IntegerSchema().format("int64")) + ); + + if (withProperties) { + schema.addProperties("other", new ArraySchema() + .items(new Schema().$ref("#/definitions/Other"))); + } + + return schema; + } +} \ No newline at end of file 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 new file mode 100644 index 00000000000..aae32dd3012 --- /dev/null +++ b/modules/openapi-generator/src/test/java/org/openapitools/codegen/validations/oas/OpenApiSecuritySchemeValidationsTest.java @@ -0,0 +1,87 @@ +package org.openapitools.codegen.validations.oas; + +import io.swagger.v3.oas.models.security.SecurityScheme; +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.List; +import java.util.stream.Collectors; + +public class OpenApiSecuritySchemeValidationsTest { + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "disable apache nginx via turning off recommendations") + public void testApacheNginxWithDisabledRecommendations(SecurityScheme.In in, String key, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(false); + OpenApiSecuritySchemeValidations validator = new OpenApiSecuritySchemeValidations(config); + + SecurityScheme securityScheme = new SecurityScheme().in(in).name(key); + + ValidationResult result = validator.validate(securityScheme); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> ValidationConstants.ApacheNginxUnderscoreFailureMessage.equals(invalid.getMessage())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + Assert.assertEquals(warnings.size(), 0, "Expected recommendations to be disabled completely."); + } + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "disable apache nginx via turning off rule") + public void testApacheNginxWithDisabledRule(SecurityScheme.In in, String key, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableApacheNginxUnderscoreRecommendation(false); + OpenApiSecuritySchemeValidations validator = new OpenApiSecuritySchemeValidations(config); + + SecurityScheme securityScheme = new SecurityScheme().in(in).name(key); + + ValidationResult result = validator.validate(securityScheme); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> ValidationConstants.ApacheNginxUnderscoreFailureMessage.equals(invalid.getMessage())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + Assert.assertEquals(warnings.size(), 0, "Expected rule to be disabled."); + } + + @Test(dataProvider = "apacheNginxRecommendationExpectations", description = "default apache nginx recommendation") + public void testDefaultRecommendationApacheNginx(SecurityScheme.In in, String key, boolean matches) { + RuleConfiguration config = new RuleConfiguration(); + config.setEnableRecommendations(true); + OpenApiSecuritySchemeValidations validator = new OpenApiSecuritySchemeValidations(config); + + SecurityScheme securityScheme = new SecurityScheme().in(in).name(key); + + ValidationResult result = validator.validate(securityScheme); + Assert.assertNotNull(result.getWarnings()); + + List warnings = result.getWarnings().stream() + .filter(invalid -> ValidationConstants.ApacheNginxUnderscoreFailureMessage.equals(invalid.getMessage())) + .collect(Collectors.toList()); + + Assert.assertNotNull(warnings); + if (matches) { + Assert.assertEquals(warnings.size(), 1, "Expected " + key + " to match recommendation."); + } else { + Assert.assertEquals(warnings.size(), 0, "Expected " + key + " not to match recommendation."); + } + } + + @DataProvider(name = "apacheNginxRecommendationExpectations") + public Object[][] apacheNginxRecommendationExpectations() { + return new Object[][]{ + {SecurityScheme.In.HEADER, "api_key", true}, + {SecurityScheme.In.HEADER, "apikey", false}, + {SecurityScheme.In.COOKIE, "api_key", false}, + {SecurityScheme.In.COOKIE, "apikey", false}, + {SecurityScheme.In.QUERY, "api_key", false}, + {SecurityScheme.In.COOKIE, "apikey", false} + }; + } +} \ No newline at end of file