root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Resolve property-dependent parameter names for exception messages 

Prior to this commit when a required parameter defined as a property or
expression placeholder was missing, the exception thrown would refer to
the placeholder instead of the resolved name.

This change covers messaging handlers and web controllers, both blocking
and reactive. It also fixes the error message when handling null values
for non-required parameters, as well as in cases that need conversion.

See gh-32323
Closes gh-32462
This commit is contained in:
Andrea Mauro 2024-03-09 10:45:01 +01:00 committed by Simon Baslé
parent 5698191ba0
commit 458c30cb63
9 changed files with 234 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/AbstractNamedValueMethodArgumentResolver.java
View File

@ -97,9 +97,9 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements SyncHa
arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
}
else if (namedValueInfo.required && !nestedParameter.isOptional()) {
handleMissingValue(namedValueInfo.name, nestedParameter, message);
handleMissingValue(resolvedName.toString(), nestedParameter, message);
}
arg = handleNullValue(namedValueInfo.name, arg, nestedParameter.getNestedParameterType());
arg = handleNullValue(resolvedName.toString(), arg, nestedParameter.getNestedParameterType());
}
else if ("".equals(arg) && namedValueInfo.defaultValue != null) {
arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
@ -113,7 +113,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements SyncHa
arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
}
else if (namedValueInfo.required && !nestedParameter.isOptional()) {
handleMissingValue(namedValueInfo.name, nestedParameter, message);
handleMissingValue(resolvedName.toString(), nestedParameter, message);
}
}
}

6
spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java
View File

@ -105,9 +105,9 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle
arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
}
else if (namedValueInfo.required && !nestedParameter.isOptional()) {
handleMissingValue(namedValueInfo.name, nestedParameter, message);
handleMissingValue(resolvedName.toString(), nestedParameter, message);
}
arg = handleNullValue(namedValueInfo.name, arg, nestedParameter.getNestedParameterType());
arg = handleNullValue(resolvedName.toString(), arg, nestedParameter.getNestedParameterType());
}
else if ("".equals(arg) && namedValueInfo.defaultValue != null) {
arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
@ -121,7 +121,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle
arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
}
else if (namedValueInfo.required && !nestedParameter.isOptional()) {
handleMissingValue(namedValueInfo.name, nestedParameter, message);
handleMissingValue(resolvedName.toString(), nestedParameter, message);
}
}
}

35
spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/reactive/HeaderMethodArgumentResolverTests.java
View File

