From 138ef0c4c9867041bfc8df602473ebc07fa611a1 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 29 Apr 2024 09:09:46 +0200 Subject: [PATCH] Apply Kotlin Value Class unboxing to generated Property Accessors. Unwrap wrapped value types if necessary when using generated property accessors. Closes #3087 --- ...lassGeneratingPropertyAccessorFactory.java | 2 +- .../data/mapping/model/KotlinValueUtils.java | 40 ++++++++++++++++--- .../KotlinPropertyAccessorFactoryTests.java | 28 +++++++++++++ .../data/mapping/model/InlineClasses.kt | 6 +++ 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java index 86752b826..eefbed43b 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -1481,7 +1481,7 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert Parameter parameter = copy.getParameters()[kotlinCopyByProperty.getParameterPosition()]; - return o -> ClassUtils.isAssignableValue(parameter.getType(), o) || vh == null ? o : vh.wrap(o); + return o -> ClassUtils.isAssignableValue(parameter.getType(), o) || vh == null ? o : vh.applyWrapping(o); } return Function.identity(); diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java b/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java index fa987a8fd..3d775e241 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java @@ -33,6 +33,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.function.BiFunction; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -205,6 +206,8 @@ class KotlinValueUtils { private final KFunction wrapperConstructor; + private final KProperty valueProperty; + private final boolean applyBoxing; private final @Nullable ValueBoxing next; @@ -263,7 +266,6 @@ class KotlinValueUtils { boolean applyBoxing; if (kClass.isValue()) { - wrapperConstructor = kClass.getConstructors().iterator().next(); KParameter nested = wrapperConstructor.getParameters().get(0); KType nestedType = nested.getType(); @@ -280,10 +282,12 @@ class KotlinValueUtils { } Assert.notNull(nestedClass, () -> String.format("Cannot resolve nested class from type %s", nestedType)); - + this.valueProperty = kClass.getMembers().stream().filter(it -> it instanceof KProperty) + .map(KProperty.class::cast).findFirst().get(); next = new ValueBoxing(rules, nestedType, nestedClass, nested.isOptional()); } else { applyBoxing = false; + this.valueProperty = null; } this.kClass = kClass; @@ -378,20 +382,46 @@ class KotlinValueUtils { } /** - * Apply wrapping into the boxing wrapper type if applicable. + * Wrap the value into the boxing wrapper type if requested. Already wrapped values are left unchanged. * * @param o * @return */ @Nullable public Object wrap(@Nullable Object o) { + return doWrap(o, false, ValueBoxing::wrap); + } + + /** + * Apply wrapping into the boxing wrapper type if applicable. For types, that do not require wrapping but are + * wrapped, the component type is being unwrapped. + * + * @param o + * @return + * @since 3.2.6 + */ + @Nullable + Object applyWrapping(@Nullable Object o) { + return doWrap(o, true, ValueBoxing::applyWrapping); + } + + /** + * Apply staged wrapping into the boxing wrapper type if value boxing is requested. Otherwise, apply unwrapping and + * pass on the result into {@code nextWrapStage}. + */ + @Nullable + Object doWrap(@Nullable Object o, boolean unwrap, BiFunction nextWrapStage) { if (applyBoxing) { - return o == null || kClass.isInstance(o) ? o : wrapperConstructor.call(next.wrap(o)); + return o == null || kClass.isInstance(o) ? o : wrapperConstructor.call(nextWrapStage.apply(next, o)); + } else if (unwrap && kClass.isValue()) { + if (o != null && kClass.isInstance(o)) { + o = valueProperty.getGetter().call(o); + } } if (hasNext()) { - return next.wrap(o); + return nextWrapStage.apply(next, o); } return o; diff --git a/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java b/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java index a790be7fc..ced816abd 100644 --- a/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java +++ b/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java @@ -21,6 +21,7 @@ import kotlin.jvm.JvmClassMappingKt; import kotlin.reflect.KClass; import kotlin.reflect.jvm.internal.KotlinReflectionInternalError; +import java.lang.reflect.Constructor; import java.util.function.Function; import java.util.stream.Stream; @@ -255,6 +256,33 @@ public class KotlinPropertyAccessorFactoryTests { assertThat(propertyAccessor.getProperty(recursive)).isEqualTo(newOuter); } + @MethodSource("factories") + @ParameterizedTest // GH-1947 + void shouldUnwrapValueTypeIfNecessary(PersistentPropertyAccessorFactory factory) throws Exception { + + BasicPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(MyEntity.class); + + Constructor declaredConstructor = MyValueClass.class.getDeclaredConstructor(String.class); + + Object instance = createInstance(entity, parameter -> { + + String name = parameter.getName(); + + return switch (name) { + case "id" -> 1L; + case "name" -> "foo"; + default -> "bar"; + }; + + }); + + var propertyAccessor = factory.getPropertyAccessor(entity, instance); + var createdBy = entity.getRequiredPersistentProperty("createdBy"); + + propertyAccessor.setProperty(createdBy, BeanUtils.instantiateClass(declaredConstructor, "baz")); + } + private Object createInstance(BasicPersistentEntity entity, Function, Object> parameterProvider) { return instantiators.getInstantiatorFor(entity).createInstance(entity, diff --git a/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt index 0d3e5adfe..5f7a6c630 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt @@ -58,6 +58,12 @@ value class MyGenericValue(val id: T) @JvmInline value class MyGenericBoundValue(val id: T) +data class MyEntity( + val id: Long = 0L, + val name: String, + val createdBy: MyValueClass = MyValueClass("UNKNOWN"), +) + data class WithGenericValue( // ctor: WithGenericValue(CharSequence string, CharSequence charseq, Object recursive, DefaultConstructorMarker $constructor_marker) val string: MyGenericBoundValue,