Merge pull request #3534 from kliushnichenko/validation-tests-cleanup

cleanup validation tests
jooby-project · Sep 18, 2024 · 7433c2c · 7433c2c
2 parents ca9cb7c + c68de34
commit 7433c2c
Show file tree

Hide file tree

Showing 13 changed files with 210 additions and 57 deletions.
diff --git a/docs/asciidoc/context.adoc b/docs/asciidoc/context.adoc
@@ -576,7 +576,7 @@ a javadoc:Session[] but the lifecycle is shorter: *data is kept for only one req
   });
 
   post("/save", ctx -> {
-    ctx.flash("success", "Item created");          <1>
+    ctx.flash().put("success", "Item created");    <1>
     return ctx.sendRedirect("/");                  <2>
   });
 ----
@@ -589,7 +589,7 @@ a javadoc:Session[] but the lifecycle is shorter: *data is kept for only one req
   }
   
   post("/save") { ctx ->
-    ctx.flash("success", "Item created")           <1>
+    ctx.flash().put("success", "Item created")     <1>
     ctx.sendRedirect("/")                          <2>
   }
 ----

diff --git a/modules/jooby-apt/pom.xml b/modules/jooby-apt/pom.xml
@@ -58,6 +58,12 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <scope>test</scope>
+    </dependency>
+
     <dependency>
       <groupId>org.jacoco</groupId>
       <artifactId>org.jacoco.agent</artifactId>

diff --git a/modules/jooby-apt/src/main/java/io/jooby/internal/apt/MvcParameter.java b/modules/jooby-apt/src/main/java/io/jooby/internal/apt/MvcParameter.java
@@ -100,11 +100,11 @@ yield hasAnnotation(NULLABLE)
           yield ParameterGenerator.BodyParam.toSourceCode(
               kt, route, null, type, parameterName, isNullable(kt));
         } else {
-          yield strategy
-              .get()
-              .getKey()
-              .toSourceCode(
-                  kt, route, strategy.get().getValue(), type, parameterName, isNullable(kt));
+          var paramGenerator = strategy.get().getKey();
+          paramGenerator.verifyType(parameterType, parameterName, route);
+
+          yield paramGenerator.toSourceCode(
+              kt, route, strategy.get().getValue(), type, parameterName, isNullable(kt));
         }
       }
     };

diff --git a/modules/jooby-apt/src/main/java/io/jooby/internal/apt/ParameterGenerator.java b/modules/jooby-apt/src/main/java/io/jooby/internal/apt/ParameterGenerator.java
@@ -6,13 +6,9 @@
 package io.jooby.internal.apt;
 
 import static io.jooby.internal.apt.AnnotationSupport.findAnnotationValue;
+import static io.jooby.internal.apt.Types.BUILT_IN;
 import static java.util.stream.Collectors.joining;
 
-import java.net.URI;
-import java.net.URL;
-import java.time.Duration;
-import java.time.Period;
-import java.time.ZoneId;
 import java.util.*;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
@@ -53,10 +49,10 @@ public String toSourceCode(
       }
     }
   },
-  CookieParam("cookie", "io.jooby.annotation.CookieParam", "jakarta.ws.rs.CookieParam"),
-  FlashParam("flash", "io.jooby.annotation.FlashParam"),
+  CookieParam("cookie", BUILT_IN, "io.jooby.annotation.CookieParam", "jakarta.ws.rs.CookieParam"),
+  FlashParam("flash", BUILT_IN, "io.jooby.annotation.FlashParam"),
   FormParam("form", "io.jooby.annotation.FormParam", "jakarta.ws.rs.FormParam"),
