Add suppressed missing parameters exception from ValueObjectBinder

Update `DataObjectBinder` interface and `ValueObjectBinder`
implementation so that suppressed exceptions are added whenever
parameter names cannot be discovered.

See gh-38603
This commit is contained in:
Phillip Webb 2023-12-05 14:08:08 -08:00
parent 6b58051aad
commit f609022731
4 changed files with 101 additions and 16 deletions

View File

@ -366,7 +366,13 @@ public class Binder {
(dataObjectBinder) -> dataObjectBinder.create(target, context));
result = handler.onCreate(name, target, context, result);
result = context.getConverter().convert(result, target);
Assert.state(result != null, () -> "Unable to create instance for " + target.getType());
if (result == null) {
IllegalStateException ex = new IllegalStateException(
"Unable to create instance for " + target.getType());
this.dataObjectBinders.get(target.getBindMethod())
.forEach((dataObjectBinder) -> dataObjectBinder.onUnableToCreateInstance(target, context, ex));
throw ex;
}
}
handler.onFinish(name, target, context, result);
return context.getConverter().convert(result, target);

View File

@ -53,4 +53,15 @@ interface DataObjectBinder {
*/
<T> T create(Bindable<T> target, Context context);
/**
* Callback that can be used to add additional suppressed exceptions when an instance
* cannot be created.
* @param <T> the source type
* @param target the bindable that was being created
* @param context the bind context
* @param exception the exception about to be thrown
*/
default <T> void onUnableToCreateInstance(Bindable<T> target, Binder.Context context, RuntimeException exception) {
}
}

View File

