Skip to content

Commit

Permalink
Propagate resolvedName to getDefaultValue and pass it to handleMissin…
Browse files Browse the repository at this point in the history
…gValue and handleNullValue to improve error message

Prior to this commit when an error occurred related to a property or expression placeholder, the exception thrown would refer to the placeholder instead of the resolved name.

See gh-32323
  • Loading branch information
andrea-mauro committed Mar 18, 2024
1 parent 45485d8 commit 9fb1c34
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public Mono<Object> resolveArgument(
return Mono.justOrEmpty(arg);
})
.switchIfEmpty(getDefaultValue(
namedValueInfo, parameter, bindingContext, model, exchange));
namedValueInfo, resolvedName.toString(), parameter, bindingContext, model, exchange));
}

/**
Expand Down Expand Up @@ -218,7 +218,7 @@ private Object applyConversion(@Nullable Object value, NamedValueInfo namedValue
/**
* 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(() -> {
Expand All @@ -230,10 +230,10 @@ private Mono<Object> getDefaultValue(NamedValueInfo namedValueInfo, MethodParame
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@
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;
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}.
Expand All @@ -64,6 +66,7 @@ class RequestHeaderMethodArgumentResolverTests {
private MethodParameter paramDate;
private MethodParameter paramInstant;
private MethodParameter paramMono;
private MethodParameter primitivePlaceholderParam;


@BeforeEach
Expand All @@ -87,6 +90,7 @@ void setup() throws Exception {
this.paramDate = new SynthesizingMethodParameter(method, 6);
this.paramInstant = new SynthesizingMethodParameter(method, 7);
this.paramMono = new SynthesizingMethodParameter(method, 8);
this.primitivePlaceholderParam = new SynthesizingMethodParameter(method, 9);
}


Expand Down Expand Up @@ -200,6 +204,46 @@ void resolveNameFromSystemPropertyThroughPlaceholder() {
}
}

@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(
Expand Down Expand Up @@ -252,7 +296,8 @@ public void params(
@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) {
}

}

0 comments on commit 9fb1c34

Please sign in to comment.