From 8e3555b4d0e608be475fb592419a324e669fc84d Mon Sep 17 00:00:00 2001 From: Volodymyr Kliushnichenko Date: Tue, 17 Sep 2024 09:48:00 +0300 Subject: [PATCH 1/2] cleanup --- docs/asciidoc/context.adoc | 4 ++-- .../java/tests/validation/BeanValidationsController.java | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/docs/asciidoc/context.adoc b/docs/asciidoc/context.adoc index 6f4d6bf976..8b241fa7ed 100644 --- 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/src/test/java/tests/validation/BeanValidationsController.java b/modules/jooby-apt/src/test/java/tests/validation/BeanValidationsController.java index 5b936e04ac..2d719ea90d 100644 --- 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; From c68de34e770d4619fbaed56a8c360e43ab8a43b7 Mon Sep 17 00:00:00 2001 From: Volodymyr Kliushnichenko Date: Tue, 17 Sep 2024 22:23:47 +0300 Subject: [PATCH 2/2] add type verification for flash/session/cookie/header mvc params --- modules/jooby-apt/pom.xml | 6 ++ .../io/jooby/internal/apt/MvcParameter.java | 10 +-- .../internal/apt/ParameterGenerator.java | 68 +++++++------------ .../java/io/jooby/internal/apt/Types.java | 52 ++++++++++++++ .../tests/verifyarg/ControllerCookie.java | 15 ++++ .../java/tests/verifyarg/ControllerFlash.java | 15 ++++ .../tests/verifyarg/ControllerFlashOpt.java | 17 +++++ .../tests/verifyarg/ControllerHeader.java | 15 ++++ .../tests/verifyarg/ControllerSession.java | 15 ++++ .../tests/verifyarg/VerifyArgTypeTest.java | 38 +++++++++++ pom.xml | 6 ++ 11 files changed, 208 insertions(+), 49 deletions(-) create mode 100644 modules/jooby-apt/src/main/java/io/jooby/internal/apt/Types.java create mode 100644 modules/jooby-apt/src/test/java/tests/verifyarg/ControllerCookie.java create mode 100644 modules/jooby-apt/src/test/java/tests/verifyarg/ControllerFlash.java create mode 100644 modules/jooby-apt/src/test/java/tests/verifyarg/ControllerFlashOpt.java create mode 100644 modules/jooby-apt/src/test/java/tests/verifyarg/ControllerHeader.java create mode 100644 modules/jooby-apt/src/test/java/tests/verifyarg/ControllerSession.java create mode 100644 modules/jooby-apt/src/test/java/tests/verifyarg/VerifyArgTypeTest.java diff --git a/modules/jooby-apt/pom.xml b/modules/jooby-apt/pom.xml index b0ac22f4e6..ef3387259e 100644 --- a/modules/jooby-apt/pom.xml +++ b/modules/jooby-apt/pom.xml @@ -58,6 +58,12 @@ test + + org.junit.jupiter + junit-jupiter-params + test + + org.jacoco org.jacoco.agent 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 index 738cb3ea71..20cc987450 100644 --- 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 index 12e89567b2..e230ebf355 100644 --- 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 namePredicate() { @@ -65,7 +61,7 @@ protected Predicate 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 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 annotations; + private Set typeRestrictions = Set.of(); // empty set means no restrictions by default private static final Set CONTAINER = Set.of(List.class, Set.class, Optional.class); - private static final Set 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 new file mode 100644 index 0000000000..f653f32547 --- /dev/null +++ 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 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/verifyarg/ControllerCookie.java b/modules/jooby-apt/src/test/java/tests/verifyarg/ControllerCookie.java new file mode 100644 index 0000000000..78881ed0ee --- /dev/null +++ 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 new file mode 100644 index 0000000000..50e0dba14b --- /dev/null +++ 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 new file mode 100644 index 0000000000..89d40bb4a8 --- /dev/null +++ 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 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 new file mode 100644 index 0000000000..8ba9073027 --- /dev/null +++ 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 new file mode 100644 index 0000000000..60b0219a9e --- /dev/null +++ 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 new file mode 100644 index 0000000000..a6017af41a --- /dev/null +++ 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 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 index 03b557d10f..61c1b33517 100644 --- a/pom.xml +++ b/pom.xml @@ -1156,6 +1156,12 @@ ${junit.version} + + org.junit.jupiter + junit-jupiter-params + ${junit.version} + + org.jacoco org.jacoco.agent