Skip to content

Commit

Permalink
Add SpringRequestParamHandler to implement the required by default be…
Browse files Browse the repository at this point in the history
…haviour in @RequestParam parameters. Add and improve tests
  • Loading branch information
aureamunoz committed Jan 31, 2025
1 parent 01d9040 commit b9c599a
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import jakarta.ws.rs.Priorities;
Expand All @@ -25,6 +26,7 @@
import org.jboss.jandex.DotName;
import org.jboss.jandex.IndexView;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.MethodParameterInfo;
import org.jboss.jandex.Type;
import org.jboss.logging.Logger;
import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames;
Expand All @@ -49,13 +51,13 @@
import io.quarkus.spring.web.resteasy.reactive.runtime.ResponseEntityHandler;
import io.quarkus.spring.web.resteasy.reactive.runtime.ResponseStatusHandler;
import io.quarkus.spring.web.resteasy.reactive.runtime.SpringMultiValueMapParamExtractor;
import io.quarkus.spring.web.resteasy.reactive.runtime.SpringRequestParamHandler;
import io.quarkus.spring.web.runtime.common.ResponseStatusExceptionMapper;

public class SpringWebResteasyReactiveProcessor {

private static final Logger LOGGER = Logger.getLogger(SpringWebResteasyReactiveProcessor.class.getName());

public static final String NAME = SpringWebResteasyReactiveProcessor.class.getName();
private static final DotName REST_CONTROLLER_ANNOTATION = DotName
.createSimple("org.springframework.web.bind.annotation.RestController");

Expand Down Expand Up @@ -402,53 +404,66 @@ private String replaceSpringWebWildcards(String methodPath) {
@BuildStep
MethodScannerBuildItem scanner() {
return new MethodScannerBuildItem(new MethodScanner() {
// @Override
// public List<HandlerChainCustomizer> scan(MethodInfo method, ClassInfo actualEndpointClass,
// Map<String, Object> methodContext) {
// return Collections.emptyList();
// }
/**
* In Spring, parameters annotated with {@code @RequestParam} are required by default unless explicitly marked as
* optional.
* This method ensures the same behavior in Quarkus by checking if a parameter is required and enforcing its
* presence.
*
* The method scans for parameters annotated with {@code @RequestParam} and verifies:
* <ul>
* <li>If the parameter is marked as required (default behavior in Spring).</li>
* <li>If it has no default value.</li>
* <li>If it is not of type {@code Optional<T>}.</li>
* </ul>
*
* If all these conditions are met, it registers a {@link SpringRequestParamHandler} to enforce the required
* constraint.
*
* @param method The method being scanned.
* @param actualEndpointClass The actual class defining the endpoint.
* @param methodContext A map containing metadata about the method.
* @return A singleton list with {@link SpringRequestParamHandler} if the conditions are met,
* otherwise an empty list.
*/

@Override
public List<HandlerChainCustomizer> scan(MethodInfo method, ClassInfo actualEndpointClass,
Map<String, Object> methodContext) {
if (methodContext.containsKey("RequestParam")) {
for (MethodParameterInfo parameterInfo : method.parameters()) {
if (parameterInfo.annotation(REQUEST_PARAM) != null) {
AnnotationInstance annotation = parameterInfo.annotation(REQUEST_PARAM);
boolean required = annotation.value("required") == null || annotation.value("required").asBoolean();
String defaultValue = annotation.value("defaultValue") != null
? annotation.value("defaultValue").asString()
: "";
boolean isOptional = parameterInfo.type().name()
.equals(DotName.createSimple(Optional.class.getName()));

if (required && defaultValue.isBlank() && !isOptional) {
return Collections.singletonList(new SpringRequestParamHandler());
}
}
}
}
return Collections.emptyList();
}

@Override
public ParameterExtractor handleCustomParameter(Type paramType, Map<DotName, AnnotationInstance> annotations,
boolean field, Map<String, Object> methodContext) {
if (annotations.containsKey(REQUEST_PARAM)) {
methodContext.put("RequestParam", true);
}
if (paramType.name().equals(SPRING_MULTIVALUE_MAP)) {
methodContext.put(SPRING_MULTIVALUE_MAP.toString(), true);
return new SpringMultiValueMapParamExtractor();
}
return null;
}

// @Override
// public boolean isMethodSignatureAsync(MethodInfo info) {
// for (var param : info.parameterTypes()) {
// if (param.name().equals(SPRING_MULTIVALUE_MAP)) {
// return true;
// }
// }
// return false;
// }
});
}

// // @BuildStep
// public void transformSpringParameters(CombinedIndexBuildItem index,
// BuildProducer<GeneratedClassBuildItem> generatedClasses) {
// for (ClassInfo classInfo : index.getIndex().getKnownClasses()) {
// for (MethodInfo method : classInfo.methods()) {
// for (Type paramType : method.parameterTypes()) {
// if (paramType.name().toString().equals("org.springframework.util.MultiValueMap")) {
// Type newType = Type.create(DotName.createSimple("jakarta.ws.rs.core.MultivaluedMap"), paramType.kind());
// transformParameterType(method, paramType, newType);
// }
// }
// }
// }
// }
//
// private void transformParameterType(MethodInfo method, Type oldType, Type newType) {
// //rewrite the method using the jakarta multivalued type
// }

@BuildStep
public MethodScannerBuildItem responseEntitySupport() {
return new MethodScannerBuildItem(new MethodScanner() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package io.quarkus.spring.web.resteasy.reactive.runtime;

import java.util.Collections;
import java.util.List;
import java.util.Map;

import org.jboss.resteasy.reactive.common.jaxrs.ResponseImpl;
import org.jboss.resteasy.reactive.common.model.ResourceClass;
import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;
import org.jboss.resteasy.reactive.server.model.HandlerChainCustomizer;
import org.jboss.resteasy.reactive.server.model.ServerResourceMethod;
import org.jboss.resteasy.reactive.server.spi.ServerRestHandler;

/**
* In Spring, parameters annotated with {@code @RequestParam} are required by default unless explicitly marked as
* optional.
* This {@link SpringRequestParamHandler} enforces the required constraint responding with a BAD_REQUEST status.
*
*/
public class SpringRequestParamHandler implements HandlerChainCustomizer {
@Override
public List<ServerRestHandler> handlers(HandlerChainCustomizer.Phase phase, ResourceClass resourceClass,
ServerResourceMethod resourceMethod) {
if (phase == Phase.AFTER_RESPONSE_CREATED) {
return Collections.singletonList(new ServerRestHandler() {
@Override
public void handle(ResteasyReactiveRequestContext requestContext) throws Exception {
Map<String, List<String>> parametersMap = requestContext.serverRequest().getParametersMap();
if (parametersMap.isEmpty()) {
ResponseImpl response = (ResponseImpl) requestContext.getResponse().get();
response.setStatus(400);
}
}
});
}
return Collections.emptyList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,33 @@ public class RequestParamControllerTest {
.addClasses(RequestParamController.class));

@Test
public void whenInvokeGetQueryStringThenTheOriginQueryStringIsReturned() throws Exception {
public void testSimpleMapping() throws Exception {
when().get("/api/foos?id=abc")
.then()
.statusCode(200)
.body(is("ID: abc"));

// should return 400 because, in Spring, method parameters annotated with @RequestParam are required by default.
// see SpringWebResteasyReactiveProcessor L298
// In Spring, method parameters annotated with @RequestParam are required by default.
when().get("/api/foos")
.then()
.statusCode(200);
.statusCode(400);
}

@Test
public void whenConfigureParamToBeOptionalThenTheGetQueryWorksWithAndWithoutRequestParam() throws Exception {
public void testSimpleMappingSpecifyingName() throws Exception {
when().post("/api/foos?id=abc&name=bar")
.then()
.statusCode(200)
.body(is("ID: abc Name: bar"));

when().post("/api/foos")
.then()
.statusCode(400);

}

@Test
public void testNotRequiredParam() throws Exception {
when().get("/api/foos/notParamRequired?id=abc")
.then()
.statusCode(200)
Expand All @@ -44,7 +56,7 @@ public void whenConfigureParamToBeOptionalThenTheGetQueryWorksWithAndWithoutRequ
}

@Test
public void whenWrapingParamInOptionalThenTheGetQueryWorksWithAndWithoutRequestParam() throws Exception {
public void testOptionalParam() throws Exception {
when().get("/api/foos/optional?id=abc")
.then()
.statusCode(200)
Expand All @@ -57,7 +69,7 @@ public void whenWrapingParamInOptionalThenTheGetQueryWorksWithAndWithoutRequestP
}

@Test
public void whenDefaultValueProvidedThenItIsReturnedIfRequestParamIsAbsent() throws Exception {
public void testDefaultValueForParam() throws Exception {
when().get("/api/foos/defaultValue?id=abc")
.then()
.statusCode(200)
Expand All @@ -70,12 +82,16 @@ public void whenDefaultValueProvidedThenItIsReturnedIfRequestParamIsAbsent() thr
}

@Test
public void whenInvokePostQueryWithSpecificParamNameThenTheOriginQueryStringIsReturned() throws Exception {
public void testMultipleMapping() throws Exception {
when().post("/api/foos/map?id=abc&name=bar")
.then()
.statusCode(200)
.body(containsString("Parameters are [name=[bar], id=[abc]]"));

when().post("/api/foos/map")
.then()
.statusCode(400);

}

@Test
Expand All @@ -84,15 +100,22 @@ public void testMultivalue() throws Exception {
.then()
.statusCode(200)
.body(containsString("IDs are [1,2,3]"));

when().get("/api/foos/multivalue")
.then()
.statusCode(400);
}

@Test
public void whenInvokePostQueryWithMultiMapParamNameThenTheOriginQueryStringIsReturned() throws Exception {
public void testMultiMap() throws Exception {
when().post("/api/foos/multiMap?id=abc&id=123")
.then()
.statusCode(200)
.body(containsString("Parameters are id=abc, 123"));

when().post("/api/foos/multiMap")
.then()
.statusCode(400);
}

}

0 comments on commit b9c599a

Please sign in to comment.