#1548 - Delay request parameter variable lookup until necessary.
We pulled up the parameter name lookup for request parameters to avoid repeated lookups but unfortunately missed that the lookup will fail e.g. for @RequestParam Map<…> as we now strictly tried to find a request parameter name that doesn't actually exist in this case.
This commit is contained in:
@@ -42,7 +42,7 @@ import org.springframework.util.ConcurrentReferenceHashMap;
|
||||
*/
|
||||
public class MethodParameters {
|
||||
|
||||
private static final ParameterNameDiscoverer DISCOVERER = new DefaultParameterNameDiscoverer();
|
||||
private static ParameterNameDiscoverer DISCOVERER = new DefaultParameterNameDiscoverer();
|
||||
private static final Map<Method, MethodParameters> CACHE = new ConcurrentReferenceHashMap<>();
|
||||
|
||||
private final List<MethodParameter> parameters;
|
||||
|
||||
@@ -18,6 +18,7 @@ package org.springframework.hateoas.server.core;
|
||||
import java.lang.reflect.Method;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.function.Function;
|
||||
import java.util.stream.Collectors;
|
||||
@@ -96,6 +97,7 @@ public class SpringAffordanceBuilder {
|
||||
.orElse(ResolvableType.NONE);
|
||||
|
||||
List<QueryParameter> queryMethodParameters = parameters.getParametersWith(RequestParam.class).stream() //
|
||||
.filter(it -> !Map.class.isAssignableFrom(it.getParameterType()))
|
||||
.map(QueryParameter::of) //
|
||||
.collect(Collectors.toList());
|
||||
|
||||
|
||||
@@ -132,7 +132,10 @@ public class WebHandler {
|
||||
|
||||
bindRequestParameters(builder, parameter, arguments, conversionService);
|
||||
|
||||
if (SKIP_VALUE.equals(parameter.getVerifiedValue(arguments))) {
|
||||
boolean isSkipValue = SKIP_VALUE.equals(parameter.getVerifiedValue(arguments));
|
||||
boolean isMapParameter = Map.class.isAssignableFrom(parameter.parameter.getParameterType());
|
||||
|
||||
if (isSkipValue && !isMapParameter) {
|
||||
|
||||
values.put(parameter.getVariableName(), SKIP_VALUE);
|
||||
|
||||
@@ -186,7 +189,7 @@ public class WebHandler {
|
||||
return;
|
||||
}
|
||||
|
||||
String key = parameter.getVariableName();
|
||||
Class<?> parameterType = parameter.parameter.getParameterType();
|
||||
|
||||
if (value instanceof MultiValueMap) {
|
||||
|
||||
@@ -198,7 +201,11 @@ public class WebHandler {
|
||||
}
|
||||
}
|
||||
|
||||
} else if (value instanceof Map) {
|
||||
return;
|
||||
|
||||
}
|
||||
|
||||
if (value instanceof Map) {
|
||||
|
||||
Map<String, String> requestParams = (Map<String, String>) value;
|
||||
|
||||
@@ -206,14 +213,22 @@ public class WebHandler {
|
||||
builder.queryParam(requestParamEntry.getKey(), encodeParameter(requestParamEntry.getValue()));
|
||||
}
|
||||
|
||||
} else if (value instanceof Collection) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (Map.class.isAssignableFrom(parameterType) && SKIP_VALUE.equals(value)) {
|
||||
return;
|
||||
}
|
||||
|
||||
String key = parameter.getVariableName();
|
||||
|
||||
if (value instanceof Collection) {
|
||||
|
||||
for (Object element : (Collection<?>) value) {
|
||||
if (key != null) {
|
||||
builder.queryParam(key, encodeParameter(element));
|
||||
}
|
||||
}
|
||||
|
||||
} else if (SKIP_VALUE.equals(value)) {
|
||||
|
||||
if (parameter.isRequired()) {
|
||||
|
||||
@@ -20,8 +20,11 @@ import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.*;
|
||||
|
||||
import java.lang.reflect.Method;
|
||||
import java.util.Arrays;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.hateoas.IanaLinkRelations;
|
||||
@@ -29,8 +32,10 @@ import org.springframework.hateoas.Link;
|
||||
import org.springframework.hateoas.TemplateVariable;
|
||||
import org.springframework.hateoas.TemplateVariable.VariableType;
|
||||
import org.springframework.hateoas.TestUtils;
|
||||
import org.springframework.hateoas.server.core.MethodParameters;
|
||||
import org.springframework.http.HttpEntity;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.test.util.ReflectionTestUtils;
|
||||
import org.springframework.util.MultiValueMap;
|
||||
import org.springframework.web.bind.annotation.GetMapping;
|
||||
import org.springframework.web.bind.annotation.PathVariable;
|
||||
@@ -634,6 +639,27 @@ class WebMvcLinkBuilderUnitTest extends TestUtils {
|
||||
linkTo(methodOn(ControllerWithHandlerMethodParameterThatNeedsConversion.class).method(41L)).withSelfRel();
|
||||
}
|
||||
|
||||
@Test // #1548
|
||||
void mapsRequestParamMap() {
|
||||
|
||||
Object original = ReflectionTestUtils.getField(MethodParameters.class, "DISCOVERER");
|
||||
|
||||
try {
|
||||
|
||||
ReflectionTestUtils.setField(MethodParameters.class, "DISCOVERER", null);
|
||||
|
||||
Stream.of(null, new HashMap<String, String>()).forEach(it -> {
|
||||
|
||||
Link link = linkTo(methodOn(ControllerWithMethods.class).methodWithMapRequestParam(it)).withSelfRel();
|
||||
|
||||
assertThat(link.getHref()).endsWith("/with-map");
|
||||
});
|
||||
|
||||
} finally {
|
||||
ReflectionTestUtils.setField(MethodParameters.class, "DISCOVERER", original);
|
||||
}
|
||||
}
|
||||
|
||||
private static UriComponents toComponents(Link link) {
|
||||
return UriComponentsBuilder.fromUriString(link.expand().getHref()).build();
|
||||
}
|
||||
@@ -714,6 +740,11 @@ class WebMvcLinkBuilderUnitTest extends TestUtils {
|
||||
HttpEntity<Void> methodWithJdk8Optional(@RequestParam Optional<Integer> value) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@RequestMapping(path = "/with-map") // #1548
|
||||
HttpEntity<Void> methodWithMapRequestParam(@RequestParam Map<String, String> params) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
@RequestMapping("/parent")
|
||||
|
||||
Reference in New Issue
Block a user