Skip to content

Commit b935b29

Browse files
committed
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
1 parent eb1ad95 commit b935b29

File tree

4 files changed

+70
-14
lines changed

4 files changed

+70
-14
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,12 @@ private static KotlinDefaultMask from(KFunction<?> function, Predicate<KParamete
148148
masks.add(mask);
149149
}
150150

151-
return new KotlinDefaultMask(masks.stream().mapToInt(i -> i).toArray());
151+
int[] defaulting = new int[masks.size()];
152+
for (int i = 0; i < masks.size(); i++) {
153+
defaulting[i] = masks.get(i);
154+
}
155+
156+
return new KotlinDefaultMask(defaulting);
152157
}
153158

154159
public int[] getDefaulting() {

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import kotlin.reflect.jvm.ReflectJvmMapping;
2121

2222
import java.lang.reflect.Constructor;
23+
import java.lang.reflect.Modifier;
2324
import java.util.ArrayList;
2425
import java.util.IdentityHashMap;
2526
import java.util.List;
@@ -52,7 +53,6 @@ class KotlinInstantiationDelegate {
5253
private final Map<KParameter, Integer> indexByKParameter;
5354
private final List<Function<Object, Object>> wrappers = new ArrayList<>();
5455
private final Constructor<?> constructorToInvoke;
55-
private final boolean hasDefaultConstructorMarker;
5656

5757
public KotlinInstantiationDelegate(PreferredConstructor<?, ?> preferredConstructor,
5858
Constructor<?> constructorToInvoke) {
@@ -73,7 +73,6 @@ public KotlinInstantiationDelegate(PreferredConstructor<?, ?> preferredConstruct
7373
}
7474

7575
this.constructorToInvoke = constructorToInvoke;
76-
this.hasDefaultConstructorMarker = hasDefaultConstructorMarker(constructorToInvoke.getParameters());
7776

7877
for (KParameter kParameter : kParameters) {
7978

@@ -92,11 +91,7 @@ static boolean hasDefaultConstructorMarker(java.lang.reflect.Parameter[] paramet
9291
* @return number of constructor arguments.
9392
*/
9493
public int getRequiredParameterCount() {
95-
96-
return hasDefaultConstructorMarker ? constructorToInvoke.getParameterCount()
97-
: (constructorToInvoke.getParameterCount()
98-
+ KotlinDefaultMask.getMaskCount(constructorToInvoke.getParameterCount())
99-
+ /* DefaultConstructorMarker */1);
94+
return constructorToInvoke.getParameterCount();
10095
}
10196

10297
/**
@@ -160,13 +155,14 @@ public <P extends PersistentProperty<P>> Object[] extractInvocationArguments(Obj
160155
}
161156

162157
/**
163-
* Resolves a {@link PreferredConstructor} to a synthetic Kotlin constructor accepting the same user-space parameters
164-
* suffixed by Kotlin-specifics required for defaulting and the {@code kotlin.jvm.internal.DefaultConstructorMarker}.
158+
* Resolves a {@link PreferredConstructor} to the constructor to be invoked. This can be a synthetic Kotlin
159+
* constructor accepting the same user-space parameters suffixed by Kotlin-specifics required for defaulting and the
160+
* {@code kotlin.jvm.internal.DefaultConstructorMarker} or an actual non-synthetic constructor (i.e. private
161+
* constructor).
165162
*
166163
* @since 2.0
167164
* @author Mark Paluch
168165
*/
169-
170166
@SuppressWarnings("unchecked")
171167
@Nullable
172168
public static PreferredConstructor<?, ?> resolveKotlinJvmConstructor(
@@ -190,11 +186,18 @@ private static Constructor<?> doResolveKotlinConstructor(Constructor<?> detected
190186

191187
Class<?> entityType = detectedConstructor.getDeclaringClass();
192188
Constructor<?> hit = null;
189+
Constructor<?> privateFallback = null;
193190
KFunction<?> kotlinFunction = ReflectJvmMapping.getKotlinFunction(detectedConstructor);
194191

195192
for (Constructor<?> candidate : entityType.getDeclaredConstructors()) {
196193

197-
// use only synthetic constructors
194+
if (Modifier.isPrivate(candidate.getModifiers())) {
195+
if (detectedConstructor.equals(candidate)) {
196+
privateFallback = candidate;
197+
}
198+
}
199+
200+
// introspect only synthetic constructors
198201
if (!candidate.isSynthetic()) {
199202
continue;
200203
}
@@ -234,6 +237,10 @@ private static Constructor<?> doResolveKotlinConstructor(Constructor<?> detected
234237
}
235238
}
236239

240+
if (hit == null) {
241+
return privateFallback;
242+
}
243+
237244
return hit;
238245
}
239246

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,44 @@ data class WithMyValueClass(val id: MyValueClass) {
4747
// ---------
4848
}
4949

50-
data class WithMyValueClassPrivateConstructor private constructor(val id: MyValueClass)
50+
data class WithMyValueClassPrivateConstructor private constructor(val id: MyValueClass) {
5151

52-
data class WithMyValueClassPrivateConstructorAndDefaultValue private constructor(val id: MyValueClass = MyValueClass("id"))
52+
// ByteCode explanation
53+
54+
// ---------
55+
// default constructor, detected by Discoverers.KOTLIN
56+
// private WithMyValueClassPrivateConstructor(String id) {}
57+
// ---------
58+
}
59+
60+
data class WithNullableMyValueClassPrivateConstructor private constructor(val id: MyNullableValueClass?) {
61+
62+
// ByteCode explanation
63+
64+
// ---------
65+
// default constructor, detected by Discoverers.KOTLIN
66+
// private WithNullableMyValueClassPrivateConstructor(MyNullableValueClass id) {}
67+
// ---------
68+
}
69+
70+
data class WithMyValueClassPrivateConstructorAndDefaultValue private constructor(
71+
val id: MyValueClass = MyValueClass(
72+
"id"
73+
)
74+
) {
75+
76+
// ByteCode explanation
77+
// ---------
78+
// default constructor, detected by Discoverers.KOTLIN
79+
// private WithMyValueClassPrivateConstructorAndDefaultValue(java.lang.String id) {}
80+
// ---------
81+
82+
// ---------
83+
// synthetic constructor that we actually want to use
84+
// synthetic WithMyValueClassPrivateConstructorAndDefaultValue(java.lang.String id, int arg1, kotlin.jvm.internal.DefaultConstructorMarker arg2) {}
85+
// ---------
86+
87+
}
5388

5489
@JvmInline
5590
value class MyNullableValueClass(val id: String? = "id")

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,15 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests {
202202
assertThat(instance.id.id).isEqualTo("hello")
203203
}
204204

205+
@Test // GH-3389
206+
fun `should use private default constructor for types using nullable value class`() {
207+
208+
every { provider.getParameterValue<String>(any()) } returns "hello"
209+
val instance = construct(WithNullableMyValueClassPrivateConstructor::class)
210+
211+
assertThat(instance.id?.id).isEqualTo("hello")
212+
}
213+
205214
@Test // GH-3389
206215
fun `should use private default constructor for types using value class with default value`() {
207216

0 commit comments

Comments
 (0)