loafle/openapi-generator
3
0
Fork 0
forked from loafle/openapi-generator-original
Code Pull Requests Activity

[core] Extracting recommendations to validation framework (#4979)

Browse Source 
* [core] Extracting recommendations to validation framework

This is work to extract recommendation logic out of the CLI validate command
into a shared evaluation instance which can be used elsewhere (such as Gradle,
or the Online tool).

For now, these validations are in addition to those provided by swagger-parser and
are only the following recommendations:

* Apache/Nginx warning that header values with underscore are dropped by default
* Unused models/schemas
* Use of properties with oneOf, which is ambiguous in OpenAPI Specification

I've allowed for disabling recommendations via System properties, since this is
something that has been requested a few times by users. System properties in this
commit include:

* openapi.generator.rule.recommendations=false
  - Allows for disabling recommendations completely. This wouldn't include all warnings
    and errors, only those we deem to be suggestions
* openapi.generator.rule.apache-nginx-underscore=false
  - Allows for disabling the Apache/Nginx warning when header names have underscore
  - This is a legacy CGI configuration, and doesn't affect all web servers
* openapi.generator.rule.oneof-properties-ambiguity=false
  - We support this functionality, but the specification may not intend for it
  - This is more to reduce noise
* openapi.generator.rule.unused-schemas=false
  - We will warn when a schema is not referenced outside of Components, which
    users have requested to be able to turn off
* openapi.generator.rule.anti-patterns.uri-unexpected-body=false

* Move recommendation/validations to oas package and add javadoc comments

* Refactor and test recommendation validations

* Refactor validation function signatures to return explicit state rather than boolean

* Add operation recommendation for GET/HEAD w/body
This commit is contained in:
Jim Schubert
2020-01-31 12:19:16 -05:00
committed by GitHub
parent f06ac9d91c
commit 22c6c0ca68
20 changed files with 1089 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/GenericValidator.java
View File

@@ -23,9 +23,8 @@ import java.util.List;
*
* @param <TInput> The type of object being evaluated.
*/
@SuppressWarnings({"WeakerAccess"})
public class GenericValidator<TInput> implements Validator<TInput> {
private List<ValidationRule> rules;
protected List<ValidationRule> rules;
/**
* Constructs a new instance of {@link GenericValidator}.
@@ -48,11 +47,11 @@ public class GenericValidator<TInput> implements Validator<TInput> {
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()));
}
});
}

21
modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Invalid.java
View File

@@ -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;
}

12
modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/Validated.java
View File

@@ -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.

10
modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationResult.java
View File

@@ -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<Validated> 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;
}
}

85
modules/openapi-generator-core/src/main/java/org/openapitools/codegen/validation/ValidationRule.java
View File

@@ -26,7 +26,7 @@ public class ValidationRule {
private Severity severity;
private String description;
private String failureMessage;
private Function<Object, Boolean> test;
private Function<Object, Result> 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 <code>true</code>,
* the evaluated instance will be considered "valid" according to this rule.
*/
ValidationRule(Severity severity, String description, String failureMessage, Function<Object, Boolean> test) {
ValidationRule(Severity severity, String description, String failureMessage, Function<Object, Result> test) {
this.severity = severity;
this.description = description;
this.failureMessage = failureMessage;
@@ -60,7 +60,7 @@ public class ValidationRule {
*
* @return <code>true</code> if the object state is valid according to this rule, otherwise <code>false</code>.
*/
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 <T> ValidationRule create(Severity severity, String description, String failureMessage, Function<T, Boolean> fn) {
return new ValidationRule(severity, description, failureMessage, (Function<Object, Boolean>) fn);
public static <T> ValidationRule create(Severity severity, String description, String failureMessage, Function<T, Result> fn) {
return new ValidationRule(severity, description, failureMessage, (Function<Object, Result>) fn);
}
/**
@@ -121,8 +121,8 @@ public class ValidationRule {
* @return A new instance of a {@link ValidationRule}
*/
@SuppressWarnings("unchecked")
public static <T> ValidationRule error(String failureMessage, Function<T, Boolean> fn) {
return new ValidationRule(Severity.ERROR, null, failureMessage, (Function<Object, Boolean>) fn);
public static <T> ValidationRule error(String failureMessage, Function<T, Result> fn) {
return new ValidationRule(Severity.ERROR, null, failureMessage, (Function<Object, Result>) fn);
}
/**
@@ -137,8 +137,8 @@ public class ValidationRule {
* @return A new instance of a {@link ValidationRule}
*/
@SuppressWarnings("unchecked")
public static <T> ValidationRule warn(String description, String failureMessage, Function<T, Boolean> fn) {
return new ValidationRule(Severity.WARNING, description, failureMessage, (Function<Object, Boolean>) fn);
public static <T> ValidationRule warn(String description, String failureMessage, Function<T, Result> fn) {
return new ValidationRule(Severity.WARNING, description, failureMessage, (Function<Object, Result>) 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;
}
}
}

32
modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/GenericValidatorTest.java
View File

@@ -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<ValidationRule> 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

22
modules/openapi-generator-core/src/test/java/org/openapitools/codegen/validation/ValidationRuleTest.java
View File

@@ -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());
}
}