[core] Extracting recommendations to validation framework

modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java

@@ -22,14 +22,14 @@
 
 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 void run() {
         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<String> unusedModels = ModelUtils.getUnusedSchemas(specification);
-                if (unusedModels != null) {
-                    unusedModels.forEach(name -> warnings.add("Unused model: " + name));
-                }
-
-                // 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<String, Schema> 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);
-                            }
-                        }
-                    }
-                });
-            }
-        }
+        RuleConfiguration ruleConfiguration = new RuleConfiguration();
+        ruleConfiguration.setEnableRecommendations(recommend != null ? recommend : false);
+
+        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())
             );
         }
 

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

@@ -23,9 +23,8 @@
  *
  * @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 ValidationResult validate(TInput input) {
         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()));
                 }
             });
         }

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

@@ -23,6 +23,7 @@
 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;
     }
 

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

@@ -54,6 +54,18 @@ boolean isValid() {
     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.

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

@@ -24,7 +24,6 @@
 /**
  * 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 List<Invalid> getWarnings(){
     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;
+    }
 }

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<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 String getFailureMessage() {
      *
      * @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 String getDescription() {
      * @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 @@ static ValidationRule empty() {
      * @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 static <T> ValidationRule create(Severity severity, String description, S
      * @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 static <T> ValidationRule error(String failureMessage, Function<T, Boolea
      * @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 String toString() {
                 ", 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;
+        }
+    }
 }

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

@@ -25,7 +25,7 @@
 import java.util.Optional;
 
 public class GenericValidatorTest {
-    class Person {
+    static class Person {
         private int age;
         private String name;
 
@@ -35,33 +35,33 @@ class Person {
         }
     }
 
-    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