Skip to content

Commit 09281c7

Browse files
christophstroblchristophstrobl
committed
Polishing.
Guard usage of KotlinReflectUtils with type presence check. Extend tests to cover primitive arrays. Move methods from KotlinValueUtils to KotlinReflectUtils. Move copy value cache to KotlinCopyMethod. Original Pull Request: #2866
1 parent 80ca2b4 commit 09281c7

File tree

10 files changed

+149
-53
lines changed

10 files changed

+149
-53
lines changed

‎src/main/java/org/springframework/data/mapping/model/BeanWrapper.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.List;
2626
import java.util.Map;
2727

28+
import org.springframework.core.KotlinDetector;
2829
import org.springframework.data.mapping.MappingException;
2930
import org.springframework.data.mapping.PersistentProperty;
3031
import org.springframework.data.mapping.PersistentPropertyAccessor;
@@ -74,7 +75,7 @@ public void setProperty(PersistentProperty<?> property, @Nullable Object value)
7475
return;
7576
}
7677

77-
if (KotlinReflectionUtils.isDataClass(property.getOwner().getType())) {
78+
if (KotlinDetector.isKotlinPresent() && KotlinReflectionUtils.isDataClass(property.getOwner().getType())) {
7879

7980
this.bean = (T) KotlinCopyUtil.setProperty(property, bean, value);
8081
return;
@@ -154,8 +155,6 @@ public T getBean() {
154155
*/
155156
static class KotlinCopyUtil {
156157

157-
private static final Map<Class<?>, KCallable<?>> copyMethodCache = new ConcurrentReferenceHashMap<>();
158-
159158
/**
160159
* Set a single property by calling {@code copy(…)} on a Kotlin data class. Copying creates a new instance that
161160
* holds all values of the original instance and the newly set {@link PersistentProperty} value.
@@ -165,7 +164,7 @@ static class KotlinCopyUtil {
165164
static <T> Object setProperty(PersistentProperty<?> property, T bean, @Nullable Object value) {
166165

167166
Class<?> type = property.getOwner().getType();
168-
KCallable<?> copy = copyMethodCache.computeIfAbsent(type, it -> getCopyMethod(it, property));
167+
KCallable<?> copy = getCopyMethod(type, property);
169168

170169
if (copy == null) {
171170
throw new UnsupportedOperationException(String.format("Kotlin class %s has no .copy(…) method for property %s",

‎src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java

+17-9
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.ArrayList;
3131
import java.util.Arrays;
3232
import java.util.List;
33+
import java.util.Map;
3334
import java.util.Optional;
3435
import java.util.function.Predicate;
3536
import java.util.stream.Collectors;
@@ -38,17 +39,22 @@
3839
import org.springframework.data.mapping.PersistentEntity;
3940
import org.springframework.data.mapping.PersistentProperty;
4041
import org.springframework.data.mapping.SimplePropertyHandler;
42+
import org.springframework.data.util.KotlinReflectionUtils;
4143
import org.springframework.util.Assert;
44+
import org.springframework.util.ConcurrentReferenceHashMap;
4245

4346
/**
4447
* Value object to represent a Kotlin {@code copy} method. The lookup requires a {@code copy} method that matches the
4548
* primary constructor of the class regardless of whether the primary constructor is the persistence constructor.
4649
*
4750
* @author Mark Paluch
51+
* @author Christoph Strobl
4852
* @since 2.1
4953
*/
5054
class KotlinCopyMethod {
5155

56+
private static final Map<Class<?>, Optional<KotlinCopyMethod>> COPY_METHOD_CACHE = new ConcurrentReferenceHashMap<>();
57+
5258
private final Method publicCopyMethod;
5359
private final Method syntheticCopyMethod;
5460
private final int parameterCount;
@@ -78,15 +84,17 @@ public static Optional<KotlinCopyMethod> findCopyMethod(Class<?> type) {
7884

7985
Assert.notNull(type, "Type must not be null");
8086

81-
Optional<Method> syntheticCopyMethod = findSyntheticCopyMethod(type);
87+
return COPY_METHOD_CACHE.computeIfAbsent(type, it -> {
8288

83-
if (!syntheticCopyMethod.isPresent()) {
84-
return Optional.empty();
85-
}
89+
Optional<Method> syntheticCopyMethod = findSyntheticCopyMethod(type);
8690

87-
Optional<Method> publicCopyMethod = syntheticCopyMethod.flatMap(KotlinCopyMethod::findPublicCopyMethod);
91+
if (!syntheticCopyMethod.isPresent()) {
92+
return Optional.empty();
93+
}
8894

89-
return publicCopyMethod.map(method -> new KotlinCopyMethod(method, syntheticCopyMethod.get()));
95+
Optional<Method> publicCopyMethod = syntheticCopyMethod.flatMap(KotlinCopyMethod::findPublicCopyMethod);
96+
return publicCopyMethod.map(method -> new KotlinCopyMethod(method, syntheticCopyMethod.get()));
97+
});
9098
}
9199

92100
public Method getPublicCopyMethod() {
@@ -171,7 +179,7 @@ private static Optional<Method> findPublicCopyMethod(Method defaultKotlinMethod)
171179
return Optional.empty();
172180
}
173181

174-
boolean usesValueClasses = KotlinValueUtils.hasValueClassProperty(type);
182+
boolean usesValueClasses = KotlinReflectionUtils.hasValueClassProperty(type);
175183
List<KParameter> constructorArguments = getComponentArguments(primaryConstructor);
176184
Predicate<String> isCopyMethod;
177185

@@ -242,7 +250,7 @@ private static Optional<Method> findSyntheticCopyMethod(Class<?> type) {
242250
return Optional.empty();
243251
}
244252

245-
boolean usesValueClasses = KotlinValueUtils.hasValueClassProperty(type);
253+
boolean usesValueClasses = KotlinReflectionUtils.hasValueClassProperty(type);
246254

247255
Predicate<String> isCopyMethod = usesValueClasses ? (it -> it.startsWith("copy-") && it.endsWith("$default"))
248256
: (it -> it.equals("copy$default"));
@@ -277,7 +285,7 @@ private static boolean matchesPrimaryConstructor(Class<?>[] parameterTypes, KFun
277285

278286
KParameter kParameter = constructorArguments.get(i);
279287

280-
if (KotlinValueUtils.isValueClass(kParameter.getType())) {
288+
if (KotlinReflectionUtils.isValueClass(kParameter.getType())) {
281289
// sigh. This can require deep unwrapping because the public vs. the synthetic copy methods use different
282290
// parameter types.
283291
continue;

‎src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java

+2-35
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import java.util.Collections;
3232
import java.util.List;
3333

34-
import org.springframework.core.KotlinDetector;
3534
import org.springframework.lang.Nullable;
3635
import org.springframework.util.Assert;
3736

@@ -43,39 +42,6 @@
4342
*/
4443
class KotlinValueUtils {
4544

46-
/**
47-
* Returns whether the given {@link KType} is a {@link KClass#isValue() value} class.
48-
*
49-
* @param type the kotlin type to inspect.
50-
* @return {@code true} the type is a value class.
51-
*/
52-
public static boolean isValueClass(KType type) {
53-
return type.getClassifier()instanceof KClass<?> kc && kc.isValue();
54-
}
55-
56-
/**
57-
* Returns whether the given class makes uses Kotlin {@link KClass#isValue() value} classes.
58-
*
59-
* @param type the kotlin type to inspect.
60-
* @return {@code true} when at least one property uses Kotlin value classes.
61-
*/
62-
public static boolean hasValueClassProperty(Class<?> type) {
63-
64-
if (!KotlinDetector.isKotlinType(type)) {
65-
return false;
66-
}
67-
68-
KClass<?> kotlinClass = JvmClassMappingKt.getKotlinClass(type);
69-
70-
for (KCallable<?> member : kotlinClass.getMembers()) {
71-
if (member instanceof KProperty<?> kp && isValueClass(kp.getReturnType())) {
72-
return true;
73-
}
74-
}
75-
76-
return false;
77-
}
78-
7945
/**
8046
* Creates a value hierarchy across value types from a given {@link KParameter} for COPY method usage.
8147
*
@@ -122,9 +88,10 @@ enum BoxingRules {
12288
public boolean shouldApplyBoxing(KType type, boolean optional, KParameter component) {
12389

12490
Type javaType = ReflectJvmMapping.getJavaType(component.getType());
125-
boolean isPrimitive = javaType instanceof Class<?> c && c.isPrimitive();
12691

12792
if (type.isMarkedNullable() || optional) {
93+
94+
boolean isPrimitive = javaType instanceof Class<?> c && c.isPrimitive();
12895
return (isPrimitive && type.isMarkedNullable()) || component.getType().isMarkedNullable();
12996
}
13097

‎src/main/java/org/springframework/data/mapping/model/MappingInstantiationException.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.List;
2525
import java.util.Optional;
2626

27+
import org.springframework.core.KotlinDetector;
2728
import org.springframework.data.mapping.FactoryMethod;
2829
import org.springframework.data.mapping.InstanceCreatorMetadata;
2930
import org.springframework.data.mapping.PersistentEntity;
@@ -117,7 +118,7 @@ private static String toString(PreferredConstructor<?, ?> preferredConstructor)
117118

118119
Constructor<?> constructor = preferredConstructor.getConstructor();
119120

120-
if (KotlinReflectionUtils.isSupportedKotlinClass(constructor.getDeclaringClass())) {
121+
if (KotlinDetector.isKotlinPresent() && KotlinReflectionUtils.isSupportedKotlinClass(constructor.getDeclaringClass())) {
121122

122123
KFunction<?> kotlinFunction = ReflectJvmMapping.getKotlinFunction(constructor);
123124

@@ -133,7 +134,7 @@ private static String toString(FactoryMethod<?, ?> factoryMethod) {
133134

134135
Method constructor = factoryMethod.getFactoryMethod();
135136

136-
if (KotlinReflectionUtils.isSupportedKotlinClass(constructor.getDeclaringClass())) {
137+
if (KotlinDetector.isKotlinPresent() && KotlinReflectionUtils.isSupportedKotlinClass(constructor.getDeclaringClass())) {
137138

138139
KFunction<?> kotlinFunction = ReflectJvmMapping.getKotlinFunction(constructor);
139140

‎src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java

+5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.Optional;
3030

3131
import org.springframework.core.DefaultParameterNameDiscoverer;
32+
import org.springframework.core.KotlinDetector;
3233
import org.springframework.core.ParameterNameDiscoverer;
3334
import org.springframework.core.annotation.AnnotationUtils;
3435
import org.springframework.data.annotation.PersistenceCreator;
@@ -220,6 +221,10 @@ <T, P extends PersistentProperty<P>> PreferredConstructor<T, P> discover(TypeInf
220221
* @return the appropriate discoverer for {@code type}.
221222
*/
222223
private static Discoverers findDiscoverer(Class<?> type) {
224+
225+
if(!KotlinDetector.isKotlinPresent()) {
226+
return DEFAULT;
227+
}
223228
return KotlinReflectionUtils.isSupportedKotlinClass(type) ? KOTLIN : DEFAULT;
224229
}
225230

‎src/main/java/org/springframework/data/repository/core/support/MethodInvocationValidator.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.aopalliance.intercept.MethodInterceptor;
2323
import org.aopalliance.intercept.MethodInvocation;
2424
import org.springframework.core.DefaultParameterNameDiscoverer;
25+
import org.springframework.core.KotlinDetector;
2526
import org.springframework.core.MethodParameter;
2627
import org.springframework.core.ParameterNameDiscoverer;
2728
import org.springframework.dao.EmptyResultDataAccessException;
@@ -58,7 +59,7 @@ public class MethodInvocationValidator implements MethodInterceptor {
5859
*/
5960
public static boolean supports(Class<?> repositoryInterface) {
6061

61-
return KotlinReflectionUtils.isSupportedKotlinClass(repositoryInterface)
62+
return KotlinDetector.isKotlinPresent() && KotlinReflectionUtils.isSupportedKotlinClass(repositoryInterface)
6263
|| NullableUtils.isNonNull(repositoryInterface, ElementType.METHOD)
6364
|| NullableUtils.isNonNull(repositoryInterface, ElementType.PARAMETER);
6465
}

‎src/main/java/org/springframework/data/util/KotlinReflectionUtils.java

+37-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737

3838
/**
3939
* Reflection utility methods specific to Kotlin reflection. Requires Kotlin classes to be present to avoid linkage
40-
* errors.
40+
* errors - ensure to guard usage with {@link KotlinDetector#isKotlinPresent()}.
4141
*
4242
* @author Mark Paluch
4343
* @author Christoph Strobl
@@ -132,6 +132,42 @@ public static Class<?> getReturnType(Method method) {
132132
return JvmClassMappingKt.getJavaClass(KTypesJvm.getJvmErasure(kotlinFunction.getReturnType()));
133133
}
134134

135+
/**
136+
* Returns whether the given {@link KType} is a {@link KClass#isValue() value} class.
137+
*
138+
* @param type the kotlin type to inspect.
139+
* @return {@code true} the type is a value class.
140+
* @since 3.2
141+
*/
142+
public static boolean isValueClass(KType type) {
143+
144+
return type.getClassifier() instanceof KClass<?> kc && kc.isValue();
145+
}
146+
147+
/**
148+
* Returns whether the given class makes uses Kotlin {@link KClass#isValue() value} classes.
149+
*
150+
* @param type the kotlin type to inspect.
151+
* @return {@code true} when at least one property uses Kotlin value classes.
152+
* @since 3.2
153+
*/
154+
public static boolean hasValueClassProperty(Class<?> type) {
155+
156+
if (!KotlinDetector.isKotlinType(type)) {
157+
return false;
158+
}
159+
160+
KClass<?> kotlinClass = JvmClassMappingKt.getKotlinClass(type);
161+
162+
for (KCallable<?> member : kotlinClass.getMembers()) {
163+
if (member instanceof KProperty<?> kp && isValueClass(kp.getReturnType())) {
164+
return true;
165+
}
166+
}
167+
168+
return false;
169+
}
170+
135171
/**
136172
* Returns {@literal} whether the given {@link MethodParameter} is nullable. Its declaring method can reference a
137173
* Kotlin function, property or interface property.

‎src/main/java/org/springframework/data/util/Predicates.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.lang.reflect.Modifier;
2222
import java.util.function.Predicate;
2323

24+
import org.springframework.core.KotlinDetector;
2425
import org.springframework.util.Assert;
2526

2627
/**
@@ -45,7 +46,7 @@ public interface Predicates {
4546
Predicate<Member> IS_PUBLIC = member -> Modifier.isPublic(member.getModifiers());
4647
Predicate<Member> IS_SYNTHETIC = Member::isSynthetic;
4748

48-
Predicate<Class<?>> IS_KOTLIN = KotlinReflectionUtils::isSupportedKotlinClass;
49+
Predicate<Class<?>> IS_KOTLIN = KotlinDetector.isKotlinPresent() ? KotlinReflectionUtils::isSupportedKotlinClass : type -> false;
4950
Predicate<Member> IS_STATIC = member -> Modifier.isStatic(member.getModifiers());
5051

5152
Predicate<Method> IS_BRIDGE_METHOD = Method::isBridge;

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

+19
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,25 @@ data class WithPrimitiveNullableValue(
8989
// copy: copy-lcs_1S0$default(WithPrimitiveNullableValue var0, PrimitiveNullableValue var1, PrimitiveNullableValue var2, PrimitiveNullableValue var3, PrimitiveNullableValue var4, int var5, Object var6)
9090
)
9191

92+
@JvmInline
93+
value class PrimitiveArrayValue(val ids: IntArray)
94+
95+
@JvmInline
96+
value class PrimitiveNullableArrayValue(val ids: IntArray?)
97+
98+
data class WithPrimitiveArrays(
99+
100+
// ctor WithPrimitiveArrays(int[], int[], int[], int[], int[], int, DefaultConstructorMarker)
101+
102+
val pa: PrimitiveArrayValue,
103+
val pan: PrimitiveArrayValue?,
104+
val pna: PrimitiveNullableArrayValue,
105+
val pad: PrimitiveArrayValue = PrimitiveArrayValue(intArrayOf(1, 2, 3)),
106+
val pand: PrimitiveArrayValue? = PrimitiveArrayValue(intArrayOf(1, 2, 3))
107+
108+
// copy: copy-NCSWWqw$default(WithPrimitiveArrays var0, int[] var1, int[] var2, PrimitiveNullableArrayValue var3, int[] var4, int[] var5, int var4, Object var5) {
109+
)
110+
92111
@JvmInline
93112
value class PrimitiveValue(val id: Int)
94113

‎src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt

+59
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,65 @@ class KotlinValueUtilsUnitTests {
145145
assertThat(nvdn.appliesBoxing()).isTrue
146146
}
147147

148+
@Test // GH-1947
149+
internal fun inlinesTypesToPrimitiveArrayCopyRules() {
150+
151+
val copy = KotlinCopyMethod.findCopyMethod(WithPrimitiveArrays::class.java).get();
152+
assertThat(copy.syntheticCopyMethod.toString()).contains("(org.springframework.data.mapping.model.WithPrimitiveArrays,int[],int[],org.springframework.data.mapping.model.PrimitiveNullableArrayValue,int[],int[],int,java.lang.Object)")
153+
154+
val parameters = copy.copyFunction.parameters;
155+
156+
val pa = KotlinValueUtils.getConstructorValueHierarchy(parameters[1]);
157+
assertThat(pa.actualType).isEqualTo(IntArray::class.java)
158+
assertThat(pa.appliesBoxing()).isFalse
159+
160+
val pan = KotlinValueUtils.getConstructorValueHierarchy(parameters[2]);
161+
assertThat(pan.actualType).isEqualTo(IntArray::class.java)
162+
assertThat(pan.appliesBoxing()).isFalse
163+
164+
val pna = KotlinValueUtils.getConstructorValueHierarchy(parameters[3]);
165+
assertThat(pna.actualType).isEqualTo(IntArray::class.java)
166+
assertThat(pna.parameterType).isEqualTo(PrimitiveNullableArrayValue::class.java)
167+
assertThat(pna.appliesBoxing()).isTrue
168+
169+
val pad = KotlinValueUtils.getConstructorValueHierarchy(parameters[4]);
170+
assertThat(pad.actualType).isEqualTo(IntArray::class.java)
171+
assertThat(pad.appliesBoxing()).isFalse
172+
173+
val pand = KotlinValueUtils.getConstructorValueHierarchy(parameters[5]);
174+
assertThat(pand.actualType).isEqualTo(IntArray::class.java)
175+
assertThat(pand.appliesBoxing()).isFalse
176+
}
177+
178+
@Test // GH-1947
179+
internal fun inlinesPrimitiveArrayConstructorRules() {
180+
181+
val ctor = WithPrimitiveArrays::class.constructors.iterator().next();
182+
assertThat(ctor.javaConstructor.toString()).contains("(int[],int[],int[],int[],int[],kotlin.jvm.internal.DefaultConstructorMarker)")
183+
184+
val iterator = ctor.parameters.iterator()
185+
186+
val pa = KotlinValueUtils.getConstructorValueHierarchy(iterator.next());
187+
assertThat(pa.actualType).isEqualTo(IntArray::class.java)
188+
assertThat(pa.appliesBoxing()).isFalse
189+
190+
val pan = KotlinValueUtils.getConstructorValueHierarchy(iterator.next());
191+
assertThat(pan.actualType).isEqualTo(IntArray::class.java)
192+
assertThat(pan.appliesBoxing()).isFalse
193+
194+
val pna = KotlinValueUtils.getConstructorValueHierarchy(iterator.next());
195+
assertThat(pna.actualType).isEqualTo(IntArray::class.java)
196+
assertThat(pna.appliesBoxing()).isFalse
197+
198+
val pad = KotlinValueUtils.getConstructorValueHierarchy(iterator.next());
199+
assertThat(pad.actualType).isEqualTo(IntArray::class.java)
200+
assertThat(pad.appliesBoxing()).isFalse
201+
202+
val pand = KotlinValueUtils.getConstructorValueHierarchy(iterator.next());
203+
assertThat(pand.actualType).isEqualTo(IntArray::class.java)
204+
assertThat(pand.appliesBoxing()).isFalse
205+
}
206+
148207
@Test // GH-1947
149208
internal fun inlinesGenericTypesConstructorRules() {
150209

0 commit comments

Comments
 (0)