@ -19,6 +19,7 @@ package org.springframework.boot.context.properties.bind;
import java.lang.annotation.Annotation;
import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Parameter;
import java.util.ArrayList;
@ -27,6 +28,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Consumer;
import kotlin.reflect.KFunction;
import kotlin.reflect.KParameter;
@ -35,6 +37,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeanUtils;
import org.springframework.boot.context.properties.bind.Binder.Context;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
import org.springframework.core.CollectionFactory;
import org.springframework.core.DefaultParameterNameDiscoverer;
@ -69,7 +72,7 @@ class ValueObjectBinder implements DataObjectBinder {
@Override
public <T> T bind(ConfigurationPropertyName name, Bindable<T> target, Binder.Context context,
DataObjectPropertyBinder propertyBinder) {
ValueObject<T> valueObject = ValueObject.get(target, this.constructorProvider, context);
ValueObject<T> valueObject = ValueObject.get(target, this.constructorProvider, context, Discoverer.LENIENT);
if (valueObject == null) {
return null;
}
@ -90,7 +93,7 @@ class ValueObjectBinder implements DataObjectBinder {
@Override
public <T> T create(Bindable<T> target, Binder.Context context) {
ValueObject<T> valueObject = ValueObject.get(target, this.constructorProvider, context);
ValueObject<T> valueObject = ValueObject.get(target, this.constructorProvider, context, Discoverer.LENIENT);
if (valueObject == null) {
return null;
}
@ -102,6 +105,16 @@ class ValueObjectBinder implements DataObjectBinder {
return valueObject.instantiate(args);
}
@Override
public <T> void onUnableToCreateInstance(Bindable<T> target, Context context, RuntimeException exception) {
try {
ValueObject.get(target, this.constructorProvider, context, Discoverer.STRICT);
}
catch (Exception ex) {
exception.addSuppressed(ex);
}
}
private <T> T getDefaultValue(Binder.Context context, ConstructorParameter parameter) {
ResolvableType type = parameter.getType();
Annotation[] annotations = parameter.getAnnotations();
@ -187,7 +200,7 @@ class ValueObjectBinder implements DataObjectBinder {
@SuppressWarnings("unchecked")
static <T> ValueObject<T> get(Bindable<T> bindable, BindConstructorProvider constructorProvider,
Binder.Context context) {
Binder.Context context, ParameterNameDiscoverer parameterNameDiscoverer) {
Class<T> type = (Class<T>) bindable.getType().resolve();
if (type == null || type.isEnum() || Modifier.isAbstract(type.getModifiers())) {
return null;
@ -198,9 +211,10 @@ class ValueObjectBinder implements DataObjectBinder {
return null;
}
if (KotlinDetector.isKotlinType(type)) {
return KotlinValueObject.get((Constructor<T>) bindConstructor, bindable.getType());
return KotlinValueObject.get((Constructor<T>) bindConstructor, bindable.getType(),
parameterNameDiscoverer);
}
return DefaultValueObject.get(bindConstructor, bindable.getType());
return DefaultValueObject.get(bindConstructor, bindable.getType(), parameterNameDiscoverer);
}
}
@ -246,12 +260,13 @@ class ValueObjectBinder implements DataObjectBinder {
return this.constructorParameters;
}
static <T> ValueObject<T> get(Constructor<T> bindConstructor, ResolvableType type) {
static <T> ValueObject<T> get(Constructor<T> bindConstructor, ResolvableType type,
ParameterNameDiscoverer parameterNameDiscoverer) {
KFunction<T> kotlinConstructor = ReflectJvmMapping.getKotlinFunction(bindConstructor);
if (kotlinConstructor != null) {
return new KotlinValueObject<>(bindConstructor, kotlinConstructor, type);
}
return DefaultValueObject.get(bindConstructor, type);
return DefaultValueObject.get(bindConstructor, type, parameterNameDiscoverer);
}
}
@ -262,8 +277,6 @@ class ValueObjectBinder implements DataObjectBinder {
*/
private static final class DefaultValueObject<T> extends ValueObject<T> {
private static final ParameterNameDiscoverer PARAMETER_NAME_DISCOVERER = new DefaultParameterNameDiscoverer();
private final List<ConstructorParameter> constructorParameters;
private DefaultValueObject(Constructor<T> constructor, List<ConstructorParameter> constructorParameters) {
@ -277,12 +290,10 @@ class ValueObjectBinder implements DataObjectBinder {
}
@SuppressWarnings("unchecked")
static <T> ValueObject<T> get(Constructor<?> bindConstructor, ResolvableType type) {
String[] names = PARAMETER_NAME_DISCOVERER.getParameterNames(bindConstructor);
static <T> ValueObject<T> get(Constructor<?> bindConstructor, ResolvableType type,
ParameterNameDiscoverer parameterNameDiscoverer) {
String[] names = parameterNameDiscoverer.getParameterNames(bindConstructor);
if (names == null) {
logger.debug(LogMessage.format(
"Unable to use value object binding with %s as parameter names cannot be discovered",
bindConstructor));
return null;
}
List<ConstructorParameter> constructorParameters = parseConstructorParameters(bindConstructor, type, names);
@ -339,4 +350,49 @@ class ValueObjectBinder implements DataObjectBinder {
}
/**
* {@link ParameterNameDiscoverer} used for value data object binding.
*/
static final class Discoverer implements ParameterNameDiscoverer {
private static final ParameterNameDiscoverer DEFAULT_DELEGATE = new DefaultParameterNameDiscoverer();
private static final ParameterNameDiscoverer LENIENT = new Discoverer(DEFAULT_DELEGATE, (message) -> {
});
private static final ParameterNameDiscoverer STRICT = new Discoverer(DEFAULT_DELEGATE, (message) -> {
throw new IllegalStateException(message.toString());
});
private final ParameterNameDiscoverer delegate;
private final Consumer<LogMessage> noParameterNamesHandler;
private Discoverer(ParameterNameDiscoverer delegate, Consumer<LogMessage> noParameterNamesHandler) {
this.delegate = delegate;
this.noParameterNamesHandler = noParameterNamesHandler;
}
@Override
public String[] getParameterNames(Method method) {
throw new UnsupportedOperationException();
}
@Override
public String[] getParameterNames(Constructor<?> constructor) {
String[] names = this.delegate.getParameterNames(constructor);
if (names != null) {
return names;
}
LogMessage message = LogMessage.format(
"Unable to use value object binding with constructor [%s] as parameter names cannot be discovered. "
+ "Ensure that the compiler uses the '-parameters' flag",
constructor);
this.noParameterNamesHandler.accept(message);
logger.debug(message);
return null;
}
}
}

View File

@ -27,6 +27,7 @@ import java.util.Objects;
import java.util.Optional;
import com.jayway.jsonpath.JsonPath;
import com.jayway.jsonpath.internal.CharacterIndex;
import org.junit.jupiter.api.Test;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
@ -394,7 +395,7 @@ class ValueObjectBinderTests {
}
@Test // gh-38201
void bindWithNonExtractableParameterNamesAndNonIterablePropertySource() throws Exception {
void bindWhenNonExtractableParameterNamesOnPropertyAndNonIterablePropertySource() throws Exception {
verifyJsonPathParametersCannotBeResolved();
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("test.value", "test");
@ -404,6 +405,17 @@ class ValueObjectBinderTests {
assertThat(bound.getValue()).isEqualTo("test");
}
@Test
void createWhenNonExtractableParameterNamesOnPropertyAndNonIterablePropertySource() throws Exception {
assertThat(new DefaultParameterNameDiscoverer()
.getParameterNames(CharacterIndex.class.getDeclaredConstructor(CharSequence.class))).isNull();
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
this.sources.add(source.nonIterable());
Bindable<CharacterIndex> target = Bindable.of(CharacterIndex.class).withBindMethod(BindMethod.VALUE_OBJECT);
assertThatExceptionOfType(BindException.class).isThrownBy(() -> this.binder.bindOrCreate("test", target))
.withStackTraceContaining("Ensure that the compiler uses the '-parameters' flag");
}
private void verifyJsonPathParametersCannotBeResolved() throws NoSuchFieldException {
Class<?> jsonPathClass = NonExtractableParameterName.class.getDeclaredField("jsonPath").getType();
Constructor<?>[] constructors = jsonPathClass.getDeclaredConstructors();