Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup validation tests #3534

Merged
merged 2 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/asciidoc/context.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just aligning the doc with what we have at the moment. There is no ctx.flash(str,str) method

ctx.flash().put("success", "Item created"); <1>
return ctx.sendRedirect("/"); <2>
});
----
Expand All @@ -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>
}
----
Expand Down
6 changes: 6 additions & 0 deletions modules/jooby-apt/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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());
}
52 changes: 52 additions & 0 deletions modules/jooby-apt/src/main/java/io/jooby/internal/apt/Types.java
Original file line number Diff line number Diff line change
@@ -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());
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ public Bean validateFormBean(@Valid @FormParam Bean bean) {
return bean;
}

// todo: revive when flash `toNullable` will be fixed
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant here is that it is possible to define the param as object @FlashParam Bean bean
and apt will generate the code that is not executable, since toNullable is not there for flash/session/...
image

The use-case a bit synthetic, but still, I can add some type checking if you think it makes sense

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add the type check

// @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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
}
}
Original file line number Diff line number Diff line change
@@ -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) {
}
}
Original file line number Diff line number Diff line change
@@ -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) {
}
}
Original file line number Diff line number Diff line change
@@ -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) {
}
}
Original file line number Diff line number Diff line change
@@ -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) {
}
}
Original file line number Diff line number Diff line change
@@ -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())
);
}
}
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down