Skip ValueObjectBinder if parameter names cannot be discovered
Update `ValueObjectBinder` so that it is skipped if parameter names cannot be discovered. This is much more likely as of Since Spring Framework 6.1 as it no longer performs ASM parsing to discover names. Fixes gh-38201
This commit is contained in:
@@ -31,6 +31,8 @@ import java.util.Optional;
|
||||
import kotlin.reflect.KFunction;
|
||||
import kotlin.reflect.KParameter;
|
||||
import kotlin.reflect.jvm.ReflectJvmMapping;
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
|
||||
import org.springframework.beans.BeanUtils;
|
||||
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
|
||||
@@ -43,6 +45,7 @@ import org.springframework.core.ResolvableType;
|
||||
import org.springframework.core.annotation.MergedAnnotation;
|
||||
import org.springframework.core.annotation.MergedAnnotations;
|
||||
import org.springframework.core.convert.ConversionException;
|
||||
import org.springframework.core.log.LogMessage;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
/**
|
||||
@@ -55,6 +58,8 @@ import org.springframework.util.Assert;
|
||||
*/
|
||||
class ValueObjectBinder implements DataObjectBinder {
|
||||
|
||||
private static final Log logger = LogFactory.getLog(ValueObjectBinder.class);
|
||||
|
||||
private final BindConstructorProvider constructorProvider;
|
||||
|
||||
ValueObjectBinder(BindConstructorProvider constructorProvider) {
|
||||
@@ -261,15 +266,31 @@ class ValueObjectBinder implements DataObjectBinder {
|
||||
|
||||
private final List<ConstructorParameter> constructorParameters;
|
||||
|
||||
private DefaultValueObject(Constructor<T> constructor, ResolvableType type) {
|
||||
private DefaultValueObject(Constructor<T> constructor, List<ConstructorParameter> constructorParameters) {
|
||||
super(constructor);
|
||||
this.constructorParameters = parseConstructorParameters(constructor, type);
|
||||
this.constructorParameters = constructorParameters;
|
||||
}
|
||||
|
||||
@Override
|
||||
List<ConstructorParameter> getConstructorParameters() {
|
||||
return this.constructorParameters;
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
static <T> ValueObject<T> get(Constructor<?> bindConstructor, ResolvableType type) {
|
||||
String[] names = PARAMETER_NAME_DISCOVERER.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);
|
||||
return new DefaultValueObject<>((Constructor<T>) bindConstructor, constructorParameters);
|
||||
}
|
||||
|
||||
private static List<ConstructorParameter> parseConstructorParameters(Constructor<?> constructor,
|
||||
ResolvableType type) {
|
||||
String[] names = PARAMETER_NAME_DISCOVERER.getParameterNames(constructor);
|
||||
Assert.state(names != null, () -> "Failed to extract parameter names for " + constructor);
|
||||
ResolvableType type, String[] names) {
|
||||
Parameter[] parameters = constructor.getParameters();
|
||||
List<ConstructorParameter> result = new ArrayList<>(parameters.length);
|
||||
for (int i = 0; i < parameters.length; i++) {
|
||||
@@ -285,16 +306,6 @@ class ValueObjectBinder implements DataObjectBinder {
|
||||
return Collections.unmodifiableList(result);
|
||||
}
|
||||
|
||||
@Override
|
||||
List<ConstructorParameter> getConstructorParameters() {
|
||||
return this.constructorParameters;
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
static <T> ValueObject<T> get(Constructor<?> bindConstructor, ResolvableType type) {
|
||||
return new DefaultValueObject<>((Constructor<T>) bindConstructor, type);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -26,11 +26,13 @@ import java.util.Map;
|
||||
import java.util.Objects;
|
||||
import java.util.Optional;
|
||||
|
||||
import com.jayway.jsonpath.JsonPath;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
|
||||
import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
|
||||
import org.springframework.boot.context.properties.source.MockConfigurationPropertySource;
|
||||
import org.springframework.core.DefaultParameterNameDiscoverer;
|
||||
import org.springframework.core.ResolvableType;
|
||||
import org.springframework.core.convert.ConversionService;
|
||||
import org.springframework.core.test.tools.SourceFile;
|
||||
@@ -391,6 +393,25 @@ class ValueObjectBinderTests {
|
||||
});
|
||||
}
|
||||
|
||||
@Test // gh-38201
|
||||
void bindWithNonExtractableParameterNamesAndNonIterablePropertySource() throws Exception {
|
||||
verifyJsonPathParametersCannotBeResolved();
|
||||
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
|
||||
source.put("test.value", "test");
|
||||
this.sources.add(source.nonIterable());
|
||||
Bindable<NonExtractableParameterName> target = Bindable.of(NonExtractableParameterName.class);
|
||||
NonExtractableParameterName bound = this.binder.bindOrCreate("test", target);
|
||||
assertThat(bound.getValue()).isEqualTo("test");
|
||||
}
|
||||
|
||||
private void verifyJsonPathParametersCannotBeResolved() throws NoSuchFieldException {
|
||||
Class<?> jsonPathClass = NonExtractableParameterName.class.getDeclaredField("jsonPath").getType();
|
||||
Constructor<?>[] constructors = jsonPathClass.getDeclaredConstructors();
|
||||
assertThat(constructors).hasSize(1);
|
||||
constructors[0].setAccessible(true);
|
||||
assertThat(new DefaultParameterNameDiscoverer().getParameterNames(constructors[0])).isNull();
|
||||
}
|
||||
|
||||
private void noConfigurationProperty(BindException ex) {
|
||||
assertThat(ex.getProperty()).isNull();
|
||||
}
|
||||
@@ -845,4 +866,28 @@ class ValueObjectBinderTests {
|
||||
|
||||
}
|
||||
|
||||
static class NonExtractableParameterName {
|
||||
|
||||
private String value;
|
||||
|
||||
private JsonPath jsonPath;
|
||||
|
||||
String getValue() {
|
||||
return this.value;
|
||||
}
|
||||
|
||||
void setValue(String value) {
|
||||
this.value = value;
|
||||
}
|
||||
|
||||
JsonPath getJsonPath() {
|
||||
return this.jsonPath;
|
||||
}
|
||||
|
||||
void setJsonPath(JsonPath jsonPath) {
|
||||
this.jsonPath = jsonPath;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user