mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-09-17 08:24:13 +00:00
Fix OptionalToObjectConverter applicability check
OptionalToObjectConverter.matches() used TypeDescriptor.getElementTypeDescriptor(), which returns null for an Optional (element types are only resolved for arrays, streams and collections). ConversionUtils.canConvertElements() then treats a null source element type as "maybe" and returns true unconditionally, so ConversionService.canConvert(Optional<X>, target) reported true even when X is not convertible to the target -- a violation of the canConvert contract, since the subsequent conversion fails. To address that, this commit resolves the Optional's element type from its generic and checks it against the target, mirroring ObjectToOptionalConverter. A raw or otherwise unresolved element type remains permissive. Closes gh-36913 Signed-off-by: junhyeong9812 <pickjog@gmail.com>
This commit is contained in:
+9
-1
@@ -21,6 +21,7 @@ import java.util.Set;
|
||||
|
||||
import org.jspecify.annotations.Nullable;
|
||||
|
||||
import org.springframework.core.ResolvableType;
|
||||
import org.springframework.core.convert.ConversionService;
|
||||
import org.springframework.core.convert.TypeDescriptor;
|
||||
import org.springframework.core.convert.converter.ConditionalGenericConverter;
|
||||
@@ -51,7 +52,14 @@ final class OptionalToObjectConverter implements ConditionalGenericConverter {
|
||||
|
||||
@Override
|
||||
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
|
||||
return ConversionUtils.canConvertElements(sourceType.getElementTypeDescriptor(), targetType, this.conversionService);
|
||||
ResolvableType elementType = sourceType.getResolvableType().getGeneric();
|
||||
if (elementType.resolve() == null) {
|
||||
// Unknown Optional element type (raw Optional, wildcard, or unresolved
|
||||
// type variable): remain permissive.
|
||||
return true;
|
||||
}
|
||||
TypeDescriptor sourceElementType = new TypeDescriptor(elementType, null, null);
|
||||
return ConversionUtils.canConvertElements(sourceElementType, targetType, this.conversionService);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
+67
@@ -25,6 +25,7 @@ import java.nio.charset.StandardCharsets;
|
||||
import java.sql.Time;
|
||||
import java.sql.Timestamp;
|
||||
import java.time.Instant;
|
||||
import java.time.LocalDate;
|
||||
import java.time.ZoneId;
|
||||
import java.util.AbstractList;
|
||||
import java.util.ArrayList;
|
||||
@@ -1012,6 +1013,72 @@ class DefaultConversionServiceTests {
|
||||
|
||||
private static final TypeDescriptor rawOptionalType = TypeDescriptor.valueOf(Optional.class);
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
private Optional<?> unboundedWildcardOptional;
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
private Optional<? extends Number> boundedWildcardOptional;
|
||||
|
||||
|
||||
@Test // raw Optional: no element type information, so the converter remains permissive
|
||||
void canConvertRawOptionalRemainsPermissive() {
|
||||
assertThat(conversionService.canConvert(rawOptionalType, TypeDescriptor.valueOf(LocalDate.class))).isTrue();
|
||||
}
|
||||
|
||||
@Test // Optional<?>: the Object upper bound resolves to null, so the converter remains permissive
|
||||
void canConvertOptionalWithUnboundedWildcardRemainsPermissive() throws Exception {
|
||||
TypeDescriptor unboundedWildcardOptionalType = optionalTypeFor("unboundedWildcardOptional");
|
||||
assertThat(conversionService.canConvert(unboundedWildcardOptionalType, TypeDescriptor.valueOf(LocalDate.class))).isTrue();
|
||||
}
|
||||
|
||||
@Test // Optional<T> with an unresolved type variable also remains permissive
|
||||
void canConvertOptionalWithUnresolvedTypeVariableRemainsPermissive() throws Exception {
|
||||
TypeDescriptor typeVariableOptionalType =
|
||||
new TypeDescriptor(TypeVariableHolder.class.getDeclaredField("typeVariableOptional"));
|
||||
assertThat(conversionService.canConvert(typeVariableOptionalType, TypeDescriptor.valueOf(LocalDate.class))).isTrue();
|
||||
}
|
||||
|
||||
@Test // Optional<? extends Number>: applicability is decided against the Number upper bound
|
||||
void canConvertOptionalWithBoundedWildcardReflectsUpperBound() throws Exception {
|
||||
TypeDescriptor boundedWildcardOptionalType = optionalTypeFor("boundedWildcardOptional");
|
||||
TypeDescriptor localDateType = TypeDescriptor.valueOf(LocalDate.class);
|
||||
|
||||
assertThat(conversionService.canConvert(boundedWildcardOptionalType, TypeDescriptor.valueOf(String.class))).isTrue();
|
||||
assertThat(conversionService.convert(Optional.of(42), boundedWildcardOptionalType,
|
||||
TypeDescriptor.valueOf(String.class))).isEqualTo("42");
|
||||
assertThat(conversionService.canConvert(boundedWildcardOptionalType, localDateType)).isFalse();
|
||||
assertThatExceptionOfType(ConverterNotFoundException.class)
|
||||
.isThrownBy(() -> conversionService.convert(Optional.of(42), boundedWildcardOptionalType, localDateType));
|
||||
}
|
||||
|
||||
private static TypeDescriptor optionalTypeFor(String fieldName) throws Exception {
|
||||
return new TypeDescriptor(OptionalConversionTests.class.getDeclaredField(fieldName));
|
||||
}
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
private static class TypeVariableHolder<T> {
|
||||
|
||||
Optional<T> typeVariableOptional;
|
||||
}
|
||||
|
||||
@Test
|
||||
void canConvertOptionalToObjectReflectsContainedElementType() {
|
||||
TypeDescriptor integerOptionalType =
|
||||
new TypeDescriptor(ResolvableType.forClassWithGenerics(Optional.class, Integer.class), null, null);
|
||||
TypeDescriptor localDateType = TypeDescriptor.valueOf(LocalDate.class);
|
||||
|
||||
// Integer -> String is convertible
|
||||
assertThat(conversionService.canConvert(integerOptionalType, TypeDescriptor.valueOf(String.class))).isTrue();
|
||||
|
||||
// Integer -> LocalDate is not convertible, so canConvert must not over-report...
|
||||
assertThat(conversionService.canConvert(integerOptionalType, localDateType)).isFalse();
|
||||
// ...and must stay consistent with convert(): no converter is selected
|
||||
assertThatExceptionOfType(ConverterNotFoundException.class)
|
||||
.isThrownBy(() -> conversionService.convert(Optional.of(42), integerOptionalType, localDateType));
|
||||
|
||||
// An Optional with an unknown element type remains permissive
|
||||
assertThat(conversionService.canConvert(rawOptionalType, localDateType)).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
@SuppressWarnings("unchecked")
|
||||
|
||||
Reference in New Issue
Block a user