-  HeaderParam("header", "io.jooby.annotation.HeaderParam", "jakarta.ws.rs.HeaderParam"),
+  HeaderParam("header", BUILT_IN, "io.jooby.annotation.HeaderParam", "jakarta.ws.rs.HeaderParam"),
   Lookup("lookup", "io.jooby.annotation.Param") {
     @Override
     protected Predicate<String> namePredicate() {
@@ -65,7 +61,7 @@ protected Predicate<String> namePredicate() {
   },
   PathParam("path", "io.jooby.annotation.PathParam", "jakarta.ws.rs.PathParam"),
   QueryParam("query", "io.jooby.annotation.QueryParam", "jakarta.ws.rs.QueryParam"),
-  SessionParam("session", "io.jooby.annotation.SessionParam"),
+  SessionParam("session", BUILT_IN, "io.jooby.annotation.SessionParam"),
   BodyParam("body") {
     @Override
     public String parameterName(AnnotationMirror annotation, String defaultParameterName) {
@@ -368,6 +364,24 @@ public static ParameterGenerator findByAnnotation(String annotation) {
     this.annotations = Set.of(annotations);
   }
 
+  ParameterGenerator(String method, Set<String> typeRestrictions, String... annotations) {
+    this(method, annotations);
+    this.typeRestrictions = typeRestrictions;
+  }
+
+  public void verifyType(String type, String parameterName, MvcRoute route) {
+    if (!typeRestrictions.isEmpty()) {
+      if (typeRestrictions.stream().noneMatch(type::equals)) {
+        throw new IllegalArgumentException("""
+            Illegal argument type at '%s.%s()'.\s
+            Parameter '%s' annotated as %s cannot be of type '%s'.\s
+            Supported types are: %s
+            """.formatted(route.getRouter().getTargetType().toString(),
+            route.getMethodName(), parameterName, annotations, type, Types.BUILT_IN));
+      }
+    }
+  }
+
   protected String source(AnnotationMirror annotation) {
     if (ParameterGenerator.Lookup.annotations.contains(annotation.getAnnotationType().toString())) {
       var sources = findAnnotationValue(annotation, AnnotationSupport.VALUE);
@@ -382,40 +396,6 @@ protected String source(AnnotationMirror annotation) {
 
   protected final String method;
   private final Set<String> annotations;
+  private Set<String> typeRestrictions = Set.of(); // empty set means no restrictions by default
   private static final Set<Class> CONTAINER = Set.of(List.class, Set.class, Optional.class);
-  private static final Set<String> BUILT_IN =
-      Set.of(
-          String.class.getName(),
-          Boolean.class.getName(),
-          Boolean.TYPE.getName(),
-          Byte.class.getName(),
-          Byte.TYPE.getName(),
-          Character.class.getName(),
-          Character.TYPE.getName(),
-          Short.class.getName(),
-          Short.TYPE.getName(),
-          Integer.class.getName(),
-          Integer.TYPE.getName(),
-          Long.class.getName(),
-          Long.TYPE.getName(),
-          Float.class.getName(),
-          Float.TYPE.getName(),
-          Double.class.getName(),
-          Double.TYPE.getName(),
-          Enum.class.getName(),
-          java.util.UUID.class.getName(),
-          java.time.Instant.class.getName(),
-          java.util.Date.class.getName(),
-          java.time.LocalDate.class.getName(),
-          java.time.LocalDateTime.class.getName(),
-          java.math.BigDecimal.class.getName(),
-          java.math.BigInteger.class.getName(),
-          Duration.class.getName(),
-          Period.class.getName(),
-          java.nio.charset.Charset.class.getName(),
-          "io.jooby.StatusCode",
-          TimeZone.class.getName(),
-          ZoneId.class.getName(),
-          URI.class.getName(),
-          URL.class.getName());
 }
diff --git a/modules/jooby-apt/src/main/java/io/jooby/internal/apt/Types.java b/modules/jooby-apt/src/main/java/io/jooby/internal/apt/Types.java
@@ -0,0 +1,52 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package io.jooby.internal.apt;
+
+import java.net.URI;
+import java.net.URL;
+import java.time.Duration;
+import java.time.Period;
+import java.time.ZoneId;
+import java.util.Set;
+import java.util.TimeZone;
+
+class Types {
+  static final Set<String> BUILT_IN =
+      Set.of(
+          String.class.getName(),
+          Boolean.class.getName(),
+          Boolean.TYPE.getName(),
+          Byte.class.getName(),
+          Byte.TYPE.getName(),
+          Character.class.getName(),
+          Character.TYPE.getName(),
+          Short.class.getName(),
+          Short.TYPE.getName(),
+          Integer.class.getName(),
+          Integer.TYPE.getName(),
+          Long.class.getName(),
+          Long.TYPE.getName(),
+          Float.class.getName(),
+          Float.TYPE.getName(),
+          Double.class.getName(),
+          Double.TYPE.getName(),
+          Enum.class.getName(),
+          java.util.UUID.class.getName(),
+          java.time.Instant.class.getName(),
+          java.util.Date.class.getName(),
+          java.time.LocalDate.class.getName(),
+          java.time.LocalDateTime.class.getName(),
+          java.math.BigDecimal.class.getName(),
+          java.math.BigInteger.class.getName(),
+          Duration.class.getName(),
+          Period.class.getName(),
+          java.nio.charset.Charset.class.getName(),
+          "io.jooby.StatusCode",
+          TimeZone.class.getName(),
+          ZoneId.class.getName(),
+          URI.class.getName(),
+          URL.class.getName());
+}
diff --git a/modules/jooby-apt/src/test/java/tests/validation/BeanValidationsController.java b/modules/jooby-apt/src/test/java/tests/validation/BeanValidationsController.java
@@ -22,12 +22,6 @@ public Bean validateFormBean(@Valid @FormParam Bean bean) {
     return bean;
   }
 
-  // todo: revive when flash `toNullable` will be fixed
-  //    @POST("/validate/flash-bean")
-  //    public Bean validateFlashBean(@Valid @FlashParam Bean bean) {
-  //        return bean;
-  //    }
-
   @POST("/validate/bind-param-bean")
   public Bean validateBindParamBean(@Valid @BindParam Bean bean) {
     return bean;

diff --git a/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerCookie.java b/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerCookie.java
@@ -0,0 +1,15 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package tests.verifyarg;
+
+import io.jooby.annotation.CookieParam;
+import io.jooby.annotation.GET;
+
+class ControllerCookie {
+  @GET
+  public void param(@CookieParam Object param) {
+  }
+}
diff --git a/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerFlash.java b/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerFlash.java
@@ -0,0 +1,15 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package tests.verifyarg;
+
+import io.jooby.annotation.FlashParam;
+import io.jooby.annotation.GET;
+
+class ControllerFlash {
+  @GET
+  public void param(@FlashParam Object param) {
+  }
+}
diff --git a/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerFlashOpt.java b/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerFlashOpt.java
@@ -0,0 +1,17 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package tests.verifyarg;
+
+import io.jooby.annotation.FlashParam;
+import io.jooby.annotation.GET;
+
+import java.util.Optional;
+
+class ControllerFlashOpt {
+  @GET
+  public void param(@FlashParam Optional<Object> param) {
+  }
+}
diff --git a/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerHeader.java b/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerHeader.java
@@ -0,0 +1,15 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package tests.verifyarg;
+
+import io.jooby.annotation.GET;
+import jakarta.ws.rs.HeaderParam;
+
+class ControllerHeader {
+  @GET
+  public void param(@HeaderParam("test") Object param) {
+  }
+}
diff --git a/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerSession.java b/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerSession.java
@@ -0,0 +1,15 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package tests.verifyarg;
+
+import io.jooby.annotation.GET;
+import io.jooby.annotation.SessionParam;
+
+class ControllerSession {
+  @GET
+  public void param(@SessionParam Object param) {
+  }
+}
diff --git a/modules/jooby-apt/src/test/java/tests/verifyarg/VerifyArgTypeTest.java b/modules/jooby-apt/src/test/java/tests/verifyarg/VerifyArgTypeTest.java
@@ -0,0 +1,38 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package tests.verifyarg;
+
+import io.jooby.apt.ProcessorRunner;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class VerifyArgTypeTest {
+
+  @ParameterizedTest
+  @MethodSource("provideControllers")
+  public void compileController_illegalArgumentType_shouldThrowException(Object controller) {
+    RuntimeException ex = assertThrows(RuntimeException.class,
+        () -> new ProcessorRunner(controller));
+
+    assertTrue(ex.getMessage().contains("Illegal argument type at"));
+  }
+
+  private static List<Arguments> provideControllers() {
+    return List.of(
+        Arguments.of(new ControllerFlash()),
+        Arguments.of(new ControllerFlashOpt()),
+        Arguments.of(new ControllerCookie()),
+        Arguments.of(new ControllerSession()),
+        Arguments.of(new ControllerHeader())
+    );
+  }
+}
diff --git a/pom.xml b/pom.xml
@@ -1156,6 +1156,12 @@
         <version>${junit.version}</version>
       </dependency>
 
+      <dependency>
+        <groupId>org.junit.jupiter</groupId>
+        <artifactId>junit-jupiter-params</artifactId>
+        <version>${junit.version}</version>
+      </dependency>
+
       <dependency>
         <groupId>org.jacoco</groupId>
         <artifactId>org.jacoco.agent</artifactId>