Refine Kotlin constructor detection.

Attempt two-pass constructor detection in KotlinInstantiationDelegate to detect private constructors that are not synthetic ones.

See #3389
Original pull request: #3390

Author: mp911de

src/main/java/org/springframework/data/mapping/model/KotlinDefaultMask.java

@@ -148,7 +148,12 @@ private static KotlinDefaultMask from(KFunction<?> function, Predicate<KParamete
 			masks.add(mask);
 		}
 
-		return new KotlinDefaultMask(masks.stream().mapToInt(i -> i).toArray());
+		int[] defaulting = new int[masks.size()];
+		for (int i = 0; i < masks.size(); i++) {
+			defaulting[i] = masks.get(i);
+		}
+
+		return new KotlinDefaultMask(defaulting);
 	}
 
 	public int[] getDefaulting() {

src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java

@@ -20,6 +20,7 @@
 import kotlin.reflect.jvm.ReflectJvmMapping;
 
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.IdentityHashMap;
 import java.util.List;
@@ -53,7 +54,6 @@ class KotlinInstantiationDelegate {
 	private final Map<KParameter, Integer> indexByKParameter;
 	private final List<Function<@Nullable Object, @Nullable Object>> wrappers = new ArrayList<>();
 	private final Constructor<?> constructorToInvoke;
-	private final boolean hasDefaultConstructorMarker;
 
 	public KotlinInstantiationDelegate(PreferredConstructor<?, ?> preferredConstructor,
 			Constructor<?> constructorToInvoke) {
@@ -74,7 +74,6 @@ public KotlinInstantiationDelegate(PreferredConstructor<?, ?> preferredConstruct
 		}
 
 		this.constructorToInvoke = constructorToInvoke;
-		this.hasDefaultConstructorMarker = hasDefaultConstructorMarker(constructorToInvoke.getParameters());
 
 		for (KParameter kParameter : kParameters) {
 
@@ -93,11 +92,7 @@ static boolean hasDefaultConstructorMarker(java.lang.reflect.Parameter[] paramet
 	 * @return number of constructor arguments.
 	 */
 	public int getRequiredParameterCount() {
-
-		return hasDefaultConstructorMarker ? constructorToInvoke.getParameterCount()
-				: (constructorToInvoke.getParameterCount()
-						+ KotlinDefaultMask.getMaskCount(constructorToInvoke.getParameterCount())
-						+ /* DefaultConstructorMarker */1);
+		return constructorToInvoke.getParameterCount();
 	}
 
 	/**
@@ -154,13 +149,14 @@ public <P extends PersistentProperty<P>> void extractInvocationArguments(@Nullab
 	}
 
 	/**
-	 * Resolves a {@link PreferredConstructor} to a synthetic Kotlin constructor accepting the same user-space parameters
-	 * suffixed by Kotlin-specifics required for defaulting and the {@code kotlin.jvm.internal.DefaultConstructorMarker}.
+	 * Resolves a {@link PreferredConstructor} to the constructor to be invoked. This can be a synthetic Kotlin
+	 * constructor accepting the same user-space parameters suffixed by Kotlin-specifics required for defaulting and the
+	 * {@code kotlin.jvm.internal.DefaultConstructorMarker} or an actual non-synthetic constructor (i.e. private
+	 * constructor).
 	 *
 	 * @since 2.0
 	 * @author Mark Paluch
 	 */
-
 	@SuppressWarnings("unchecked")
 	@Nullable
 	public static PreferredConstructor<?, ?> resolveKotlinJvmConstructor(
@@ -184,11 +180,18 @@ private static Constructor<?> doResolveKotlinConstructor(Constructor<?> detected
 
 		Class<?> entityType = detectedConstructor.getDeclaringClass();
 		Constructor<?> hit = null;
+		Constructor<?> privateFallback = null;
 		KFunction<?> kotlinFunction = ReflectJvmMapping.getKotlinFunction(detectedConstructor);
 
 		for (Constructor<?> candidate : entityType.getDeclaredConstructors()) {
 
-			// use only synthetic constructors
+			if (Modifier.isPrivate(candidate.getModifiers())) {
+				if (detectedConstructor.equals(candidate)) {
+					privateFallback = candidate;
+				}
+			}
+
+			// introspect only synthetic constructors
 			if (!candidate.isSynthetic()) {
 				continue;
 			}
@@ -228,6 +231,10 @@ private static Constructor<?> doResolveKotlinConstructor(Constructor<?> detected
 			}
 		}
 
+		if (hit == null) {
+			return privateFallback;
+		}
+
 		return hit;
 	}
 

src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt

@@ -47,9 +47,44 @@ data class WithMyValueClass(val id: MyValueClass) {
 	// ---------
 }
 
-data class WithMyValueClassPrivateConstructor private constructor(val id: MyValueClass)
+data class WithMyValueClassPrivateConstructor private constructor(val id: MyValueClass) {
 
-data class WithMyValueClassPrivateConstructorAndDefaultValue private constructor(val id: MyValueClass = MyValueClass("id"))
+	// ByteCode explanation
+
+	// ---------
+	// default constructor, detected by Discoverers.KOTLIN
+	// private WithMyValueClassPrivateConstructor(String id) {}
+	// ---------
+}
+
+data class WithNullableMyValueClassPrivateConstructor private constructor(val id: MyNullableValueClass?) {
+
+	// ByteCode explanation
+
+	// ---------
+	// default constructor, detected by Discoverers.KOTLIN
+	// private WithNullableMyValueClassPrivateConstructor(MyNullableValueClass id) {}
+	// ---------
+}
+
+data class WithMyValueClassPrivateConstructorAndDefaultValue private constructor(
+	val id: MyValueClass = MyValueClass(
+		"id"
+	)
+) {
+
+	// ByteCode explanation
+	// ---------
+	// default constructor, detected by Discoverers.KOTLIN
+	// private WithMyValueClassPrivateConstructorAndDefaultValue(java.lang.String id) {}
+	// ---------
+
+	// ---------
+	// synthetic constructor that we actually want to use
+	// synthetic WithMyValueClassPrivateConstructorAndDefaultValue(java.lang.String id, int arg1, kotlin.jvm.internal.DefaultConstructorMarker arg2) {}
+	// ---------
+
+}
 
 @JvmInline
 value class MyNullableValueClass(val id: String? = "id")

src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt

@@ -199,6 +199,15 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests {
 		assertThat(instance.id.id).isEqualTo("hello")
 	}
 
+	@Test // GH-3389
+	fun `should use private default constructor for types using nullable value class`() {
+
+		every { provider.getParameterValue<String>(any()) } returns "hello"
+		val instance = construct(WithNullableMyValueClassPrivateConstructor::class)
+
+		assertThat(instance.id?.id).isEqualTo("hello")
+	}
+
 	@Test // GH-3389
 	fun `should use private default constructor for types using value class with default value`() {