Skip to content

Commit

Permalink
Reactive routes - fail if void method and no param can end the response
Browse files Browse the repository at this point in the history
- resolves #15470
  • Loading branch information
mkouba committed Mar 9, 2021
1 parent 7347d9b commit f43a974
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 17 deletions.
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 exampe {@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

0 comments on commit f43a974

Please sign in to comment.