From 8379ac772af44ac56b5cd9752822a5a6f3c770de Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 27 Mar 2025 17:54:29 +0100 Subject: [PATCH] Introduce OptionalToObjectConverter We have had an ObjectToOptionalConverter since Spring Framework 4.1; however, prior to this commit we did not have a standard Converter for the inverse (Optional to Object). To address that, this commit introduces an OptionalToObjectConverter that unwraps an Optional, using the ConversionService to convert the object contained in the Optional (potentially null) to the target type. This allows for conversions such as the following. - Optional.empty() -> null - Optional.of(42) with Integer target -> 42 - Optional.of(42) with String target -> "42" - Optional.of(42) with Optional target -> Optional.of("42") The OptionalToObjectConverter is also registered by default in DefaultConversionService, alongside the existing ObjectToOptionalConverter. See gh-20433 Closes gh-34544 --- .../support/DefaultConversionService.java | 3 +- .../support/ObjectToOptionalConverter.java | 1 + .../support/OptionalToObjectConverter.java | 68 ++++++++++++++++ .../DefaultConversionServiceTests.java | 77 +++++++++++++++++++ .../spel/ExpressionWithConversionTests.java | 41 ++++++++++ 5 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 spring-core/src/main/java/org/springframework/core/convert/support/OptionalToObjectConverter.java diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java index 9f486796538..d7881accdca 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -99,6 +99,7 @@ public class DefaultConversionService extends GenericConversionService { converterRegistry.addConverter(new IdToEntityConverter((ConversionService) converterRegistry)); converterRegistry.addConverter(new FallbackObjectToStringConverter()); converterRegistry.addConverter(new ObjectToOptionalConverter((ConversionService) converterRegistry)); + converterRegistry.addConverter(new OptionalToObjectConverter((ConversionService) converterRegistry)); } /** diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java index a41f5190375..3cd9adf8779 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/ObjectToOptionalConverter.java @@ -36,6 +36,7 @@ import org.springframework.util.CollectionUtils; * @author Rossen Stoyanchev * @author Juergen Hoeller * @since 4.1 + * @see OptionalToObjectConverter */ final class ObjectToOptionalConverter implements ConditionalGenericConverter { diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/OptionalToObjectConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/OptionalToObjectConverter.java new file mode 100644 index 00000000000..7393473d81e --- /dev/null +++ b/spring-core/src/main/java/org/springframework/core/convert/support/OptionalToObjectConverter.java @@ -0,0 +1,68 @@ +/* + * Copyright 2002-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.convert.support; + +import java.util.Optional; +import java.util.Set; + +import org.jspecify.annotations.Nullable; + +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalGenericConverter; + +/** + * Convert an {@link Optional} to an {@link Object} by unwrapping the {@code Optional}, + * using the {@link ConversionService} to convert the object contained in the + * {@code Optional} (potentially {@code null}) to the target type. + * + * @author Sam Brannen + * @since 7.0 + * @see ObjectToOptionalConverter + */ +final class OptionalToObjectConverter implements ConditionalGenericConverter { + + private final ConversionService conversionService; + + + OptionalToObjectConverter(ConversionService conversionService) { + this.conversionService = conversionService; + } + + + @Override + public Set getConvertibleTypes() { + return Set.of(new ConvertiblePair(Optional.class, Object.class)); + } + + @Override + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + return ConversionUtils.canConvertElements(sourceType.getElementTypeDescriptor(), targetType, this.conversionService); + } + + @Override + public @Nullable Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + if (source == null) { + return null; + } + Optional optional = (Optional) source; + Object unwrappedSource = optional.orElse(null); + TypeDescriptor unwrappedSourceType = TypeDescriptor.forObject(unwrappedSource); + return this.conversionService.convert(unwrappedSource, unwrappedSourceType, targetType); + } + +} diff --git a/spring-core/src/test/java/org/springframework/core/convert/converter/DefaultConversionServiceTests.java b/spring-core/src/test/java/org/springframework/core/convert/converter/DefaultConversionServiceTests.java index b50c793a889..5e07edd8f3b 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/converter/DefaultConversionServiceTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/converter/DefaultConversionServiceTests.java @@ -48,6 +48,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.core.MethodParameter; +import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.TypeDescriptor; @@ -56,6 +57,7 @@ import org.springframework.util.ClassUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.byLessThan; import static org.assertj.core.api.Assertions.entry; /** @@ -978,6 +980,19 @@ class DefaultConversionServiceTests { assertThat(conversionService.convert(null, rawOptionalType, TypeDescriptor.valueOf(Object.class))).isNull(); } + @Test // gh-34544 + void convertEmptyOptionalToNull() { + Optional empty = Optional.empty(); + + assertThat(conversionService.convert(empty, Object.class)).isNull(); + assertThat(conversionService.convert(empty, String.class)).isNull(); + + assertThat(conversionService.convert(empty, rawOptionalType, TypeDescriptor.valueOf(Object.class))).isNull(); + assertThat(conversionService.convert(empty, rawOptionalType, TypeDescriptor.valueOf(String.class))).isNull(); + assertThat(conversionService.convert(empty, rawOptionalType, TypeDescriptor.valueOf(Integer[].class))).isNull(); + assertThat(conversionService.convert(empty, rawOptionalType, TypeDescriptor.valueOf(List.class))).isNull(); + } + @Test void convertEmptyOptionalToOptional() { assertThat((Object) conversionService.convert(Optional.empty(), Optional.class)).isSameAs(Optional.empty()); @@ -985,6 +1000,68 @@ class DefaultConversionServiceTests { .isSameAs(Optional.empty()); } + @Test // gh-34544 + @SuppressWarnings("unchecked") + void convertOptionalToOptionalWithoutConversionOfContainedObject() { + assertThat(conversionService.convert(Optional.of(42), Optional.class)).contains(42); + + assertThat(conversionService.convert(Optional.of("enigma"), Optional.class)).contains("enigma"); + assertThat((Optional) conversionService.convert(Optional.of("enigma"), rawOptionalType, rawOptionalType)) + .contains("enigma"); + } + + @Test // gh-34544 + @SuppressWarnings("unchecked") + void convertOptionalToOptionalWithConversionOfContainedObject() { + TypeDescriptor integerOptionalType = + new TypeDescriptor(ResolvableType.forClassWithGenerics(Optional.class, Integer.class), null, null); + TypeDescriptor stringOptionalType = + new TypeDescriptor(ResolvableType.forClassWithGenerics(Optional.class, String.class), null, null); + + assertThat((Optional) conversionService.convert(Optional.of(42), integerOptionalType, stringOptionalType)) + .contains("42"); + } + + @Test // gh-34544 + @SuppressWarnings("unchecked") + void convertOptionalToObjectWithoutConversionOfContainedObject() { + assertThat(conversionService.convert(Optional.of("enigma"), String.class)).isEqualTo("enigma"); + assertThat(conversionService.convert(Optional.of(42), Integer.class)).isEqualTo(42); + assertThat(conversionService.convert(Optional.of(new int[] {1, 2, 3}), int[].class)).containsExactly(1, 2, 3); + assertThat(conversionService.convert(Optional.of(new Integer[] {1, 2, 3}), Integer[].class)).containsExactly(1, 2, 3); + assertThat(conversionService.convert(Optional.of(List.of(1, 2, 3)), List.class)).containsExactly(1, 2, 3); + } + + @Test // gh-34544 + @SuppressWarnings("unchecked") + void convertOptionalToObjectWithConversionOfContainedObject() { + assertThat(conversionService.convert(Optional.of(42), String.class)).isEqualTo("42"); + assertThat(conversionService.convert(Optional.of(3.14F), Double.class)).isCloseTo(3.14, byLessThan(0.001)); + assertThat(conversionService.convert(Optional.of(new int[] {1, 2, 3}), Integer[].class)).containsExactly(1, 2, 3); + assertThat(conversionService.convert(Optional.of(List.of(1, 2, 3)), Set.class)).containsExactly(1, 2, 3); + } + + @Test // gh-34544 + @SuppressWarnings("unchecked") + void convertNestedOptionalsToObject() { + assertThat(conversionService.convert(Optional.of(Optional.of("unwrap me twice")), String.class)) + .isEqualTo("unwrap me twice"); + } + + @Test // gh-34544 + @SuppressWarnings("unchecked") + void convertOptionalToObjectViaTypeDescriptorForMethodParameter() { + Method method = ClassUtils.getMethod(getClass(), "handleList", List.class); + MethodParameter parameter = new MethodParameter(method, 0); + TypeDescriptor descriptor = new TypeDescriptor(parameter); + + Optional> source = Optional.of(List.of(1, 2, 3)); + assertThat((List) conversionService.convert(source, rawOptionalType, descriptor)).containsExactly(1, 2, 3); + } + + public void handleList(List value) { + } + public void handleOptionalList(Optional> value) { } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionWithConversionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionWithConversionTests.java index 6b3d1640b8a..34840388a97 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionWithConversionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionWithConversionTests.java @@ -18,8 +18,10 @@ package org.springframework.expression.spel; import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.Set; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; import org.springframework.core.MethodParameter; @@ -38,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Andy Clement * @author Dave Syer + * @author Sam Brannen */ class ExpressionWithConversionTests extends AbstractExpressionTests { @@ -152,6 +155,27 @@ class ExpressionWithConversionTests extends AbstractExpressionTests { assertThat(baz.value).isEqualTo("quux"); } + @Test // gh-34544 + void convertOptionalToContainedTargetForMethodInvocations() { + StandardEvaluationContext context = new StandardEvaluationContext(new JediService()); + + // Verify findByName('Yoda') returns an Optional. + Expression expression = parser.parseExpression("findByName('Yoda') instanceof T(java.util.Optional)"); + assertThat(expression.getValue(context, Boolean.class)).isTrue(); + + // Verify we can pass a Jedi directly to greet(). + expression = parser.parseExpression("greet(findByName('Yoda').get())"); + assertThat(expression.getValue(context, String.class)).isEqualTo("Hello, Yoda"); + + // Verify that an Optional will be unwrapped to a Jedi to pass to greet(). + expression = parser.parseExpression("greet(findByName('Yoda'))"); + assertThat(expression.getValue(context, String.class)).isEqualTo("Hello, Yoda"); + + // Verify that an empty Optional will be converted to null to pass to greet(). + expression = parser.parseExpression("greet(findByName(''))"); + assertThat(expression.getValue(context, String.class)).isEqualTo("Hello, null"); + } + public static class Foo { @@ -180,4 +204,21 @@ class ExpressionWithConversionTests extends AbstractExpressionTests { } } + record Jedi(String name) { + } + + static class JediService { + + public Optional findByName(String name) { + if (name.isEmpty()) { + return Optional.empty(); + } + return Optional.of(new Jedi(name)); + } + + public String greet(@Nullable Jedi jedi) { + return "Hello, " + (jedi != null ? jedi.name() : null); + } + } + }