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

Reactive routes - fail if void method and no param can end the response #15572

Merged
merged 1 commit into from
Mar 9, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ private void validateRouteMethod(BeanInfo bean, MethodInfo method,
"Route business method returning a Uni/Multi must have a generic parameter [method: %s, bean: %s]",
method, bean));
}
boolean canEndResponse = false;
int idx = 0;
int failureParams = 0;
for (Type paramType : params) {
Expand Down Expand Up @@ -507,12 +508,22 @@ private void validateRouteMethod(BeanInfo bean, MethodInfo method,
// A param injector may validate the parameter annotations
injector.validate(bean, method, routeAnnotation, paramType, paramAnnotations);

if (injector.canEndResponse) {
canEndResponse = true;
}

if (Route.HandlerType.FAILURE == handlerType && isThrowable(paramType, index)) {
failureParams++;
}
idx++;
}

if (method.returnType().kind() == Kind.VOID && !canEndResponse) {
throw new IllegalStateException(String.format(
"Route method that returns void must accept at least one parameter that can end the response [method: %s, bean: %s]",
method, bean));
}

if (failureParams > 1) {
throw new IllegalStateException(String.format(
"A failure handler may only define one failure parameter - route method %s declared on %s",
Expand Down Expand Up @@ -1131,17 +1142,18 @@ private List<ParameterInjector> getMatchingInjectors(Type paramType, Set<Annotat
static List<ParameterInjector> initParamInjectors() {
List<ParameterInjector> injectors = new ArrayList<>();

injectors.add(ParameterInjector.builder().matchType(io.quarkus.vertx.web.deployment.DotNames.ROUTING_CONTEXT)
.resultHandleProvider(new ResultHandleProvider() {
@Override
public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstance> annotations,
ResultHandle routingContext, MethodCreator invoke, int position,
BuildProducer<ReflectiveHierarchyBuildItem> reflectiveHierarchy) {
return routingContext;
}
}).build());
injectors.add(
ParameterInjector.builder().canEndResponse().matchType(io.quarkus.vertx.web.deployment.DotNames.ROUTING_CONTEXT)
.resultHandleProvider(new ResultHandleProvider() {
@Override
public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstance> annotations,
ResultHandle routingContext, MethodCreator invoke, int position,
BuildProducer<ReflectiveHierarchyBuildItem> reflectiveHierarchy) {
return routingContext;
}
}).build());

injectors.add(ParameterInjector.builder().matchType(DotNames.ROUTING_EXCHANGE)
injectors.add(ParameterInjector.builder().canEndResponse().matchType(DotNames.ROUTING_EXCHANGE)
.resultHandleProvider(new ResultHandleProvider() {
@Override
public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstance> annotations,
Expand All @@ -1154,7 +1166,7 @@ public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstanc
}
}).build());

injectors.add(ParameterInjector.builder().matchType(DotNames.HTTP_SERVER_REQUEST)
injectors.add(ParameterInjector.builder().canEndResponse().matchType(DotNames.HTTP_SERVER_REQUEST)
.resultHandleProvider(new ResultHandleProvider() {
@Override
public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstance> annotations,
Expand All @@ -1166,7 +1178,7 @@ public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstanc
}
}).build());

injectors.add(ParameterInjector.builder().matchType(DotNames.HTTP_SERVER_RESPONSE)
injectors.add(ParameterInjector.builder().canEndResponse().matchType(DotNames.HTTP_SERVER_RESPONSE)
.resultHandleProvider(new ResultHandleProvider() {
@Override
public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstance> annotations,
Expand All @@ -1178,7 +1190,7 @@ public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstanc
}
}).build());

injectors.add(ParameterInjector.builder().matchType(DotNames.MUTINY_HTTP_SERVER_REQUEST)
injectors.add(ParameterInjector.builder().canEndResponse().matchType(DotNames.MUTINY_HTTP_SERVER_REQUEST)
.resultHandleProvider(new ResultHandleProvider() {
@Override
public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstance> annotations,
Expand All @@ -1195,7 +1207,7 @@ public ResultHandle get(MethodInfo method, Type paramType, Set<AnnotationInstanc
}).build());

injectors
.add(ParameterInjector.builder().matchType(DotNames.MUTINY_HTTP_SERVER_RESPONSE)
.add(ParameterInjector.builder().canEndResponse().matchType(DotNames.MUTINY_HTTP_SERVER_RESPONSE)
.resultHandleProvider(new ResultHandleProvider() {
@Override
public ResultHandle get(MethodInfo method, Type paramType,
Expand Down Expand Up @@ -1429,6 +1441,7 @@ static Builder builder() {
final ResultHandleProvider provider;
final Route.HandlerType targetHandlerType;
final ParamValidator validator;
final boolean canEndResponse;

ParameterInjector(ParameterInjector.Builder builder) {
if (builder.predicate != null) {
Expand Down Expand Up @@ -1494,6 +1507,7 @@ public boolean test(Type paramType, Set<AnnotationInstance> paramAnnotations, In
this.provider = builder.provider;
this.targetHandlerType = builder.targetHandlerType;
this.validator = builder.validator;
this.canEndResponse = builder.canEndResponse;
}

boolean matches(Type paramType, Set<AnnotationInstance> paramAnnotations, IndexView index) {
Expand Down Expand Up @@ -1527,6 +1541,7 @@ static class Builder {
ResultHandleProvider provider;
Route.HandlerType targetHandlerType;
ParamValidator validator;
boolean canEndResponse;

Builder matchType(DotName className) {
return matchType(Type.create(className, Kind.CLASS));
Expand Down Expand Up @@ -1609,6 +1624,11 @@ Builder validate(ParamValidator validator) {
return this;
}

Builder canEndResponse() {
this.canEndResponse = true;
return this;
}

ParameterInjector build() {
return new ParameterInjector(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void test() {
public static class Routes {

@Route
void fail(@Param String type) {
String fail(@Param String type) {
throw new RuntimeException("Unknown!");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void test() {
public static class Routes {

@Route
void fail(@Param String type) {
String fail(@Param String type) {
if ("unsupported".equals(type)) {
throw new UnsupportedOperationException("Unsupported!");
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.quarkus.vertx.web.params;

import static org.junit.jupiter.api.Assertions.fail;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.vertx.web.Param;
import io.quarkus.vertx.web.Route;

public class VoidInvalidParameterTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(SimpleBean.class))
.setExpectedException(IllegalStateException.class);

@Test
public void testValidationFailed() {
fail();
}

static class SimpleBean {

@Route
void hello(@Param("myParam") String param) {
// This route is illegal - it's not possible to end the response
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@
* </code>
* </pre>
*
* If the annotated method returns {@code void} then it has to accept at least one argument.
* If the annotated method returns {@code void} then it has to accept at least one argument that makes it possible to end the
* response, for example {@link RoutingContext}.
* If the annotated method does not return {@code void} then the arguments are optional.
* <p>
* If both {@link #path()} and {@link #regex()} are set the regular expression is used for matching.
Expand Down