From 3a66a5defa4bd14f0f27cc1282848f69533407a4 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 21 Feb 2024 09:26:24 +0100 Subject: [PATCH 1/4] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8576363d34..44e2893086 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 3.3.0-SNAPSHOT + 3.3.x-3041-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. From 8cfb0d6023a97f9f5965a0b8c499731114607b16 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 21 Feb 2024 08:45:46 +0100 Subject: [PATCH 2/4] Add failing testcase --- ...ssGeneratingEntityInstantiatorUnitTests.kt | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt index 5ee79c3495..0e8508f29b 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -21,8 +21,11 @@ import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test import org.springframework.data.annotation.PersistenceConstructor +import org.springframework.data.annotation.Persistent import org.springframework.data.mapping.PersistentEntity import org.springframework.data.mapping.context.SamplePersistentProperty +import org.springframework.data.mapping.model.KotlinValueUtils.BoxingRules +import kotlin.jvm.internal.Reflection import kotlin.reflect.KClass /** @@ -149,6 +152,28 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { assertThat(instance.aBool).isTrue() } + @Test // GH-3041 + fun `should pick preferred constructor if multiple with same argument count are present`() { + + val entity = + mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + WithConstructorsHavingSameParameterCount::class.java + ) + + every { provider.getParameterValue(any()) }.returns(1L).andThen(null) + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + val instance: WithConstructorsHavingSameParameterCount = + KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + assertThat(instance.id).isEqualTo(1L) + assertThat(instance.notes).isEmpty(); + } + @Test // DATACMNS-1338 fun `should create instance using @PersistenceConstructor`() { @@ -271,6 +296,11 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { val aFloat: Float = 0.0f, val aDouble: Double = 0.0, val aChar: Char = 'a', val aBool: Boolean = true ) + + data class WithConstructorsHavingSameParameterCount @PersistenceConstructor constructor(val id: Long?, val notes: Map = emptyMap()) { + constructor(notes: Map, additionalNotes: Map = emptyMap()) : this(null, notes + additionalNotes) + } + data class ContactWithPersistenceConstructor(val firstname: String, val lastname: String) { @PersistenceConstructor From de42269f00337e1bc09f22dda7c390e20fee3f85 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 21 Feb 2024 09:25:19 +0100 Subject: [PATCH 3/4] Default generic type arguments when resolving KType for Class. We now fill up missing KTypeProjection arguments with star because the Kotlin Reflection.typeOf resolution fails if arguments are not provided. --- .../data/mapping/model/KotlinValueUtils.java | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) 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 0d5b7bf1ea..48aba32fe0 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java @@ -25,12 +25,14 @@ import kotlin.reflect.KProperty; import kotlin.reflect.KType; import kotlin.reflect.KTypeParameter; +import kotlin.reflect.KTypeProjection; import kotlin.reflect.jvm.ReflectJvmMapping; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.stream.IntStream; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -39,6 +41,7 @@ * Utilities for Kotlin Value class support. * * @author Mark Paluch + * @author Christoph Strobl * @since 3.2 */ class KotlinValueUtils { @@ -72,7 +75,27 @@ public static ValueBoxing getConstructorValueHierarchy(KParameter parameter) { public static ValueBoxing getConstructorValueHierarchy(Class cls) { KClass kotlinClass = JvmClassMappingKt.getKotlinClass(cls); - return new ValueBoxing(BoxingRules.CONSTRUCTOR, Reflection.typeOf(kotlinClass), kotlinClass, false); + KType kType = extractKType(kotlinClass); + return new ValueBoxing(BoxingRules.CONSTRUCTOR, kType, kotlinClass, false); + } + + /** + * Get the {@link KType} for a given {@link KClass} and potentially fill missing generic type arguments with + * {@link KTypeProjection#star} to prevent Kotlin internal checks to fail. + * + * @param kotlinClass + * @return + */ + private static KType extractKType(KClass kotlinClass) { + + return kotlinClass.getTypeParameters().isEmpty() ? Reflection.typeOf(kotlinClass) + : Reflection.typeOf(JvmClassMappingKt.getJavaClass(kotlinClass), stubKTypeProjections(kotlinClass)); + } + + private static KTypeProjection[] stubKTypeProjections(KClass kotlinClass) { + + return IntStream.range(0, kotlinClass.getTypeParameters().size()).mapToObj(it -> KTypeProjection.star) + .toArray(KTypeProjection[]::new); } /** From e800c68d3294b7d9fe49483a46b0def0f5c039db Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 21 Feb 2024 11:37:28 +0100 Subject: [PATCH 4/4] Switch to Arrays.fill --- .../data/mapping/model/KotlinValueUtils.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 48aba32fe0..7823e69194 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java @@ -30,9 +30,9 @@ import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.stream.IntStream; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -94,8 +94,9 @@ private static KType extractKType(KClass kotlinClass) { private static KTypeProjection[] stubKTypeProjections(KClass kotlinClass) { - return IntStream.range(0, kotlinClass.getTypeParameters().size()).mapToObj(it -> KTypeProjection.star) - .toArray(KTypeProjection[]::new); + KTypeProjection[] kTypeProjections = new KTypeProjection[kotlinClass.getTypeParameters().size()]; + Arrays.fill(kTypeProjections, KTypeProjection.star); + return kTypeProjections; } /**