validation: cleanup

- make constrainctvalidation cast safe - use config api for properties
jooby-project · Sep 7, 2024 · 1007f4b · 1007f4b
1 parent 4b3ab1d
commit 1007f4b
Show file tree

Hide file tree

Showing 6 changed files with 256 additions and 194 deletions.
diff --git a/...es/jooby-avaje-validator/src/main/java/io/jooby/avaje/validator/AvajeValidatorModule.java b/...es/jooby-avaje-validator/src/main/java/io/jooby/avaje/validator/AvajeValidatorModule.java
@@ -7,12 +7,12 @@
 
 import java.time.Duration;
 import java.time.temporal.ChronoUnit;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Locale;
-import java.util.Optional;
+import java.util.*;
 import java.util.function.Consumer;
+import java.util.function.Function;
 
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigValueType;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import io.avaje.validation.ConstraintViolationException;
 import io.avaje.validation.Validator;
@@ -46,11 +46,11 @@
  * ConstraintViolationException} and transforms it into a {@link
  * io.jooby.validation.ValidationResult}
  *
- * @authors kliushnichenko, SentryMan
+ * @author kliushnichenko
+ * @author SentryMan
  * @since 3.3.1
  */
 public class AvajeValidatorModule implements Extension {
-
   private Consumer<Validator.Builder> configurer;
   private StatusCode statusCode = StatusCode.UNPROCESSABLE_ENTITY;
   private String title = "Validation failed";
@@ -105,43 +105,34 @@ public AvajeValidatorModule disableViolationHandler() {
   }
 
   @Override
-  public void install(@NonNull Jooby app) throws Exception {
-
-    var props = app.getEnvironment();
+  public void install(@NonNull Jooby app) {
+    var conf = app.getConfig();
 
-    final var locales = new ArrayList<Locale>();
     final var builder = Validator.builder();
-    Optional.ofNullable(props.getProperty("validation.failFast", "false"))
-        .map(Boolean::valueOf)
-        .ifPresent(builder::failFast);
+    withProperty(conf, "validation.failFast", conf::getBoolean).ifPresent(builder::failFast);
 
-    Optional.ofNullable(props.getProperty("validation.resourcebundle.names"))
-        .map(s -> s.split(","))
-        .ifPresent(builder::addResourceBundles);
-
-    Optional.ofNullable(props.getProperty("validation.locale.default"))
-        .map(Locale::forLanguageTag)
+    withProperty(conf, "validation.resourcebundle.names", path -> getStringList(conf, path))
+        .ifPresent(values -> values.forEach(builder::addResourceBundles));
+    // Locales from application
+    Optional.ofNullable(app.getLocales())
         .ifPresent(
-            l -> {
-              builder.setDefaultLocale(l);
-              locales.add(l);
+            locales -> {
+              builder.setDefaultLocale(locales.get(0));
+              locales.stream().skip(1).forEach(builder::addLocales);
             });
-
-    Optional.ofNullable(props.getProperty("validation.locale.addedLocales")).stream()
-        .flatMap(s -> Arrays.stream(s.split(",")))
+    withProperty(conf, "validation.locale.default", conf::getString)
         .map(Locale::forLanguageTag)
-        .forEach(
-            l -> {
-              builder.addLocales(l);
-              locales.add(l);
-            });
-
-    Optional.ofNullable(props.getProperty("validation.temporal.tolerance.value"))
-        .map(Long::valueOf)
+        .ifPresent(builder::setDefaultLocale);
+    withProperty(conf, "validation.locale.addedLocales", path -> getStringList(conf, path))
+        .orElseGet(List::of)
+        .stream()
+        .map(Locale::forLanguageTag)
+        .forEach(builder::addLocales);
+    withProperty(conf, "validation.temporal.tolerance.value", conf::getLong)
         .ifPresent(
             duration -> {
-              final var unit =
-                  Optional.ofNullable(props.getProperty("validation.temporal.tolerance.chronoUnit"))
+              var unit =
+                  withProperty(conf, "validation.temporal.tolerance.chronoUnit", conf::getString)
                       .map(ChronoUnit::valueOf)
                       .orElse(ChronoUnit.MILLIS);
               builder.temporalTolerance(Duration.of(duration, unit));
@@ -151,13 +142,12 @@ public void install(@NonNull Jooby app) throws Exception {
       configurer.accept(builder);
     }
 
-    Validator validator = builder.build();
+    var validator = builder.build();
     app.getServices().put(Validator.class, validator);
     app.getServices().put(MvcValidator.class, new MvcValidatorImpl(validator));
 
     if (!disableDefaultViolationHandler) {
-      app.error(
-          ConstraintViolationException.class, new ConstraintViolationHandler(statusCode, title));
+      app.error(new ConstraintViolationHandler(statusCode, title));
     }
   }
 
@@ -174,4 +164,17 @@ public void validate(Context ctx, Object bean) throws ConstraintViolationExcepti
       validator.validate(bean, ctx.locale());
     }
   }
+
+  private static <T> Optional<T> withProperty(Config config, String path, Function<String, T> fn) {
+    if (config.hasPath(path)) {
+      return Optional.of(fn.apply(path));
+    }
+    return Optional.empty();
+  }
+
+  private static List<String> getStringList(Config config, String path) {
+    return config.getValue(path).valueType() == ConfigValueType.STRING
+        ? List.of(config.getString(path))
+        : config.getStringList(path);
+  }
 }
diff --git a/...by-avaje-validator/src/main/java/io/jooby/avaje/validator/ConstraintViolationHandler.java b/...by-avaje-validator/src/main/java/io/jooby/avaje/validator/ConstraintViolationHandler.java
@@ -1,3 +1,8 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
 package io.jooby.avaje.validator;
 
 import static io.jooby.validation.ValidationResult.ErrorType.FIELD;
@@ -49,36 +54,32 @@
  * @since 3.2.10
  */
 public class ConstraintViolationHandler implements ErrorHandler {
-
   private static final String ROOT_VIOLATIONS_PATH = "";
-
   private final StatusCode statusCode;
   private final String title;
 
-  public ConstraintViolationHandler(StatusCode statusCode, String title) {
+  public ConstraintViolationHandler(@NonNull StatusCode statusCode, @NonNull String title) {
     this.statusCode = statusCode;
     this.title = title;
   }
 
   @Override
   public void apply(@NonNull Context ctx, @NonNull Throwable cause, @NonNull StatusCode code) {
-    var ex = (ConstraintViolationException) cause;
+    if (cause instanceof ConstraintViolationException ex) {
+      var violations = ex.violations();
 
-    var violations = ex.violations();
+      var groupedByPath = violations.stream().collect(groupingBy(ConstraintViolation::path));
+      var errors = collectErrors(groupedByPath);
 
-    Map<String, List<ConstraintViolation>> groupedByPath =
-        violations.stream().collect(groupingBy(violation -> violation.path().toString()));
-
-    List<ValidationResult.Error> errors = collectErrors(groupedByPath);
-
-    ValidationResult result = new ValidationResult(title, statusCode.value(), errors);
-    ctx.setResponseCode(statusCode).render(result);
+      var result = new ValidationResult(title, statusCode.value(), errors);
+      ctx.setResponseCode(statusCode).render(result);
+    }
   }
 
   private List<ValidationResult.Error> collectErrors(
       Map<String, List<ConstraintViolation>> groupedViolations) {
     List<ValidationResult.Error> errors = new ArrayList<>();
-    for (Map.Entry<String, List<ConstraintViolation>> entry : groupedViolations.entrySet()) {
+    for (var entry : groupedViolations.entrySet()) {
       var path = entry.getKey();
       if (ROOT_VIOLATIONS_PATH.equals(path)) {
         errors.add(new ValidationResult.Error(null, extractMessages(entry.getValue()), GLOBAL));

diff --git a/modules/jooby-avaje-validator/src/main/java/io/jooby/avaje/validator/package-info.java b/modules/jooby-avaje-validator/src/main/java/io/jooby/avaje/validator/package-info.java
@@ -1 +1,25 @@
+/**
+ * Avaje Validator Module: https://jooby.io/modules/avaje-validator.
+ *
+ * <pre>{@code
+ * {
+ *   install(new AvajeValidatorModule());
+ *
+ * }
+ *
+ * public class Controller {
+ *
+ *   @POST("/create")
+ *   public void create(@Valid Bean bean) {
+ *   }
+ *
+ * }
+ * }</pre>
+ *
+ * <p>Supports validation of a single bean, list, array, or map.
+ *
+ * <p>The module also provides a built-in error handler that catches {@link
+ * io.avaje.validation.ConstraintViolationException} and transforms it into a {@link
+ * io.jooby.validation.ValidationResult}
+ */
 package io.jooby.avaje.validator;
diff --git a/...nate-validator/src/main/java/io/jooby/hibernate/validator/ConstraintViolationHandler.java b/...nate-validator/src/main/java/io/jooby/hibernate/validator/ConstraintViolationHandler.java
@@ -1,5 +1,18 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
 package io.jooby.hibernate.validator;
 
+import static io.jooby.validation.ValidationResult.ErrorType.FIELD;
+import static io.jooby.validation.ValidationResult.ErrorType.GLOBAL;
+import static java.util.stream.Collectors.groupingBy;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
 import edu.umd.cs.findbugs.annotations.NonNull;
 import io.jooby.Context;
 import io.jooby.ErrorHandler;
@@ -8,19 +21,11 @@
 import jakarta.validation.ConstraintViolation;
 import jakarta.validation.ConstraintViolationException;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
-import static io.jooby.validation.ValidationResult.ErrorType.FIELD;
-import static io.jooby.validation.ValidationResult.ErrorType.GLOBAL;
-import static java.util.stream.Collectors.groupingBy;
-
 /**
  * Catches and transform {@link ConstraintViolationException} into {@link ValidationResult}
- * <p>
- * Payload example:
+ *
+ * <p>Payload example:
+ *
  * <pre>{@code
  * {
  *    "title": "Validation failed",
@@ -50,45 +55,47 @@
  */
 public class ConstraintViolationHandler implements ErrorHandler {
 
-    private static final String ROOT_VIOLATIONS_PATH = "";
+  private static final String ROOT_VIOLATIONS_PATH = "";
 
-    private final StatusCode statusCode;
-    private final String title;
+  private final StatusCode statusCode;
+  private final String title;
 
-    public ConstraintViolationHandler(StatusCode statusCode, String title) {
-        this.statusCode = statusCode;
-        this.title = title;
-    }
+  public ConstraintViolationHandler(@NonNull StatusCode statusCode, @NonNull String title) {
+    this.statusCode = statusCode;
+    this.title = title;
+  }
 
-    @Override
-    public void apply(@NonNull Context ctx, @NonNull Throwable cause, @NonNull StatusCode code) {
-        ConstraintViolationException ex = (ConstraintViolationException) cause;
+  @Override
+  public void apply(@NonNull Context ctx, @NonNull Throwable cause, @NonNull StatusCode code) {
+    if (cause instanceof ConstraintViolationException ex) {
+      var violations = ex.getConstraintViolations();
 
-        Set<ConstraintViolation<?>> violations = ex.getConstraintViolations();
+      var groupedByPath =
+          violations.stream()
+              .collect(groupingBy(violation -> violation.getPropertyPath().toString()));
 
-        Map<String, List<ConstraintViolation<?>>> groupedByPath = violations.stream()
-                .collect(groupingBy(violation -> violation.getPropertyPath().toString()));
+      var errors = collectErrors(groupedByPath);
 
-        List<ValidationResult.Error> errors = collectErrors(groupedByPath);
-
-        ValidationResult result = new ValidationResult(title, statusCode.value(), errors);
-        ctx.setResponseCode(statusCode).render(result);
+      var result = new ValidationResult(title, statusCode.value(), errors);
+      ctx.setResponseCode(statusCode).render(result);
     }
+  }
 
-    private List<ValidationResult.Error> collectErrors(Map<String, List<ConstraintViolation<?>>> groupedViolations) {
-        List<ValidationResult.Error> errors = new ArrayList<>();
-        for (Map.Entry<String, List<ConstraintViolation<?>>> entry : groupedViolations.entrySet()) {
-            var path = entry.getKey();
-            if (ROOT_VIOLATIONS_PATH.equals(path)) {
-                errors.add(new ValidationResult.Error(null, extractMessages(entry.getValue()), GLOBAL));
-            } else {
-                errors.add(new ValidationResult.Error(path, extractMessages(entry.getValue()), FIELD));
-            }
-        }
-        return errors;
+  private List<ValidationResult.Error> collectErrors(
+      Map<String, List<ConstraintViolation<?>>> groupedViolations) {
+    List<ValidationResult.Error> errors = new ArrayList<>();
+    for (var entry : groupedViolations.entrySet()) {
+      var path = entry.getKey();
+      if (ROOT_VIOLATIONS_PATH.equals(path)) {
+        errors.add(new ValidationResult.Error(null, extractMessages(entry.getValue()), GLOBAL));
+      } else {
+        errors.add(new ValidationResult.Error(path, extractMessages(entry.getValue()), FIELD));
+      }
     }
+    return errors;
+  }
 
-    private List<String> extractMessages(List<ConstraintViolation<?>> violations) {
-        return violations.stream().map(ConstraintViolation::getMessage).toList();
-    }
+  private List<String> extractMessages(List<ConstraintViolation<?>> violations) {
+    return violations.stream().map(ConstraintViolation::getMessage).toList();
+  }
 }