@ -36,6 +36,7 @@ import org.springframework.messaging.support.NativeMessageHeaderAccessor;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.springframework.messaging.handler.annotation.MessagingPredicates.header;
import static org.springframework.messaging.handler.annotation.MessagingPredicates.headerPlain;
@ -151,6 +152,37 @@ class HeaderMethodArgumentResolverTests {
assertThat(result).isEqualTo(Optional.empty());
}
@Test
void missingParameterFromSystemPropertyThroughPlaceholder() {
String expected = "sysbar";
System.setProperty("systemProperty", expected);
Message<byte[]> message = MessageBuilder.withPayload(new byte[0]).build();
MethodParameter param = this.resolvable.annot(header("#{systemProperties.systemProperty}")).arg();
assertThatThrownBy(() ->
resolveArgument(param, message))
.isInstanceOf(MessageHandlingException.class)
.hasMessageContaining(expected);
System.clearProperty("systemProperty");
}
@Test
void notNullablePrimitiveParameterFromSystemPropertyThroughPlaceholder() {
String expected = "sysbar";
System.setProperty("systemProperty", expected);
Message<byte[]> message = MessageBuilder.withPayload(new byte[0]).build();
MethodParameter param = this.resolvable.annot(header("${systemProperty}").required(false)).arg();
assertThatThrownBy(() ->
resolver.resolveArgument(param, message))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining(expected);
System.clearProperty("systemProperty");
}
@SuppressWarnings({"unchecked", "ConstantConditions"})
private <T> T resolveArgument(MethodParameter param, Message<?> message) {
return (T) this.resolver.resolveArgument(param, message).block(Duration.ofSeconds(5));
@ -165,7 +197,8 @@ class HeaderMethodArgumentResolverTests {
@Header(name = "#{systemProperties.systemProperty}") String param4,
String param5,
@Header("foo") Optional<String> param6,
@Header("nativeHeaders.param1") String nativeHeaderParam1) {
@Header("nativeHeaders.param1") String nativeHeaderParam1,
@Header(name = "${systemProperty}", required = false) int primitivePlaceholderParam) {
}

34
spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/HeaderMethodArgumentResolverTests.java
View File

@ -35,6 +35,7 @@ import org.springframework.messaging.support.NativeMessageHeaderAccessor;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.springframework.messaging.handler.annotation.MessagingPredicates.header;
import static org.springframework.messaging.handler.annotation.MessagingPredicates.headerPlain;
@ -137,6 +138,36 @@ class HeaderMethodArgumentResolverTests {
}
}
@Test
void missingParameterFromSystemPropertyThroughPlaceholder() {
String expected = "sysbar";
System.setProperty("systemProperty", expected);
Message<byte[]> message = MessageBuilder.withPayload(new byte[0]).build();
MethodParameter param = this.resolvable.annot(header("#{systemProperties.systemProperty}")).arg();
assertThatThrownBy(() ->
resolver.resolveArgument(param, message))
.isInstanceOf(MessageHandlingException.class)
.hasMessageContaining(expected);
System.clearProperty("systemProperty");
}
@Test
void notNullablePrimitiveParameterFromSystemPropertyThroughPlaceholder() {
String expected = "sysbar";
System.setProperty("systemProperty", expected);
Message<byte[]> message = MessageBuilder.withPayload(new byte[0]).build();
MethodParameter param = this.resolvable.annot(header("${systemProperty}").required(false)).arg();
assertThatThrownBy(() ->
resolver.resolveArgument(param, message))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining(expected);
System.clearProperty("systemProperty");
}
@Test
void resolveOptionalHeaderWithValue() throws Exception {
Message<String> message = MessageBuilder.withPayload("foo").setHeader("foo", "bar").build();
@ -162,7 +193,8 @@ class HeaderMethodArgumentResolverTests {
@Header(name = "#{systemProperties.systemProperty}") String param4,
String param5,
@Header("foo") Optional<String> param6,
@Header("nativeHeaders.param1") String nativeHeaderParam1) {
@Header("nativeHeaders.param1") String nativeHeaderParam1,
@Header(name = "${systemProperty}", required = false) int primitivePlaceholderParam) {
}

6
spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java
View File

@ -123,10 +123,10 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle
arg = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
}
else if (namedValueInfo.required && !nestedParameter.isOptional()) {
handleMissingValue(namedValueInfo.name, nestedParameter, webRequest);
handleMissingValue(resolvedName.toString(), nestedParameter, webRequest);
}
if (!hasDefaultValue) {
arg = handleNullValue(namedValueInfo.name, arg, nestedParameter.getNestedParameterType());
arg = handleNullValue(resolvedName.toString(), arg, nestedParameter.getNestedParameterType());
}
}
else if ("".equals(arg) && namedValueInfo.defaultValue != null) {
@ -142,7 +142,7 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle
arg = convertIfNecessary(parameter, webRequest, binderFactory, namedValueInfo, arg);
}
else if (namedValueInfo.required && !nestedParameter.isOptional()) {
handleMissingValueAfterConversion(namedValueInfo.name, nestedParameter, webRequest);
handleMissingValueAfterConversion(resolvedName.toString(), nestedParameter, webRequest);
}
}
}

37
spring-web/src/test/java/org/springframework/web/method/annotation/RequestHeaderMethodArgumentResolverTests.java
View File

@ -69,6 +69,7 @@ class RequestHeaderMethodArgumentResolverTests {
private MethodParameter paramInstant;
private MethodParameter paramUuid;
private MethodParameter paramUuidOptional;
private MethodParameter paramUuidPlaceholder;
private MockHttpServletRequest servletRequest;
@ -93,6 +94,7 @@ class RequestHeaderMethodArgumentResolverTests {
paramInstant = new SynthesizingMethodParameter(method, 8);
paramUuid = new SynthesizingMethodParameter(method, 9);
paramUuidOptional = new SynthesizingMethodParameter(method, 10);
paramUuidPlaceholder = new SynthesizingMethodParameter(method, 11);
servletRequest = new MockHttpServletRequest();
webRequest = new ServletWebRequest(servletRequest, new MockHttpServletResponse());
@ -186,6 +188,20 @@ class RequestHeaderMethodArgumentResolverTests {
}
}
@Test
void missingParameterFromSystemPropertyThroughPlaceholder() {
String expected = "bar";
System.setProperty("systemProperty", expected);
assertThatThrownBy(() ->
resolver.resolveArgument(paramResolvedNameWithPlaceholder, null, webRequest, null))
.isInstanceOf(MissingRequestHeaderException.class)
.extracting("headerName").isEqualTo(expected);
System.clearProperty("systemProperty");
}
@Test
void resolveDefaultValueFromRequest() throws Exception {
servletRequest.setContextPath("/bar");
@ -296,6 +312,24 @@ class RequestHeaderMethodArgumentResolverTests {
assertThat(result).isNull();
}
@Test
public void uuidPlaceholderConversionWithEmptyValue() {
String expected = "name";
servletRequest.addHeader(expected, "");
System.setProperty("systemProperty", expected);
ConfigurableWebBindingInitializer bindingInitializer = new ConfigurableWebBindingInitializer();
bindingInitializer.setConversionService(new DefaultFormattingConversionService());
assertThatThrownBy(() ->
resolver.resolveArgument(paramUuidPlaceholder, null, webRequest,
new DefaultDataBinderFactory(bindingInitializer)))
.isInstanceOf(MissingRequestHeaderException.class)
.extracting("headerName").isEqualTo(expected);
System.clearProperty("systemProperty");
}
void params(
@RequestHeader(name = "name", defaultValue = "bar") String param1,
@ -308,7 +342,8 @@ class RequestHeaderMethodArgumentResolverTests {
@RequestHeader("name") Date dateParam,
@RequestHeader("name") Instant instantParam,
@RequestHeader("name") UUID uuid,
@RequestHeader(name = "name", required = false) UUID uuidOptional) {
@RequestHeader(name = "name", required = false) UUID uuidOptional,
@RequestHeader(name = "${systemProperty}") UUID uuidPlaceholder) {
}
}

74
spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java
View File

@ -23,6 +23,7 @@ import java.util.Optional;
import jakarta.servlet.http.Part;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.propertyeditors.StringTrimmerEditor;
@ -37,7 +38,9 @@ import org.springframework.web.bind.support.DefaultDataBinderFactory;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.bind.support.WebRequestDataBinder;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.context.support.GenericWebApplicationContext;
import org.springframework.web.multipart.MultipartException;
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.support.MissingServletRequestPartException;
@ -50,6 +53,7 @@ import org.springframework.web.testfixture.servlet.MockPart;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.springframework.web.testfixture.method.MvcAnnotationPredicates.requestParam;
@ -64,7 +68,7 @@ import static org.springframework.web.testfixture.method.MvcAnnotationPredicates
*/
class RequestParamMethodArgumentResolverTests {
private RequestParamMethodArgumentResolver resolver = new RequestParamMethodArgumentResolver(null, true);
private RequestParamMethodArgumentResolver resolver;
private MockHttpServletRequest request = new MockHttpServletRequest();
@ -72,6 +76,15 @@ class RequestParamMethodArgumentResolverTests {
private ResolvableMethod testMethod = ResolvableMethod.on(getClass()).named("handle").build();
@BeforeEach
void setup() {
GenericWebApplicationContext context = new GenericWebApplicationContext();
context.refresh();
resolver = new RequestParamMethodArgumentResolver(context.getBeanFactory(), true);
// Expose request to the current thread (for SpEL expressions)
RequestContextHolder.setRequestAttributes(webRequest);
}
@Test
void supportsParameter() {
@ -141,6 +154,12 @@ class RequestParamMethodArgumentResolverTests {
param = this.testMethod.annotPresent(RequestPart.class).arg(MultipartFile.class);
assertThat(resolver.supportsParameter(param)).isFalse();
param = this.testMethod.annotPresent(RequestParam.class).arg(Integer.class);
assertThat(resolver.supportsParameter(param)).isTrue();
param = this.testMethod.annotPresent(RequestParam.class).arg(int.class);
assertThat(resolver.supportsParameter(param)).isTrue();
}
@Test
@ -678,6 +697,55 @@ class RequestParamMethodArgumentResolverTests {
assertThat(actual).isEqualTo(Optional.empty());
}
@Test
void resolveNameFromSystemPropertyThroughPlaceholder() throws Exception {
ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
initializer.setConversionService(new DefaultConversionService());
WebDataBinderFactory binderFactory = new DefaultDataBinderFactory(initializer);
Integer expected = 100;
request.addParameter("name", expected.toString());
System.setProperty("systemProperty", "name");
try {
MethodParameter param = this.testMethod.annot(requestParam().name("${systemProperty}")).arg(Integer.class);
Object result = resolver.resolveArgument(param, null, webRequest, binderFactory);
boolean condition = result instanceof Integer;
assertThat(condition).isTrue();
}
finally {
System.clearProperty("systemProperty");
}
}
@Test
void missingParameterFromSystemPropertyThroughPlaceholder() {
String expected = "name";
System.setProperty("systemProperty", expected);
MethodParameter param = this.testMethod.annot(requestParam().name("${systemProperty}")).arg(Integer.class);
assertThatThrownBy(() ->
resolver.resolveArgument(param, null, webRequest, null))
.isInstanceOf(MissingServletRequestParameterException.class)
.extracting("parameterName").isEqualTo(expected);
System.clearProperty("systemProperty");
}
@Test
void notNullablePrimitiveParameterFromSystemPropertyThroughPlaceholder() {
String expected = "sysbar";
System.setProperty("systemProperty", expected);
MethodParameter param = this.testMethod.annot(requestParam().name("${systemProperty}").notRequired()).arg(int.class);
assertThatThrownBy(() ->
resolver.resolveArgument(param, null, webRequest, null))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining(expected);
System.clearProperty("systemProperty");
}
@SuppressWarnings({"unused", "OptionalUsedAsFieldOrParameterType"})
public void handle(
@ -702,7 +770,9 @@ class RequestParamMethodArgumentResolverTests {
@RequestParam("name") Optional<Integer[]> paramOptionalArray,
@RequestParam("name") Optional<List<?>> paramOptionalList,
@RequestParam("mfile") Optional<MultipartFile> multipartFileOptional,
@RequestParam(defaultValue = "false") Boolean booleanParam) {
@RequestParam(defaultValue = "false") Boolean booleanParam,
@RequestParam("${systemProperty}") Integer placeholderParam,
@RequestParam(name = "${systemProperty}", required = false) int primitivePlaceholderParam) {
}
}

8
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java
View File

@ -120,7 +120,7 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr
return Mono.justOrEmpty(arg);
})
.switchIfEmpty(getDefaultValue(
namedValueInfo, parameter, bindingContext, model, exchange));
namedValueInfo, resolvedName.toString(), parameter, bindingContext, model, exchange));
}
/**
@ -222,7 +222,7 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr
/**
* Resolve the default value, if any.
*/
private Mono<Object> getDefaultValue(NamedValueInfo namedValueInfo, MethodParameter parameter,
private Mono<Object> getDefaultValue(NamedValueInfo namedValueInfo, String resolvedName, MethodParameter parameter,
BindingContext bindingContext, Model model, ServerWebExchange exchange) {
return Mono.fromSupplier(() -> {
@ -234,10 +234,10 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr
value = resolveEmbeddedValuesAndExpressions(namedValueInfo.defaultValue);
}
else if (namedValueInfo.required && !parameter.isOptional()) {
handleMissingValue(namedValueInfo.name, parameter, exchange);
handleMissingValue(resolvedName, parameter, exchange);
}
if (!hasDefaultValue) {
value = handleNullValue(namedValueInfo.name, value, parameter.getNestedParameterType());
value = handleNullValue(resolvedName, value, parameter.getNestedParameterType());
}
if (value != null || !hasDefaultValue) {
value = applyConversion(value, namedValueInfo, parameter, bindingContext, exchange);

47
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestHeaderMethodArgumentResolverTests.java
View File

@ -36,6 +36,7 @@ import org.springframework.util.ReflectionUtils;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
import org.springframework.web.reactive.BindingContext;
import org.springframework.web.server.MissingRequestValueException;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
@ -43,6 +44,7 @@ import org.springframework.web.testfixture.server.MockServerWebExchange;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/**
* Tests for {@link RequestHeaderMethodArgumentResolver}.
@ -64,6 +66,7 @@ class RequestHeaderMethodArgumentResolverTests {
private MethodParameter paramDate;
private MethodParameter paramInstant;
private MethodParameter paramMono;
private MethodParameter primitivePlaceholderParam;
@BeforeEach
@ -87,6 +90,7 @@ class RequestHeaderMethodArgumentResolverTests {
this.paramDate = new SynthesizingMethodParameter(method, 6);
this.paramInstant = new SynthesizingMethodParameter(method, 7);
this.paramMono = new SynthesizingMethodParameter(method, 8);
this.primitivePlaceholderParam = new SynthesizingMethodParameter(method, 9);
}
@ -200,6 +204,46 @@ class RequestHeaderMethodArgumentResolverTests {
}
}
@Test
void missingParameterFromSystemPropertyThroughPlaceholder() {
String expected = "sysbar";
MockServerHttpRequest request = MockServerHttpRequest.get("/").build();
ServerWebExchange exchange = MockServerWebExchange.from(request);
System.setProperty("systemProperty", expected);
try {
Mono<Object> mono = this.resolver.resolveArgument(
this.paramResolvedNameWithExpression, this.bindingContext, exchange);
assertThatThrownBy(() -> mono.block())
.isInstanceOf(MissingRequestValueException.class)
.extracting("name").isEqualTo(expected);
}
finally {
System.clearProperty("systemProperty");
}
}
@Test
void notNullablePrimitiveParameterFromSystemPropertyThroughPlaceholder() {
String expected = "sysbar";
MockServerHttpRequest request = MockServerHttpRequest.get("/").build();
ServerWebExchange exchange = MockServerWebExchange.from(request);
System.setProperty("systemProperty", expected);
try {
Mono<Object> mono = this.resolver.resolveArgument(
this.primitivePlaceholderParam, this.bindingContext, exchange);
assertThatThrownBy(() -> mono.block())
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining(expected);
}
finally {
System.clearProperty("systemProperty");
}
}
@Test
void notFound() {
Mono<Object> mono = resolver.resolveArgument(
@ -252,7 +296,8 @@ class RequestHeaderMethodArgumentResolverTests {
@RequestHeader("name") Map<?, ?> unsupported,
@RequestHeader("name") Date dateParam,
@RequestHeader("name") Instant instantParam,
@RequestHeader Mono<String> alsoNotSupported) {
@RequestHeader Mono<String> alsoNotSupported,
@RequestHeader(value = "${systemProperty}", required = false) int primitivePlaceholderParam) {
}
}