From 3b35d4699a09452007852bc906be92155378fcca Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 30 Mar 2023 10:08:28 +0200 Subject: [PATCH 01/11] Prepare issue branch --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8174e38303..06c2edd3b3 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 3.2.0-SNAPSHOT + 3.2.0-GH-2806-SNAPSHOT Spring Data Core Core Spring concepts underpinning every Spring Data module. From 1ab4435fde78763feec6aee266a5d2a0ab10c296 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 30 Mar 2023 11:13:09 +0200 Subject: [PATCH 02/11] Hacking around value classes instantiation. Reflection-based instantiation is still a TODO --- ...tlinClassGeneratingEntityInstantiator.java | 139 ++++-- .../data/mapping/model/KotlinCopyMethod.java | 4 +- .../data/mapping/model/KotlinDefaultMask.java | 62 ++- .../model/PreferredConstructorDiscoverer.java | 11 + ...ssGeneratingEntityInstantiatorUnitTests.kt | 408 ++++++++++-------- 5 files changed, 418 insertions(+), 206 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java index 8c29fab1c4..6c979582fe 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java @@ -15,13 +15,20 @@ */ package org.springframework.data.mapping.model; +import static org.springframework.data.mapping.model.KotlinClassGeneratingEntityInstantiator.DefaultingKotlinConstructorResolver.*; + +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KClass; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; import kotlin.reflect.jvm.ReflectJvmMapping; import java.lang.reflect.Constructor; +import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.Function; import java.util.stream.IntStream; import org.springframework.data.mapping.InstanceCreatorMetadata; @@ -53,14 +60,14 @@ protected EntityInstantiator doCreateEntityInstantiator(PersistentEntity e && creator instanceof PreferredConstructor constructor) { PreferredConstructor> defaultConstructor = new DefaultingKotlinConstructorResolver( - entity) - .getDefaultConstructor(); + entity).getDefaultConstructor(); if (defaultConstructor != null) { ObjectInstantiator instantiator = createObjectInstantiator(entity, defaultConstructor); - return new DefaultingKotlinClassInstantiatorAdapter(instantiator, constructor); + return new DefaultingKotlinClassInstantiatorAdapter(instantiator, constructor, + defaultConstructor.getConstructor()); } } @@ -95,12 +102,13 @@ static class DefaultingKotlinConstructorResolver { @Nullable private static Constructor resolveDefaultConstructor(PersistentEntity entity) { - if (!(entity.getInstanceCreatorMetadata() instanceof PreferredConstructor persistenceConstructor)) { + if (!(entity.getInstanceCreatorMetadata()instanceof PreferredConstructor persistenceConstructor)) { return null; } Constructor hit = null; - Constructor constructor = persistenceConstructor.getConstructor(); + Constructor detectedConstructor = persistenceConstructor.getConstructor(); + KFunction kotlinFunction = ReflectJvmMapping.getKotlinFunction(detectedConstructor); for (Constructor candidate : entity.getType().getDeclaredConstructors()) { @@ -109,37 +117,85 @@ private static Constructor resolveDefaultConstructor(PersistentEntity e continue; } - // candidates must contain at least two additional parameters (int, DefaultConstructorMarker). - // Number of defaulting masks derives from the original constructor arg count - int syntheticParameters = KotlinDefaultMask.getMaskCount(constructor.getParameterCount()) - + /* DefaultConstructorMarker */ 1; + java.lang.reflect.Parameter[] detectedConstructorParameters = detectedConstructor.getParameters(); + java.lang.reflect.Parameter[] candidateParameters = candidate.getParameters(); - if ((constructor.getParameterCount() + syntheticParameters) != candidate.getParameterCount()) { - continue; - } + if (!hasDefaultConstructorMarker(detectedConstructorParameters)) { - java.lang.reflect.Parameter[] constructorParameters = constructor.getParameters(); - java.lang.reflect.Parameter[] candidateParameters = candidate.getParameters(); + // candidates must contain at least two additional parameters (int, DefaultConstructorMarker). + // Number of defaulting masks derives from the original constructor arg count + int syntheticParameters = KotlinDefaultMask.getMaskCount(detectedConstructor.getParameterCount()) + + /* DefaultConstructorMarker */ 1; - if (!candidateParameters[candidateParameters.length - 1].getType().getName() - .equals("kotlin.jvm.internal.DefaultConstructorMarker")) { + if ((detectedConstructor.getParameterCount() + syntheticParameters) != candidate.getParameterCount()) { + continue; + } + } else { + + int optionalParameterCount = (int) kotlinFunction.getParameters().stream().filter(it -> it.isOptional()) + .count(); + int syntheticParameters = KotlinDefaultMask.getExactMaskCount(optionalParameterCount); + + if ((detectedConstructor.getParameterCount() + syntheticParameters) != candidate.getParameterCount()) { + continue; + } + } + + if (!hasDefaultConstructorMarker(candidateParameters)) { continue; } - if (parametersMatch(constructorParameters, candidateParameters)) { + int userParameterCount = kotlinFunction != null ? kotlinFunction.getParameters().size() + : detectedConstructor.getParameterCount(); + if (parametersMatch(detectedConstructorParameters, candidateParameters, userParameterCount)) { hit = candidate; - break; } } return hit; } + static boolean hasDefaultConstructorMarker(java.lang.reflect.Parameter[] parameters) { + + return parameters.length > 0 && parameters[parameters.length - 1].getType().getName() + .equals("kotlin.jvm.internal.DefaultConstructorMarker"); + } + private static boolean parametersMatch(java.lang.reflect.Parameter[] constructorParameters, - java.lang.reflect.Parameter[] candidateParameters) { + java.lang.reflect.Parameter[] candidateParameters, int userParameterCount) { - return IntStream.range(0, constructorParameters.length) - .allMatch(i -> constructorParameters[i].getType().equals(candidateParameters[i].getType())); + return IntStream.range(0, userParameterCount) + .allMatch(i -> parametersMatch(constructorParameters[i], candidateParameters[i])); + } + + static boolean parametersMatch(java.lang.reflect.Parameter constructorParameter, + java.lang.reflect.Parameter candidateParameter) { + + if (constructorParameter.getType().equals(candidateParameter.getType())) { + return true; + } + + // candidate can be also a wrapper + + Class aClass = unwrapValueClass(candidateParameter.getType()); + + return constructorParameter.getType().equals(aClass); + } + + private static Class unwrapValueClass(Class type) { + + KClass kotlinClass = JvmClassMappingKt.getKotlinClass(type); + if (kotlinClass != null && kotlinClass.isValue()) { + + KFunction next = kotlinClass.getConstructors().iterator().next(); + KParameter kParameter = next.getParameters().get(0); + Type javaType = ReflectJvmMapping.getJavaType(kParameter.getType()); + if (javaType instanceof Class) { + return unwrapValueClass((Class) javaType); + } + } + + return type; } @Nullable @@ -171,21 +227,39 @@ static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantia private final ObjectInstantiator instantiator; private final KFunction constructor; private final List kParameters; - private final Constructor synthetic; + private final List> wrappers = new ArrayList<>(); + private final Constructor constructorToInvoke; + private final boolean hasDefaultConstructorMarker; - DefaultingKotlinClassInstantiatorAdapter(ObjectInstantiator instantiator, PreferredConstructor constructor) { + DefaultingKotlinClassInstantiatorAdapter(ObjectInstantiator instantiator, + PreferredConstructor defaultConstructor, Constructor constructorToInvoke) { - KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(constructor.getConstructor()); + KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(defaultConstructor.getConstructor()); if (kotlinConstructor == null) { throw new IllegalArgumentException( - "No corresponding Kotlin constructor found for " + constructor.getConstructor()); + "No corresponding Kotlin constructor found for " + defaultConstructor.getConstructor()); } this.instantiator = instantiator; this.constructor = kotlinConstructor; + this.hasDefaultConstructorMarker = hasDefaultConstructorMarker(constructorToInvoke.getParameters()); this.kParameters = kotlinConstructor.getParameters(); - this.synthetic = constructor.getConstructor(); + this.constructorToInvoke = constructorToInvoke; + + for (KParameter kParameter : kParameters) { + + if (kParameter.getType().getClassifier()instanceof KClass kc && kc.isValue() && kParameter.isOptional()) { + + // using reflection to construct a value class wrapper. Everything + // else would require too many levels of indirections. + wrappers.add(o -> { + return kc.getConstructors().iterator().next().call(o); + }); + } else { + wrappers.add(Function.identity()); + } + } } @Override @@ -209,8 +283,10 @@ private

, T> Object[] extractInvocationArguments throw new IllegalArgumentException("EntityCreator must not be null"); } - Object[] params = allocateArguments(synthetic.getParameterCount() - + KotlinDefaultMask.getMaskCount(synthetic.getParameterCount()) + /* DefaultConstructorMarker */1); + Object[] params = allocateArguments(hasDefaultConstructorMarker ? constructorToInvoke.getParameterCount() + : (constructorToInvoke.getParameterCount() + + KotlinDefaultMask.getMaskCount(constructorToInvoke.getParameterCount()) + + /* DefaultConstructorMarker */1)); int userParameterCount = kParameters.size(); List> parameters = entityCreator.getParameters(); @@ -222,7 +298,7 @@ private

, T> Object[] extractInvocationArguments params[i] = provider.getParameterValue(parameter); } - KotlinDefaultMask defaultMask = KotlinDefaultMask.from(constructor, it -> { + KotlinDefaultMask defaultMask = KotlinDefaultMask.forConstructor(constructor, it -> { int index = kParameters.indexOf(it); @@ -241,6 +317,11 @@ private

, T> Object[] extractInvocationArguments return true; }); + // late rewrapping to indicate potential absence of parameters for defaulting + for (int i = 0; i < userParameterCount; i++) { + params[i] = wrappers.get(i).apply(params[i]); + } + int[] defaulting = defaultMask.getDefaulting(); // append nullability masks to creation arguments for (int i = 0; i < defaulting.length; i++) { diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java index d973ded20d..4b7a22d4b9 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java @@ -244,7 +244,7 @@ private static boolean matchesPrimaryConstructor(Class[] parameterTypes, KFun List constructorArguments = getComponentArguments(primaryConstructor); - int defaultingArgs = KotlinDefaultMask.from(primaryConstructor, kParameter -> false).getDefaulting().length; + int defaultingArgs = KotlinDefaultMask.forCopy(primaryConstructor, kParameter -> false).getDefaulting().length; if (parameterTypes.length != 1 /* $this */ + constructorArguments.size() + defaultingArgs + 1 /* object marker */) { return false; @@ -297,7 +297,7 @@ static class KotlinCopyByProperty { this.parameterPosition = findIndex(copyFunction, property.getName()); this.parameterCount = copyFunction.getParameters().size(); - this.defaultMask = KotlinDefaultMask.from(copyFunction, it -> property.getName().equals(it.getName())); + this.defaultMask = KotlinDefaultMask.forCopy(copyFunction, it -> property.getName().equals(it.getName())); } static int findIndex(KFunction function, String parameterName) { diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinDefaultMask.java b/src/main/java/org/springframework/data/mapping/model/KotlinDefaultMask.java index d7aeadf5a3..41a6e591f9 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinDefaultMask.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinDefaultMask.java @@ -54,12 +54,23 @@ public void forEach(IntConsumer maskCallback) { * Return the number of defaulting masks required to represent the number of {@code arguments}. * * @param arguments number of method arguments. - * @return the number of defaulting masks required. + * @return the number of defaulting masks required. Returns at least one to be used with {@code copy} methods. */ public static int getMaskCount(int arguments) { return ((arguments - 1) / Integer.SIZE) + 1; } + /** + * Return the number of defaulting masks required to represent the number of {@code optionalParameterCount}. + * In contrast to {@link #getMaskCount(int)}, this method can return zero if there are no optional parameters available. + * + * @param optionalParameterCount number of method arguments. + * @return the number of defaulting masks required. Returns zero if no optional parameters are available. + */ + static int getExactMaskCount(int optionalParameterCount) { + return optionalParameterCount == 0 ? 0 : getMaskCount(optionalParameterCount); + } + /** * Creates defaulting mask(s) used to invoke Kotlin {@literal default} methods that conditionally apply parameter * values. @@ -69,10 +80,47 @@ public static int getMaskCount(int arguments) { * @return {@link KotlinDefaultMask}. */ public static KotlinDefaultMask from(KFunction function, Predicate isPresent) { + return forCopy(function, isPresent); + } + + /** + * Creates defaulting mask(s) used to invoke Kotlin {@literal copy} methods that conditionally apply parameter values. + * + * @param function the {@link KFunction} that should be invoked. + * @param isPresent {@link Predicate} for the presence/absence of parameters. + * @return {@link KotlinDefaultMask}. + */ + static KotlinDefaultMask forCopy(KFunction function, Predicate isPresent) { + return from(function, isPresent, true); + } + + /** + * Creates defaulting mask(s) used to invoke Kotlin constructors where a defaulting mask isn't required unless there's + * one nullable argument. + * + * @param function the {@link KFunction} that should be invoked. + * @param isPresent {@link Predicate} for the presence/absence of parameters. + * @return {@link KotlinDefaultMask}. + */ + static KotlinDefaultMask forConstructor(KFunction function, Predicate isPresent) { + return from(function, isPresent, false); + } + + /** + * Creates defaulting mask(s) used to invoke Kotlin {@literal default} methods that conditionally apply parameter + * values. + * + * @param function the {@link KFunction} that should be invoked. + * @param isPresent {@link Predicate} for the presence/absence of parameters. + * @return {@link KotlinDefaultMask}. + */ + private static KotlinDefaultMask from(KFunction function, Predicate isPresent, + boolean requiresAtLeastOneMask) { List masks = new ArrayList<>(); int index = 0; int mask = 0; + boolean hasSeenParameter = false; List parameters = function.getParameters(); @@ -83,8 +131,12 @@ public static KotlinDefaultMask from(KFunction function, Predicate function, Predicate i).toArray()); } diff --git a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java index 73e99d225b..0e5cfda24f 100644 --- a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java +++ b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Optional; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.ParameterNameDiscoverer; @@ -179,6 +180,16 @@ > PreferredConstructor discover(TypeInf Class rawOwningType = type.getType(); + + // Kotlin can rewrite annotated constructors into synthetic ones so we need to inspect that first. + Optional> first = Arrays.stream(rawOwningType.getDeclaredConstructors()) // + .filter(it -> it.isSynthetic() && AnnotationUtils.findAnnotation(it, PersistenceCreator.class) != null) + .map(it -> buildPreferredConstructor(it, type, entity)).findFirst(); + + if(first.isPresent()){ + return first.get(); + } + return Arrays.stream(rawOwningType.getDeclaredConstructors()) // .filter(it -> !it.isSynthetic()) // Synthetic constructors should not be considered // Explicitly defined creator trumps all 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 4d03649bf7..92e9e70cd4 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,10 @@ 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.PersistenceCreator import org.springframework.data.mapping.PersistentEntity import org.springframework.data.mapping.context.SamplePersistentProperty +import kotlin.reflect.KClass /** * Unit tests for [KotlinClassGeneratingEntityInstantiator] creating instances using Kotlin data classes. @@ -33,176 +35,240 @@ import org.springframework.data.mapping.context.SamplePersistentProperty @Suppress("UNCHECKED_CAST") class KotlinClassGeneratingEntityInstantiatorUnitTests { - val provider = mockk>() - - @Test // DATACMNS-1126 - fun `should create instance`() { - - val entity = mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - Contact::class.java - ) - - every { provider.getParameterValue(any()) }.returnsMany("Walter", "White") - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - val instance: Contact = - KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - - assertThat(instance.firstname).isEqualTo("Walter") - assertThat(instance.lastname).isEqualTo("White") - } - - @Test // DATACMNS-1126 - fun `should create instance and fill in defaults`() { - - val entity = - mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - ContactWithDefaulting::class.java - ) - - every { provider.getParameterValue(any()) }.returnsMany( - "Walter", null, "Skyler", null, null, null, null, null, null, null, /* 0-9 */ - null, null, null, null, null, null, null, null, null, null, /* 10-19 */ - null, null, null, null, null, null, null, null, null, null, /* 20-29 */ - null, "Walter", null, "Junior", null - ) - - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - val instance: ContactWithDefaulting = KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - - assertThat(instance.prop0).isEqualTo("Walter") - assertThat(instance.prop2).isEqualTo("Skyler") - assertThat(instance.prop31).isEqualTo("Walter") - assertThat(instance.prop32).isEqualTo("White") - assertThat(instance.prop33).isEqualTo("Junior") - assertThat(instance.prop34).isEqualTo("White") - } - - @Test // DATACMNS-1200 - fun `absent primitive value should cause MappingInstantiationException`() { - - val entity = mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - WithBoolean::class.java - ) - - every { provider.getParameterValue(any()) } returns null - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - assertThatThrownBy { - KotlinClassGeneratingEntityInstantiator().createInstance( - entity, - provider - ) - } // - .isInstanceOf(MappingInstantiationException::class.java) // - .hasMessageContaining("init") // - .hasMessageContaining("kotlin.Boolean") // - .hasCauseInstanceOf(IllegalArgumentException::class.java) - } - - @Test // DATACMNS-1200 - fun `should apply primitive defaulting for absent parameters`() { - - val entity = - mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - WithPrimitiveDefaulting::class.java - ) - - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - val instance: WithPrimitiveDefaulting = KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - - assertThat(instance.aByte).isEqualTo(0) - assertThat(instance.aShort).isEqualTo(0) - assertThat(instance.anInt).isEqualTo(0) - assertThat(instance.aLong).isEqualTo(0L) - assertThat(instance.aFloat).isEqualTo(0.0f) - assertThat(instance.aDouble).isEqualTo(0.0) - assertThat(instance.aChar).isEqualTo('a') - assertThat(instance.aBool).isTrue() - } - - @Test // DATACMNS-1338 - fun `should create instance using @PersistenceConstructor`() { - - val entity = mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - CustomUser::class.java - ) - - every { provider.getParameterValue(any()) } returns "Walter" - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - val instance: CustomUser = - KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - - assertThat(instance.id).isEqualTo("Walter") - } - - data class Contact(val firstname: String, val lastname: String) - - data class ContactWithDefaulting(val prop0: String, val prop1: String = "White", val prop2: String, - val prop3: String = "White", val prop4: String = "White", val prop5: String = "White", - val prop6: String = "White", val prop7: String = "White", val prop8: String = "White", - val prop9: String = "White", val prop10: String = "White", val prop11: String = "White", - val prop12: String = "White", val prop13: String = "White", val prop14: String = "White", - val prop15: String = "White", val prop16: String = "White", val prop17: String = "White", - val prop18: String = "White", val prop19: String = "White", val prop20: String = "White", - val prop21: String = "White", val prop22: String = "White", val prop23: String = "White", - val prop24: String = "White", val prop25: String = "White", val prop26: String = "White", - val prop27: String = "White", val prop28: String = "White", val prop29: String = "White", - val prop30: String = "White", val prop31: String = "White", val prop32: String = "White", - val prop33: String, val prop34: String = "White" - ) - - data class WithBoolean(val state: Boolean) - - data class WithPrimitiveDefaulting(val aByte: Byte = 0, val aShort: Short = 0, val anInt: Int = 0, val aLong: Long = 0L, - val aFloat: Float = 0.0f, val aDouble: Double = 0.0, val aChar: Char = 'a', val aBool: Boolean = true) - - data class ContactWithPersistenceConstructor(val firstname: String, val lastname: String) { - - @PersistenceConstructor - constructor(firstname: String) : this(firstname, "") - } - - data class CustomUser( - var id: String? = null, - - var organisations: MutableList = mutableListOf() - ) { - @PersistenceConstructor - constructor(id: String?) : this(id, mutableListOf()) - } - - data class Organisation(var id: String? = null) + val provider = mockk>() + + @Test // DATACMNS-1126 + fun `should create instance`() { + + val entity = mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + Contact::class.java + ) + + every { provider.getParameterValue(any()) }.returnsMany("Walter", "White") + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + val instance: Contact = + KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + assertThat(instance.firstname).isEqualTo("Walter") + assertThat(instance.lastname).isEqualTo("White") + } + + @Test // DATACMNS-1126 + fun `should create instance and fill in defaults`() { + + val entity = + mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + ContactWithDefaulting::class.java + ) + + every { provider.getParameterValue(any()) }.returnsMany( + "Walter", null, "Skyler", null, null, null, null, null, null, null, /* 0-9 */ + null, null, null, null, null, null, null, null, null, null, /* 10-19 */ + null, null, null, null, null, null, null, null, null, null, /* 20-29 */ + null, "Walter", null, "Junior", null + ) + + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + val instance: ContactWithDefaulting = KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + assertThat(instance.prop0).isEqualTo("Walter") + assertThat(instance.prop2).isEqualTo("Skyler") + assertThat(instance.prop31).isEqualTo("Walter") + assertThat(instance.prop32).isEqualTo("White") + assertThat(instance.prop33).isEqualTo("Junior") + assertThat(instance.prop34).isEqualTo("White") + } + + @Test // DATACMNS-1200 + fun `absent primitive value should cause MappingInstantiationException`() { + + val entity = mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + WithBoolean::class.java + ) + + every { provider.getParameterValue(any()) } returns null + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + assertThatThrownBy { + KotlinClassGeneratingEntityInstantiator().createInstance( + entity, + provider + ) + } // + .isInstanceOf(MappingInstantiationException::class.java) // + .hasMessageContaining("init") // + .hasMessageContaining("kotlin.Boolean") // + .hasCauseInstanceOf(IllegalArgumentException::class.java) + } + + @Test // DATACMNS-1200 + fun `should apply primitive defaulting for absent parameters`() { + + val entity = + mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + WithPrimitiveDefaulting::class.java + ) + + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + val instance: WithPrimitiveDefaulting = + KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + assertThat(instance.aByte).isEqualTo(0) + assertThat(instance.aShort).isEqualTo(0) + assertThat(instance.anInt).isEqualTo(0) + assertThat(instance.aLong).isEqualTo(0L) + assertThat(instance.aFloat).isEqualTo(0.0f) + assertThat(instance.aDouble).isEqualTo(0.0) + assertThat(instance.aChar).isEqualTo('a') + assertThat(instance.aBool).isTrue() + } + + @Test // DATACMNS-1338 + fun `should create instance using @PersistenceConstructor`() { + + every { provider.getParameterValue(any()) } returns "Walter" + val instance = construct(CustomUser::class) + + assertThat(instance.id).isEqualTo("Walter") + } + + @Test + fun `should use default constructor for types using value class`() { + + every { provider.getParameterValue(any()) } returns "hello" + val instance = construct(WithMyValueClass::class) + + assertThat(instance.id.id).isEqualTo("hello") + } + + @Test + fun `should use default constructor for types using nullable value class`() { + + every { provider.getParameterValue(any()) } returns null + + val instance = construct(WithNestedMyNullableInlineClass::class) + + assertThat(instance.id?.id?.id).isEqualTo("foo") + assertThat(instance.baz?.id).isEqualTo("id") + } + + private fun construct(typeToCreate : KClass): T { + + val entity = + mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + typeToCreate.java + ) + + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + return KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + } + + data class Contact(val firstname: String, val lastname: String) + + data class ContactWithDefaulting( + val prop0: String, val prop1: String = "White", val prop2: String, + val prop3: String = "White", val prop4: String = "White", val prop5: String = "White", + val prop6: String = "White", val prop7: String = "White", val prop8: String = "White", + val prop9: String = "White", val prop10: String = "White", val prop11: String = "White", + val prop12: String = "White", val prop13: String = "White", val prop14: String = "White", + val prop15: String = "White", val prop16: String = "White", val prop17: String = "White", + val prop18: String = "White", val prop19: String = "White", val prop20: String = "White", + val prop21: String = "White", val prop22: String = "White", val prop23: String = "White", + val prop24: String = "White", val prop25: String = "White", val prop26: String = "White", + val prop27: String = "White", val prop28: String = "White", val prop29: String = "White", + val prop30: String = "White", val prop31: String = "White", val prop32: String = "White", + val prop33: String, val prop34: String = "White" + ) + + data class WithBoolean(val state: Boolean) + + data class WithPrimitiveDefaulting( + val aByte: Byte = 0, val aShort: Short = 0, val anInt: Int = 0, val aLong: Long = 0L, + val aFloat: Float = 0.0f, val aDouble: Double = 0.0, val aChar: Char = 'a', val aBool: Boolean = true + ) + + data class ContactWithPersistenceConstructor(val firstname: String, val lastname: String) { + + @PersistenceConstructor + constructor(firstname: String) : this(firstname, "") + } + + data class CustomUser( + var id: String? = null, + + var organisations: MutableList = mutableListOf() + ) { + @PersistenceConstructor + constructor(id: String?) : this(id, mutableListOf()) + } + + data class Organisation(var id: String? = null) + + @JvmInline + value class MyInlineClass(val id: String) + + data class WithMyValueClass(val id: MyInlineClass) + + @JvmInline + value class MyNullableInlineClass(val id: String? = "id") + + @JvmInline + value class MyNestedNullableInlineClass(val id: MyNullableInlineClass) + + class WithNestedMyNullableInlineClass( + val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( + MyNullableInlineClass("foo") + ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") + ) + + class WithNestedAndPersistenceConstructorNullableInlineClass( + val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( + MyNullableInlineClass("foo") + ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") + ) { + + @PersistenceCreator + constructor(id: MyNestedNullableInlineClass?, baz: MyNullableInlineClass?=MyNullableInlineClass("aaa"), s: String) : this(id, baz) { + println(s) + } + + constructor(id: MyNestedNullableInlineClass, baz: MyNullableInlineClass, i: Int) : this(id, baz) { + } + + } + + } From 783d431339fd6c0df4397ef077b1e4662107ebc7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 17 Apr 2023 14:14:26 +0200 Subject: [PATCH 03/11] Support reflective Kotlin inline class instantiation. Also, refactor common code into kotlin instantiation metadata utility. --- ...tlinClassGeneratingEntityInstantiator.java | 245 +-------------- .../model/KotlinInstantiationDelegate.java | 289 ++++++++++++++++++ .../model/ReflectionEntityInstantiator.java | 39 +++ ...ReflectionEntityInstantiatorUnitTests.java | 2 +- ...ssGeneratingEntityInstantiatorUnitTests.kt | 17 -- ...ionEntityInstantiatorDataClassUnitTests.kt | 37 ++- ...nEntityInstantiatorInlineClassUnitTests.kt | 104 +++++++ 7 files changed, 460 insertions(+), 273 deletions(-) create mode 100644 src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java create mode 100644 src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java index 6c979582fe..f5570c49f6 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiator.java @@ -15,30 +15,14 @@ */ package org.springframework.data.mapping.model; -import static org.springframework.data.mapping.model.KotlinClassGeneratingEntityInstantiator.DefaultingKotlinConstructorResolver.*; - -import kotlin.jvm.JvmClassMappingKt; -import kotlin.reflect.KClass; -import kotlin.reflect.KFunction; -import kotlin.reflect.KParameter; -import kotlin.reflect.jvm.ReflectJvmMapping; - import java.lang.reflect.Constructor; -import java.lang.reflect.Type; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; -import java.util.function.Function; -import java.util.stream.IntStream; import org.springframework.data.mapping.InstanceCreatorMetadata; -import org.springframework.data.mapping.Parameter; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.util.KotlinReflectionUtils; -import org.springframework.data.util.ReflectionUtils; -import org.springframework.lang.Nullable; /** * Kotlin-specific extension to {@link ClassGeneratingEntityInstantiator} that adapts Kotlin constructors with @@ -59,151 +43,21 @@ protected EntityInstantiator doCreateEntityInstantiator(PersistentEntity e if (KotlinReflectionUtils.isSupportedKotlinClass(entity.getType()) && creator instanceof PreferredConstructor constructor) { - PreferredConstructor> defaultConstructor = new DefaultingKotlinConstructorResolver( - entity).getDefaultConstructor(); + PreferredConstructor> kotlinJvmConstructor = KotlinInstantiationDelegate + .resolveKotlinJvmConstructor(constructor); - if (defaultConstructor != null) { + if (kotlinJvmConstructor != null) { - ObjectInstantiator instantiator = createObjectInstantiator(entity, defaultConstructor); + ObjectInstantiator instantiator = createObjectInstantiator(entity, kotlinJvmConstructor); return new DefaultingKotlinClassInstantiatorAdapter(instantiator, constructor, - defaultConstructor.getConstructor()); + kotlinJvmConstructor.getConstructor()); } } return super.doCreateEntityInstantiator(entity); } - /** - * 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}. - * - * @since 2.0 - * @author Mark Paluch - */ - static class DefaultingKotlinConstructorResolver { - - private final @Nullable PreferredConstructor defaultConstructor; - - @SuppressWarnings("unchecked") - DefaultingKotlinConstructorResolver(PersistentEntity entity) { - - Constructor hit = resolveDefaultConstructor(entity); - InstanceCreatorMetadata> creator = entity.getInstanceCreatorMetadata(); - - if ((hit != null) && creator instanceof PreferredConstructor persistenceConstructor) { - this.defaultConstructor = new PreferredConstructor<>(hit, - persistenceConstructor.getParameters().toArray(new Parameter[0])); - } else { - this.defaultConstructor = null; - } - } - - @Nullable - private static Constructor resolveDefaultConstructor(PersistentEntity entity) { - - if (!(entity.getInstanceCreatorMetadata()instanceof PreferredConstructor persistenceConstructor)) { - return null; - } - - Constructor hit = null; - Constructor detectedConstructor = persistenceConstructor.getConstructor(); - KFunction kotlinFunction = ReflectJvmMapping.getKotlinFunction(detectedConstructor); - - for (Constructor candidate : entity.getType().getDeclaredConstructors()) { - - // use only synthetic constructors - if (!candidate.isSynthetic()) { - continue; - } - - java.lang.reflect.Parameter[] detectedConstructorParameters = detectedConstructor.getParameters(); - java.lang.reflect.Parameter[] candidateParameters = candidate.getParameters(); - - if (!hasDefaultConstructorMarker(detectedConstructorParameters)) { - - // candidates must contain at least two additional parameters (int, DefaultConstructorMarker). - // Number of defaulting masks derives from the original constructor arg count - int syntheticParameters = KotlinDefaultMask.getMaskCount(detectedConstructor.getParameterCount()) - + /* DefaultConstructorMarker */ 1; - - if ((detectedConstructor.getParameterCount() + syntheticParameters) != candidate.getParameterCount()) { - continue; - } - } else { - - int optionalParameterCount = (int) kotlinFunction.getParameters().stream().filter(it -> it.isOptional()) - .count(); - int syntheticParameters = KotlinDefaultMask.getExactMaskCount(optionalParameterCount); - - if ((detectedConstructor.getParameterCount() + syntheticParameters) != candidate.getParameterCount()) { - continue; - } - } - - if (!hasDefaultConstructorMarker(candidateParameters)) { - continue; - } - - int userParameterCount = kotlinFunction != null ? kotlinFunction.getParameters().size() - : detectedConstructor.getParameterCount(); - if (parametersMatch(detectedConstructorParameters, candidateParameters, userParameterCount)) { - hit = candidate; - } - } - - return hit; - } - - static boolean hasDefaultConstructorMarker(java.lang.reflect.Parameter[] parameters) { - - return parameters.length > 0 && parameters[parameters.length - 1].getType().getName() - .equals("kotlin.jvm.internal.DefaultConstructorMarker"); - } - - private static boolean parametersMatch(java.lang.reflect.Parameter[] constructorParameters, - java.lang.reflect.Parameter[] candidateParameters, int userParameterCount) { - - return IntStream.range(0, userParameterCount) - .allMatch(i -> parametersMatch(constructorParameters[i], candidateParameters[i])); - } - - static boolean parametersMatch(java.lang.reflect.Parameter constructorParameter, - java.lang.reflect.Parameter candidateParameter) { - - if (constructorParameter.getType().equals(candidateParameter.getType())) { - return true; - } - - // candidate can be also a wrapper - - Class aClass = unwrapValueClass(candidateParameter.getType()); - - return constructorParameter.getType().equals(aClass); - } - - private static Class unwrapValueClass(Class type) { - - KClass kotlinClass = JvmClassMappingKt.getKotlinClass(type); - if (kotlinClass != null && kotlinClass.isValue()) { - - KFunction next = kotlinClass.getConstructors().iterator().next(); - KParameter kParameter = next.getParameters().get(0); - Type javaType = ReflectJvmMapping.getJavaType(kParameter.getType()); - if (javaType instanceof Class) { - return unwrapValueClass((Class) javaType); - } - } - - return type; - } - - @Nullable - PreferredConstructor getDefaultConstructor() { - return defaultConstructor; - } - } - /** * Entity instantiator for Kotlin constructors that apply parameter defaulting. Kotlin constructors that apply * argument defaulting are marked with {@link kotlin.jvm.internal.DefaultConstructorMarker} and accept additional @@ -225,41 +79,13 @@ private static Class unwrapValueClass(Class type) { static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantiator { private final ObjectInstantiator instantiator; - private final KFunction constructor; - private final List kParameters; - private final List> wrappers = new ArrayList<>(); - private final Constructor constructorToInvoke; - private final boolean hasDefaultConstructorMarker; + private final KotlinInstantiationDelegate delegate; DefaultingKotlinClassInstantiatorAdapter(ObjectInstantiator instantiator, PreferredConstructor defaultConstructor, Constructor constructorToInvoke) { - KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(defaultConstructor.getConstructor()); - - if (kotlinConstructor == null) { - throw new IllegalArgumentException( - "No corresponding Kotlin constructor found for " + defaultConstructor.getConstructor()); - } - this.instantiator = instantiator; - this.constructor = kotlinConstructor; - this.hasDefaultConstructorMarker = hasDefaultConstructorMarker(constructorToInvoke.getParameters()); - this.kParameters = kotlinConstructor.getParameters(); - this.constructorToInvoke = constructorToInvoke; - - for (KParameter kParameter : kParameters) { - - if (kParameter.getType().getClassifier()instanceof KClass kc && kc.isValue() && kParameter.isOptional()) { - - // using reflection to construct a value class wrapper. Everything - // else would require too many levels of indirections. - wrappers.add(o -> { - return kc.getConstructors().iterator().next().call(o); - }); - } else { - wrappers.add(Function.identity()); - } - } + this.delegate = new KotlinInstantiationDelegate(defaultConstructor, constructorToInvoke); } @Override @@ -267,7 +93,8 @@ static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantia public , P extends PersistentProperty

> T createInstance(E entity, ParameterValueProvider

provider) { - Object[] params = extractInvocationArguments(entity.getInstanceCreatorMetadata(), provider); + Object[] params = allocateArguments(delegate.getRequiredParameterCount()); + delegate.extractInvocationArguments(params, entity.getInstanceCreatorMetadata(), provider); try { return (T) instantiator.newInstance(params); @@ -276,59 +103,5 @@ public , P extends PersistentPrope } } - private

, T> Object[] extractInvocationArguments( - @Nullable InstanceCreatorMetadata

entityCreator, ParameterValueProvider

provider) { - - if (entityCreator == null) { - throw new IllegalArgumentException("EntityCreator must not be null"); - } - - Object[] params = allocateArguments(hasDefaultConstructorMarker ? constructorToInvoke.getParameterCount() - : (constructorToInvoke.getParameterCount() - + KotlinDefaultMask.getMaskCount(constructorToInvoke.getParameterCount()) - + /* DefaultConstructorMarker */1)); - int userParameterCount = kParameters.size(); - - List> parameters = entityCreator.getParameters(); - - // Prepare user-space arguments - for (int i = 0; i < userParameterCount; i++) { - - Parameter parameter = parameters.get(i); - params[i] = provider.getParameterValue(parameter); - } - - KotlinDefaultMask defaultMask = KotlinDefaultMask.forConstructor(constructor, it -> { - - int index = kParameters.indexOf(it); - - Parameter parameter = parameters.get(index); - Class type = parameter.getType().getType(); - - if (it.isOptional() && (params[index] == null)) { - if (type.isPrimitive()) { - - // apply primitive defaulting to prevent NPE on primitive downcast - params[index] = ReflectionUtils.getPrimitiveDefault(type); - } - return false; - } - - return true; - }); - - // late rewrapping to indicate potential absence of parameters for defaulting - for (int i = 0; i < userParameterCount; i++) { - params[i] = wrappers.get(i).apply(params[i]); - } - - int[] defaulting = defaultMask.getDefaulting(); - // append nullability masks to creation arguments - for (int i = 0; i < defaulting.length; i++) { - params[userParameterCount + i] = defaulting[i]; - } - - return params; - } } } diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java new file mode 100644 index 0000000000..20a375bd62 --- /dev/null +++ b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java @@ -0,0 +1,289 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model; + +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KClass; +import kotlin.reflect.KFunction; +import kotlin.reflect.KParameter; +import kotlin.reflect.jvm.ReflectJvmMapping; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Function; +import java.util.stream.IntStream; + +import org.springframework.data.mapping.InstanceCreatorMetadata; +import org.springframework.data.mapping.Parameter; +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.mapping.PreferredConstructor; +import org.springframework.data.util.ReflectionUtils; +import org.springframework.lang.Nullable; + +/** + * Delegate to allocate instantiation arguments and to resolve the actual constructor to call for inline/value class + * instantiation. This class captures all Kotlin-specific quirks, from default constructor resolution up to mangled + * inline-type handling so instantiator-components can delegate to this class for constructor translation and + * constructor argument extraction. + * + * @author Mark Paluch + * @since 3.1 + */ +class KotlinInstantiationDelegate { + + private final KFunction constructor; + private final List kParameters; + private final List> wrappers = new ArrayList<>(); + private final Constructor constructorToInvoke; + private final boolean hasDefaultConstructorMarker; + + public KotlinInstantiationDelegate(PreferredConstructor preferredConstructor, + Constructor constructorToInvoke) { + + KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(preferredConstructor.getConstructor()); + + if (kotlinConstructor == null) { + throw new IllegalArgumentException( + "No corresponding Kotlin constructor found for " + preferredConstructor.getConstructor()); + } + + this.constructor = kotlinConstructor; + this.kParameters = kotlinConstructor.getParameters(); + this.constructorToInvoke = constructorToInvoke; + this.hasDefaultConstructorMarker = hasDefaultConstructorMarker(constructorToInvoke.getParameters()); + + for (KParameter kParameter : kParameters) { + wrappers.add(getWrapper(kParameter, true)); + } + } + + /** + * @param kParameter the kotlin parameter to wrap. + * @param domainTypeUsage optional in the domain type require value type casting. Inner/nested ones don't. This is + * because calling the synthetic constructor with an optional parameter requires an inline class while + * optional parameters via reflection are handled within Kotlin-Reflection. + * @return + */ + private Function getWrapper(KParameter kParameter, boolean domainTypeUsage) { + + if (kParameter.getType().getClassifier()instanceof KClass kc && kc.isValue() + && (!domainTypeUsage || kParameter.isOptional())) { + + KFunction ctor = kc.getConstructors().iterator().next(); + + // using reflection to construct a value class wrapper. Everything + // else would require too many levels of indirections. + + KParameter nested = ctor.getParameters().get(0); + Function wrapper = getWrapper(nested, false); + + return o -> ctor.call(wrapper.apply(o)); + } + + return Function.identity(); + } + + static boolean hasDefaultConstructorMarker(java.lang.reflect.Parameter[] parameters) { + + return parameters.length > 0 + && parameters[parameters.length - 1].getType().getName().equals("kotlin.jvm.internal.DefaultConstructorMarker"); + } + + /** + * @return number of constructor arguments. + */ + public int getRequiredParameterCount() { + + return hasDefaultConstructorMarker ? constructorToInvoke.getParameterCount() + : (constructorToInvoke.getParameterCount() + + KotlinDefaultMask.getMaskCount(constructorToInvoke.getParameterCount()) + + /* DefaultConstructorMarker */1); + } + + /** + * Extract the actual construction arguments for a direct constructor call. + * + * @param params + * @param entityCreator + * @param provider + * @return + * @param

+ */ + public

> Object[] extractInvocationArguments(Object[] params, + @Nullable InstanceCreatorMetadata

entityCreator, ParameterValueProvider

provider) { + + if (entityCreator == null) { + throw new IllegalArgumentException("EntityCreator must not be null"); + } + + int userParameterCount = kParameters.size(); + + List> parameters = entityCreator.getParameters(); + + // Prepare user-space arguments + for (int i = 0; i < userParameterCount; i++) { + + Parameter parameter = parameters.get(i); + params[i] = provider.getParameterValue(parameter); + } + + KotlinDefaultMask defaultMask = KotlinDefaultMask.forConstructor(constructor, it -> { + + int index = kParameters.indexOf(it); + + Parameter parameter = parameters.get(index); + Class type = parameter.getType().getType(); + + if (it.isOptional() && (params[index] == null)) { + if (type.isPrimitive()) { + + // apply primitive defaulting to prevent NPE on primitive downcast + params[index] = ReflectionUtils.getPrimitiveDefault(type); + } + return false; + } + + return true; + }); + + // late rewrapping to indicate potential absence of parameters for defaulting + for (int i = 0; i < userParameterCount; i++) { + params[i] = wrappers.get(i).apply(params[i]); + } + + int[] defaulting = defaultMask.getDefaulting(); + // append nullability masks to creation arguments + for (int i = 0; i < defaulting.length; i++) { + params[userParameterCount + i] = defaulting[i]; + } + + return params; + } + + /** + * 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}. + * + * @since 2.0 + * @author Mark Paluch + */ + + @SuppressWarnings("unchecked") + @Nullable + public static PreferredConstructor resolveKotlinJvmConstructor( + PreferredConstructor preferredConstructor) { + + Constructor hit = doResolveKotlinConstructor(preferredConstructor.getConstructor()); + + if (hit != null) { + return new PreferredConstructor<>(hit, preferredConstructor.getParameters().toArray(new Parameter[0])); + } + + return null; + } + + @Nullable + private static Constructor doResolveKotlinConstructor(Constructor detectedConstructor) { + + Class entityType = detectedConstructor.getDeclaringClass(); + Constructor hit = null; + KFunction kotlinFunction = ReflectJvmMapping.getKotlinFunction(detectedConstructor); + + for (Constructor candidate : entityType.getDeclaredConstructors()) { + + // use only synthetic constructors + if (!candidate.isSynthetic()) { + continue; + } + + java.lang.reflect.Parameter[] detectedConstructorParameters = detectedConstructor.getParameters(); + java.lang.reflect.Parameter[] candidateParameters = candidate.getParameters(); + + if (!KotlinInstantiationDelegate.hasDefaultConstructorMarker(detectedConstructorParameters)) { + + // candidates must contain at least two additional parameters (int, DefaultConstructorMarker). + // Number of defaulting masks derives from the original constructor arg count + int syntheticParameters = KotlinDefaultMask.getMaskCount(detectedConstructor.getParameterCount()) + + /* DefaultConstructorMarker */ 1; + + if ((detectedConstructor.getParameterCount() + syntheticParameters) != candidate.getParameterCount()) { + continue; + } + } else { + + int optionalParameterCount = (int) kotlinFunction.getParameters().stream().filter(it -> it.isOptional()) + .count(); + int syntheticParameters = KotlinDefaultMask.getExactMaskCount(optionalParameterCount); + + if ((detectedConstructor.getParameterCount() + syntheticParameters) != candidate.getParameterCount()) { + continue; + } + } + + if (!KotlinInstantiationDelegate.hasDefaultConstructorMarker(candidateParameters)) { + continue; + } + + int userParameterCount = kotlinFunction != null ? kotlinFunction.getParameters().size() + : detectedConstructor.getParameterCount(); + if (parametersMatch(detectedConstructorParameters, candidateParameters, userParameterCount)) { + hit = candidate; + } + } + + return hit; + } + + private static boolean parametersMatch(java.lang.reflect.Parameter[] constructorParameters, + java.lang.reflect.Parameter[] candidateParameters, int userParameterCount) { + + return IntStream.range(0, userParameterCount) + .allMatch(i -> parametersMatch(constructorParameters[i], candidateParameters[i])); + } + + static boolean parametersMatch(java.lang.reflect.Parameter constructorParameter, + java.lang.reflect.Parameter candidateParameter) { + + if (constructorParameter.getType().equals(candidateParameter.getType())) { + return true; + } + + // candidate can be also a wrapper + + Class aClass = unwrapValueClass(candidateParameter.getType()); + + return constructorParameter.getType().equals(aClass); + } + + private static Class unwrapValueClass(Class type) { + + KClass kotlinClass = JvmClassMappingKt.getKotlinClass(type); + if (kotlinClass != null && kotlinClass.isValue()) { + + KFunction next = kotlinClass.getConstructors().iterator().next(); + KParameter kParameter = next.getParameters().get(0); + Type javaType = ReflectJvmMapping.getJavaType(kParameter.getType()); + if (javaType instanceof Class) { + return unwrapValueClass((Class) javaType); + } + } + + return type; + } + +} diff --git a/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java b/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java index 212145b3c4..840ddd2af6 100644 --- a/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java +++ b/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java @@ -16,18 +16,22 @@ package org.springframework.data.mapping.model; import java.lang.reflect.Array; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import org.springframework.beans.BeanInstantiationException; import org.springframework.beans.BeanUtils; +import org.springframework.core.KotlinDetector; import org.springframework.data.mapping.FactoryMethod; import org.springframework.data.mapping.InstanceCreatorMetadata; import org.springframework.data.mapping.Parameter; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PreferredConstructor; +import org.springframework.data.util.KotlinReflectionUtils; import org.springframework.util.ReflectionUtils; /** @@ -52,6 +56,18 @@ public , P extends PersistentPrope if (creator == null) { return instantiateClass(entity); } + + if (KotlinDetector.isKotlinReflectPresent() && KotlinReflectionUtils.isSupportedKotlinClass(entity.getType()) + && creator instanceof PreferredConstructor constructor) { + + PreferredConstructor> kotlinJvmConstructor = KotlinInstantiationDelegate + .resolveKotlinJvmConstructor(constructor); + + if (kotlinJvmConstructor != null) { + return instantiateKotlinClass(entity, provider, constructor, kotlinJvmConstructor); + } + } + int parameterCount = creator.getParameterCount(); Object[] params = parameterCount == 0 ? EMPTY_ARGS : new Object[parameterCount]; @@ -81,6 +97,29 @@ public , P extends PersistentPrope } } + @SuppressWarnings("unchecked") + private static , P extends PersistentProperty

> T instantiateKotlinClass( + E entity, ParameterValueProvider

provider, PreferredConstructor preferredConstructor, + PreferredConstructor> kotlinJvmConstructor) { + + Constructor ctor = kotlinJvmConstructor.getConstructor(); + KotlinInstantiationDelegate delegate = new KotlinInstantiationDelegate(preferredConstructor, ctor); + Object[] params = new Object[delegate.getRequiredParameterCount()]; + delegate.extractInvocationArguments(params, entity.getInstanceCreatorMetadata(), provider); + + try { + return (T) ctor.newInstance(params); + } catch (InstantiationException ex) { + throw new BeanInstantiationException(ctor, "Is it an abstract class?", ex); + } catch (IllegalAccessException ex) { + throw new BeanInstantiationException(ctor, "Is the preferredConstructor accessible?", ex); + } catch (IllegalArgumentException ex) { + throw new BeanInstantiationException(ctor, "Illegal arguments for preferredConstructor", ex); + } catch (InvocationTargetException ex) { + throw new BeanInstantiationException(ctor, "Constructor threw exception", ex.getTargetException()); + } + } + @SuppressWarnings("unchecked") private , P extends PersistentProperty

> T instantiateClass( E entity) { diff --git a/src/test/java/org/springframework/data/mapping/model/ReflectionEntityInstantiatorUnitTests.java b/src/test/java/org/springframework/data/mapping/model/ReflectionEntityInstantiatorUnitTests.java index 301139c0bb..8f7653b1b8 100755 --- a/src/test/java/org/springframework/data/mapping/model/ReflectionEntityInstantiatorUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/ReflectionEntityInstantiatorUnitTests.java @@ -68,6 +68,7 @@ void instantiatesTypeWithPreferredConstructorUsingParameterValueProvider() { PreferredConstructor constructor = PreferredConstructorDiscoverer.discover(Foo.class); + doReturn(Foo.class).when(entity).getType(); doReturn(constructor).when(entity).getInstanceCreatorMetadata(); var instance = INSTANCE.createInstance(entity, provider); @@ -78,7 +79,6 @@ void instantiatesTypeWithPreferredConstructorUsingParameterValueProvider() { } @Test // DATACMNS-300 - @SuppressWarnings({ "unchecked", "rawtypes" }) void throwsExceptionOnBeanInstantiationException() { doReturn(PersistentEntity.class).when(entity).getType(); 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 92e9e70cd4..5bf7f337df 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -253,22 +253,5 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") ) - class WithNestedAndPersistenceConstructorNullableInlineClass( - val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( - MyNullableInlineClass("foo") - ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") - ) { - - @PersistenceCreator - constructor(id: MyNestedNullableInlineClass?, baz: MyNullableInlineClass?=MyNullableInlineClass("aaa"), s: String) : this(id, baz) { - println(s) - } - - constructor(id: MyNestedNullableInlineClass, baz: MyNullableInlineClass, i: Int) : this(id, baz) { - } - - } - - } diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt index c24f9c2028..9bb7ead0ac 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt @@ -21,6 +21,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import org.springframework.data.mapping.PersistentEntity import org.springframework.data.mapping.context.SamplePersistentProperty +import kotlin.reflect.KClass /** * Unit tests for [ReflectionEntityInstantiator] creating instances using Kotlin data classes. @@ -36,17 +37,9 @@ class ReflectionEntityInstantiatorDataClassUnitTests { @Test // DATACMNS-1126 fun `should create instance`() { - val entity = mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - Contact::class.java - ) - every { provider.getParameterValue(any()) }.returnsMany("Walter", "White") - every { entity.instanceCreatorMetadata } returns constructor - val instance: Contact = - ReflectionEntityInstantiator.INSTANCE.createInstance(entity, provider) + val instance: Contact = construct(Contact::class) assertThat(instance.firstname).isEqualTo("Walter") assertThat(instance.lastname).isEqualTo("White") @@ -55,23 +48,29 @@ class ReflectionEntityInstantiatorDataClassUnitTests { @Test // DATACMNS-1126 fun `should create instance and fill in defaults`() { - val entity = - mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - ContactWithDefaulting::class.java - ) - every { provider.getParameterValue(any()) }.returnsMany("Walter", null) - every { entity.instanceCreatorMetadata } returns constructor - val instance: ContactWithDefaulting = - ReflectionEntityInstantiator.INSTANCE.createInstance(entity, provider) + val instance: ContactWithDefaulting = construct(ContactWithDefaulting::class) assertThat(instance.firstname).isEqualTo("Walter") assertThat(instance.lastname).isEqualTo("White") } + private fun construct(typeToCreate: KClass): T { + + val entity = mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + typeToCreate.java + ) + + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + return ReflectionEntityInstantiator.INSTANCE.createInstance(entity, provider) + } + data class Contact(val firstname: String, val lastname: String) data class ContactWithDefaulting(val firstname: String, val lastname: String = "White") diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt new file mode 100644 index 0000000000..a4615a8747 --- /dev/null +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt @@ -0,0 +1,104 @@ +/* + * Copyright 2021-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model + +import io.mockk.every +import io.mockk.mockk +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.data.annotation.PersistenceCreator +import org.springframework.data.mapping.PersistentEntity +import org.springframework.data.mapping.context.SamplePersistentProperty +import kotlin.reflect.KClass + +/** + * Unit tests for [ReflectionEntityInstantiator] creating instances using Kotlin inline classes. + * + * @author Mark Paluch + * See also https://github.com/spring-projects/spring-framework/issues/28638 + */ +class ReflectionEntityInstantiatorInlineClassUnitTests { + + val provider = mockk>() + + @Test // GH-2806 + fun `should create instance`() { + + every { provider.getParameterValue(any()) }.returnsMany("Walter") + + val instance: WithMyValueClass = + construct(WithMyValueClass::class) + + assertThat(instance.id.id).isEqualTo("Walter") + } + + @Test // GH-2806 + fun `should create instance with defaulting with value`() { + + every { provider.getParameterValue(any()) }.returnsMany("Walter", "") + + val instance: WithNestedMyNullableInlineClass = + construct(WithNestedMyNullableInlineClass::class) + + assertThat(instance.id?.id?.id).isEqualTo("Walter") + assertThat(instance.baz?.id).isEqualTo("") + } + + @Test // GH-2806 + fun `should create instance with defaulting without value`() { + + every { provider.getParameterValue(any()) } returns null + + val instance: WithNestedMyNullableInlineClass = construct(WithNestedMyNullableInlineClass::class) + + assertThat(instance.id?.id?.id).isEqualTo("foo") + assertThat(instance.baz?.id).isEqualTo("id") + } + + private fun construct(typeToCreate: KClass): T { + + val entity = mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + typeToCreate.java + ) + + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + return ReflectionEntityInstantiator.INSTANCE.createInstance(entity, provider) + } + + @JvmInline + value class MyInlineClass(val id: String) + + data class WithMyValueClass(val id: MyInlineClass) + + @JvmInline + value class MyNullableInlineClass(val id: String? = "id") + + @JvmInline + value class MyNestedNullableInlineClass(val id: MyNullableInlineClass) + + class WithNestedMyNullableInlineClass( + val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( + MyNullableInlineClass("foo") + ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") + ) + +} + From 302c9759ab70a1e21502f598244eeef02e336ee3 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 18 Apr 2023 09:24:07 +0200 Subject: [PATCH 04/11] Provide bytecode documentation. --- .../model/KotlinInstantiationDelegate.java | 4 ++ .../data/mapping/model/InlineClasses.kt | 71 +++++++++++++++++++ ...ssGeneratingEntityInstantiatorUnitTests.kt | 17 ----- ...nEntityInstantiatorInlineClassUnitTests.kt | 18 ----- 4 files changed, 75 insertions(+), 35 deletions(-) create mode 100644 src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java index 20a375bd62..96e920ba92 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java @@ -190,6 +190,10 @@ public

> Object[] extractInvocationArguments(Obj Constructor hit = doResolveKotlinConstructor(preferredConstructor.getConstructor()); + if (hit == preferredConstructor.getConstructor()) { + return preferredConstructor; + } + if (hit != null) { return new PreferredConstructor<>(hit, preferredConstructor.getParameters().toArray(new Parameter[0])); } diff --git a/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt new file mode 100644 index 0000000000..ce23d208d9 --- /dev/null +++ b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt @@ -0,0 +1,71 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model + +/** + * @author Mark Paluch + */ +@JvmInline +value class MyInlineClass(val id: String) + +data class WithMyValueClass(val id: MyInlineClass) { + + // ByteCode explanation + + // --------- + // default constructor, detected by Discoverers.KOTLIN + // private WithMyValueClass(java.lang.String arg0) {} + // --------- + + // --------- + // synthetic constructor that we actually want to use + // public synthetic WithMyValueClass(java.lang.String arg0, kotlin.jvm.internal.DefaultConstructorMarker arg1) {} + // --------- +} + +@JvmInline +value class MyNullableInlineClass(val id: String? = "id") + +@JvmInline +value class MyNestedNullableInlineClass(val id: MyNullableInlineClass) + +class WithNestedMyNullableInlineClass( + val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( + MyNullableInlineClass("foo") + ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") + + // ByteCode explanation + + // --------- + // private WithNestedMyNullableInlineClass(MyNestedNullableInlineClass id, MyNullableInlineClass baz) {} + // --------- + + // --------- + // default constructor, detected by Discoverers.KOTLIN + // note that these constructors use boxed variants ("MyNestedNullableInlineClass") + + // public WithNestedMyNullableInlineClass(MyNestedNullableInlineClass id, MyNullableInlineClass baz, DefaultConstructorMarker $constructor_marker) {} + // --------- + + // --------- + // translated by KotlinInstantiationDelegate.resolveKotlinJvmConstructor as we require a constructor that we can use to + // provide the nullability mask. This constructor only gets generated when parameters are nullable, otherwise + // Kotlin doesn't create this constructor + + // public WithNestedMyNullableInlineClass(MyNestedNullableInlineClass var1, MyNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} + // --------- + +) 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 5bf7f337df..85b230d071 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -236,22 +236,5 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { data class Organisation(var id: String? = null) - @JvmInline - value class MyInlineClass(val id: String) - - data class WithMyValueClass(val id: MyInlineClass) - - @JvmInline - value class MyNullableInlineClass(val id: String? = "id") - - @JvmInline - value class MyNestedNullableInlineClass(val id: MyNullableInlineClass) - - class WithNestedMyNullableInlineClass( - val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( - MyNullableInlineClass("foo") - ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") - ) - } diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt index a4615a8747..f4a58d3737 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt @@ -19,7 +19,6 @@ import io.mockk.every import io.mockk.mockk import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test -import org.springframework.data.annotation.PersistenceCreator import org.springframework.data.mapping.PersistentEntity import org.springframework.data.mapping.context.SamplePersistentProperty import kotlin.reflect.KClass @@ -83,22 +82,5 @@ class ReflectionEntityInstantiatorInlineClassUnitTests { return ReflectionEntityInstantiator.INSTANCE.createInstance(entity, provider) } - @JvmInline - value class MyInlineClass(val id: String) - - data class WithMyValueClass(val id: MyInlineClass) - - @JvmInline - value class MyNullableInlineClass(val id: String? = "id") - - @JvmInline - value class MyNestedNullableInlineClass(val id: MyNullableInlineClass) - - class WithNestedMyNullableInlineClass( - val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( - MyNullableInlineClass("foo") - ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") - ) - } From 7c4b8b347591cefc4cf342604cbc97f6a1b2a516 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 18 Apr 2023 15:40:16 +0200 Subject: [PATCH 05/11] Add tests for preferred constructor usage. --- .../data/mapping/model/InlineClasses.kt | 60 ++- ...ssGeneratingEntityInstantiatorUnitTests.kt | 410 +++++++++--------- ...nEntityInstantiatorInlineClassUnitTests.kt | 15 +- 3 files changed, 277 insertions(+), 208 deletions(-) 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 ce23d208d9..9742de0e02 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt @@ -15,13 +15,15 @@ */ package org.springframework.data.mapping.model +import org.springframework.data.annotation.PersistenceCreator + /** * @author Mark Paluch */ @JvmInline -value class MyInlineClass(val id: String) +value class MyInlineValueClass(val id: String) -data class WithMyValueClass(val id: MyInlineClass) { +data class WithMyInlineValueClass(val id: MyInlineValueClass) { // ByteCode explanation @@ -57,15 +59,61 @@ class WithNestedMyNullableInlineClass( // default constructor, detected by Discoverers.KOTLIN // note that these constructors use boxed variants ("MyNestedNullableInlineClass") - // public WithNestedMyNullableInlineClass(MyNestedNullableInlineClass id, MyNullableInlineClass baz, DefaultConstructorMarker $constructor_marker) {} + // public synthetic WithNestedMyNullableInlineClass(MyNestedNullableInlineClass id, MyNullableInlineClass baz, DefaultConstructorMarker $constructor_marker) {} // --------- // --------- // translated by KotlinInstantiationDelegate.resolveKotlinJvmConstructor as we require a constructor that we can use to - // provide the nullability mask. This constructor only gets generated when parameters are nullable, otherwise + // provide the defaulting mask. This constructor only gets generated when parameters are nullable, otherwise // Kotlin doesn't create this constructor - // public WithNestedMyNullableInlineClass(MyNestedNullableInlineClass var1, MyNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} + // public synthetic WithNestedMyNullableInlineClass(MyNestedNullableInlineClass var1, MyNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} // --------- - ) + +class WithInlineClassPreferredConstructor( + val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( + MyNullableInlineClass("foo") + ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") +) { + + @PersistenceCreator + constructor( + a: String, id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( + MyNullableInlineClass("foo") + ) + ) : this(id, MyNullableInlineClass(a + "-pref")) { + + } + + // ByteCode explanation + + // --------- + // private WithPreferredConstructor(MyNestedNullableInlineClass id, MyNullableInlineClass baz) {} + // --------- + + // --------- + // public WithPreferredConstructor(MyNestedNullableInlineClass var1, MyNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} + // --------- + + // --------- + // private WithPreferredConstructor(String a, MyNestedNullableInlineClass id) {} + // --------- + + // --------- + // this is the one we need to invoke to pass on the defaulting mask + // public synthetic WithPreferredConstructor(String var1, MyNestedNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} + // --------- + + // --------- + // public synthetic WithPreferredConstructor(MyNestedNullableInlineClass id, MyNullableInlineClass baz, DefaultConstructorMarker $constructor_marker) { + // --------- + + // --------- + // annotated constructor, detected by Discoverers.KOTLIN + // @PersistenceCreator + // public WithPreferredConstructor(String a, MyNestedNullableInlineClass id, DefaultConstructorMarker $constructor_marker) { + // --------- + +} + 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 85b230d071..f33a4becab 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -21,7 +21,6 @@ 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.PersistenceCreator import org.springframework.data.mapping.PersistentEntity import org.springframework.data.mapping.context.SamplePersistentProperty import kotlin.reflect.KClass @@ -35,206 +34,217 @@ import kotlin.reflect.KClass @Suppress("UNCHECKED_CAST") class KotlinClassGeneratingEntityInstantiatorUnitTests { - val provider = mockk>() - - @Test // DATACMNS-1126 - fun `should create instance`() { - - val entity = mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - Contact::class.java - ) - - every { provider.getParameterValue(any()) }.returnsMany("Walter", "White") - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - val instance: Contact = - KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - - assertThat(instance.firstname).isEqualTo("Walter") - assertThat(instance.lastname).isEqualTo("White") - } - - @Test // DATACMNS-1126 - fun `should create instance and fill in defaults`() { - - val entity = - mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - ContactWithDefaulting::class.java - ) - - every { provider.getParameterValue(any()) }.returnsMany( - "Walter", null, "Skyler", null, null, null, null, null, null, null, /* 0-9 */ - null, null, null, null, null, null, null, null, null, null, /* 10-19 */ - null, null, null, null, null, null, null, null, null, null, /* 20-29 */ - null, "Walter", null, "Junior", null - ) - - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - val instance: ContactWithDefaulting = KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - - assertThat(instance.prop0).isEqualTo("Walter") - assertThat(instance.prop2).isEqualTo("Skyler") - assertThat(instance.prop31).isEqualTo("Walter") - assertThat(instance.prop32).isEqualTo("White") - assertThat(instance.prop33).isEqualTo("Junior") - assertThat(instance.prop34).isEqualTo("White") - } - - @Test // DATACMNS-1200 - fun `absent primitive value should cause MappingInstantiationException`() { - - val entity = mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - WithBoolean::class.java - ) - - every { provider.getParameterValue(any()) } returns null - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - assertThatThrownBy { - KotlinClassGeneratingEntityInstantiator().createInstance( - entity, - provider - ) - } // - .isInstanceOf(MappingInstantiationException::class.java) // - .hasMessageContaining("init") // - .hasMessageContaining("kotlin.Boolean") // - .hasCauseInstanceOf(IllegalArgumentException::class.java) - } - - @Test // DATACMNS-1200 - fun `should apply primitive defaulting for absent parameters`() { - - val entity = - mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - WithPrimitiveDefaulting::class.java - ) - - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { provider.getParameterValue(any()) } returns null - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - val instance: WithPrimitiveDefaulting = - KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - - assertThat(instance.aByte).isEqualTo(0) - assertThat(instance.aShort).isEqualTo(0) - assertThat(instance.anInt).isEqualTo(0) - assertThat(instance.aLong).isEqualTo(0L) - assertThat(instance.aFloat).isEqualTo(0.0f) - assertThat(instance.aDouble).isEqualTo(0.0) - assertThat(instance.aChar).isEqualTo('a') - assertThat(instance.aBool).isTrue() - } - - @Test // DATACMNS-1338 - fun `should create instance using @PersistenceConstructor`() { - - every { provider.getParameterValue(any()) } returns "Walter" - val instance = construct(CustomUser::class) - - assertThat(instance.id).isEqualTo("Walter") - } - - @Test - fun `should use default constructor for types using value class`() { - - every { provider.getParameterValue(any()) } returns "hello" - val instance = construct(WithMyValueClass::class) - - assertThat(instance.id.id).isEqualTo("hello") - } - - @Test - fun `should use default constructor for types using nullable value class`() { - - every { provider.getParameterValue(any()) } returns null - - val instance = construct(WithNestedMyNullableInlineClass::class) - - assertThat(instance.id?.id?.id).isEqualTo("foo") - assertThat(instance.baz?.id).isEqualTo("id") - } - - private fun construct(typeToCreate : KClass): T { - - val entity = - mockk>() - val constructor = - PreferredConstructorDiscoverer.discover( - typeToCreate.java - ) - - every { entity.instanceCreatorMetadata } returns constructor - every { entity.type } returns constructor!!.constructor.declaringClass - every { entity.typeInformation } returns mockk() - - return KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) - } - - data class Contact(val firstname: String, val lastname: String) - - data class ContactWithDefaulting( - val prop0: String, val prop1: String = "White", val prop2: String, - val prop3: String = "White", val prop4: String = "White", val prop5: String = "White", - val prop6: String = "White", val prop7: String = "White", val prop8: String = "White", - val prop9: String = "White", val prop10: String = "White", val prop11: String = "White", - val prop12: String = "White", val prop13: String = "White", val prop14: String = "White", - val prop15: String = "White", val prop16: String = "White", val prop17: String = "White", - val prop18: String = "White", val prop19: String = "White", val prop20: String = "White", - val prop21: String = "White", val prop22: String = "White", val prop23: String = "White", - val prop24: String = "White", val prop25: String = "White", val prop26: String = "White", - val prop27: String = "White", val prop28: String = "White", val prop29: String = "White", - val prop30: String = "White", val prop31: String = "White", val prop32: String = "White", - val prop33: String, val prop34: String = "White" - ) - - data class WithBoolean(val state: Boolean) - - data class WithPrimitiveDefaulting( - val aByte: Byte = 0, val aShort: Short = 0, val anInt: Int = 0, val aLong: Long = 0L, - val aFloat: Float = 0.0f, val aDouble: Double = 0.0, val aChar: Char = 'a', val aBool: Boolean = true - ) - - data class ContactWithPersistenceConstructor(val firstname: String, val lastname: String) { - - @PersistenceConstructor - constructor(firstname: String) : this(firstname, "") - } - - data class CustomUser( - var id: String? = null, - - var organisations: MutableList = mutableListOf() - ) { - @PersistenceConstructor - constructor(id: String?) : this(id, mutableListOf()) - } + val provider = mockk>() + + @Test // DATACMNS-1126 + fun `should create instance`() { + + val entity = mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + Contact::class.java + ) + + every { provider.getParameterValue(any()) }.returnsMany("Walter", "White") + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + val instance: Contact = + KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + assertThat(instance.firstname).isEqualTo("Walter") + assertThat(instance.lastname).isEqualTo("White") + } + + @Test // DATACMNS-1126 + fun `should create instance and fill in defaults`() { + + val entity = + mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + ContactWithDefaulting::class.java + ) + + every { provider.getParameterValue(any()) }.returnsMany( + "Walter", null, "Skyler", null, null, null, null, null, null, null, /* 0-9 */ + null, null, null, null, null, null, null, null, null, null, /* 10-19 */ + null, null, null, null, null, null, null, null, null, null, /* 20-29 */ + null, "Walter", null, "Junior", null + ) + + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + val instance: ContactWithDefaulting = KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + assertThat(instance.prop0).isEqualTo("Walter") + assertThat(instance.prop2).isEqualTo("Skyler") + assertThat(instance.prop31).isEqualTo("Walter") + assertThat(instance.prop32).isEqualTo("White") + assertThat(instance.prop33).isEqualTo("Junior") + assertThat(instance.prop34).isEqualTo("White") + } + + @Test // DATACMNS-1200 + fun `absent primitive value should cause MappingInstantiationException`() { + + val entity = mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + WithBoolean::class.java + ) + + every { provider.getParameterValue(any()) } returns null + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + assertThatThrownBy { + KotlinClassGeneratingEntityInstantiator().createInstance( + entity, + provider + ) + } // + .isInstanceOf(MappingInstantiationException::class.java) // + .hasMessageContaining("init") // + .hasMessageContaining("kotlin.Boolean") // + .hasCauseInstanceOf(IllegalArgumentException::class.java) + } + + @Test // DATACMNS-1200 + fun `should apply primitive defaulting for absent parameters`() { + + val entity = + mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + WithPrimitiveDefaulting::class.java + ) + + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { provider.getParameterValue(any()) } returns null + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + val instance: WithPrimitiveDefaulting = + KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + assertThat(instance.aByte).isEqualTo(0) + assertThat(instance.aShort).isEqualTo(0) + assertThat(instance.anInt).isEqualTo(0) + assertThat(instance.aLong).isEqualTo(0L) + assertThat(instance.aFloat).isEqualTo(0.0f) + assertThat(instance.aDouble).isEqualTo(0.0) + assertThat(instance.aChar).isEqualTo('a') + assertThat(instance.aBool).isTrue() + } + + @Test // DATACMNS-1338 + fun `should create instance using @PersistenceConstructor`() { + + every { provider.getParameterValue(any()) } returns "Walter" + val instance = construct(CustomUser::class) + + assertThat(instance.id).isEqualTo("Walter") + } + + @Test + fun `should use default constructor for types using value class`() { + + every { provider.getParameterValue(any()) } returns "hello" + val instance = construct(WithMyInlineValueClass::class) + + assertThat(instance.id.id).isEqualTo("hello") + } + + @Test + fun `should use default constructor for types using nullable value class`() { + + every { provider.getParameterValue(any()) } returns null + + val instance = construct(WithNestedMyNullableInlineClass::class) + + assertThat(instance.id?.id?.id).isEqualTo("foo") + assertThat(instance.baz?.id).isEqualTo("id") + } + + @Test // GH-2806 + fun `should use annotated constructor for types using nullable value class`() { + + every { provider.getParameterValue(any()) }.returnsMany("Walter", null) + + val instance = construct(WithInlineClassPreferredConstructor::class) + + assertThat(instance.id?.id?.id).isEqualTo("foo") + assertThat(instance.baz?.id).isEqualTo("Walter-pref") + } + + private fun construct(typeToCreate: KClass): T { + + val entity = + mockk>() + val constructor = + PreferredConstructorDiscoverer.discover( + typeToCreate.java + ) + + every { entity.instanceCreatorMetadata } returns constructor + every { entity.type } returns constructor!!.constructor.declaringClass + every { entity.typeInformation } returns mockk() + + return KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + } + + data class Contact(val firstname: String, val lastname: String) + + data class ContactWithDefaulting( + val prop0: String, val prop1: String = "White", val prop2: String, + val prop3: String = "White", val prop4: String = "White", val prop5: String = "White", + val prop6: String = "White", val prop7: String = "White", val prop8: String = "White", + val prop9: String = "White", val prop10: String = "White", val prop11: String = "White", + val prop12: String = "White", val prop13: String = "White", val prop14: String = "White", + val prop15: String = "White", val prop16: String = "White", val prop17: String = "White", + val prop18: String = "White", val prop19: String = "White", val prop20: String = "White", + val prop21: String = "White", val prop22: String = "White", val prop23: String = "White", + val prop24: String = "White", val prop25: String = "White", val prop26: String = "White", + val prop27: String = "White", val prop28: String = "White", val prop29: String = "White", + val prop30: String = "White", val prop31: String = "White", val prop32: String = "White", + val prop33: String, val prop34: String = "White" + ) + + data class WithBoolean(val state: Boolean) - data class Organisation(var id: String? = null) + data class WithPrimitiveDefaulting( + val aByte: Byte = 0, val aShort: Short = 0, val anInt: Int = 0, val aLong: Long = 0L, + val aFloat: Float = 0.0f, val aDouble: Double = 0.0, val aChar: Char = 'a', val aBool: Boolean = true + ) + + data class ContactWithPersistenceConstructor(val firstname: String, val lastname: String) { + + @PersistenceConstructor + constructor(firstname: String) : this(firstname, "") + } + + data class CustomUser( + var id: String? = null, + + var organisations: MutableList = mutableListOf() + ) { + @PersistenceConstructor + constructor(id: String?) : this(id, mutableListOf()) + } + + data class Organisation(var id: String? = null) } diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt index f4a58d3737..b225c83d61 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt @@ -38,8 +38,8 @@ class ReflectionEntityInstantiatorInlineClassUnitTests { every { provider.getParameterValue(any()) }.returnsMany("Walter") - val instance: WithMyValueClass = - construct(WithMyValueClass::class) + val instance: WithMyInlineValueClass = + construct(WithMyInlineValueClass::class) assertThat(instance.id.id).isEqualTo("Walter") } @@ -67,6 +67,17 @@ class ReflectionEntityInstantiatorInlineClassUnitTests { assertThat(instance.baz?.id).isEqualTo("id") } + @Test // GH-2806 + fun `should use annotated constructor for types using nullable value class`() { + + every { provider.getParameterValue(any()) }.returnsMany("Walter", null) + + val instance = construct(WithInlineClassPreferredConstructor::class) + + assertThat(instance.id?.id?.id).isEqualTo("foo") + assertThat(instance.baz?.id).isEqualTo("Walter-pref") + } + private fun construct(typeToCreate: KClass): T { val entity = mockk>() From c29656971531e010e40281353de938cff25882ea Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 21 Jun 2023 11:27:07 +0200 Subject: [PATCH 06/11] Add support for nullability and defaulting permutations. --- .../data/mapping/model/BeanWrapper.java | 8 +- ...lassGeneratingPropertyAccessorFactory.java | 97 ++++++++-- .../data/mapping/model/KotlinCopyMethod.java | 45 +++-- .../data/mapping/model/KotlinValueBoxing.java | 69 +++++++ .../data/util/KotlinBeanInfoFactory.java | 86 +++++++++ .../data/util/KotlinReflectionUtils.java | 35 ++++ src/main/resources/META-INF/spring.factories | 1 + .../KotlinPropertyAccessorFactoryTests.java | 170 ++++++++++++++++++ .../data/mapping/model/InlineClasses.kt | 111 +++++++++--- ...ssGeneratingEntityInstantiatorUnitTests.kt | 6 +- ...nEntityInstantiatorInlineClassUnitTests.kt | 14 +- .../util/KotlinBeanInfoFactoryUnitTests.kt | 77 ++++++++ 12 files changed, 659 insertions(+), 60 deletions(-) create mode 100644 src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java create mode 100644 src/main/java/org/springframework/data/util/KotlinBeanInfoFactory.java create mode 100644 src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java create mode 100644 src/test/kotlin/org/springframework/data/util/KotlinBeanInfoFactoryUnitTests.kt diff --git a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java index 607200434b..004e87cd7d 100644 --- a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java +++ b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java @@ -168,8 +168,8 @@ static Object setProperty(PersistentProperty property, T bean, @Nullable KCallable copy = copyMethodCache.computeIfAbsent(type, it -> getCopyMethod(it, property)); if (copy == null) { - throw new UnsupportedOperationException(String.format( - "Kotlin class %s has no .copy(…) method for property %s", type.getName(), property.getName())); + throw new UnsupportedOperationException(String.format("Kotlin class %s has no .copy(…) method for property %s", + type.getName(), property.getName())); } return copy.callBy(getCallArgs(copy, property, bean, value)); @@ -179,7 +179,6 @@ private static Map getCallArgs(KCallable callable, Pe T bean, @Nullable Object value) { Map args = new LinkedHashMap<>(2, 1); - List parameters = callable.getParameters(); for (KParameter parameter : parameters) { @@ -190,7 +189,8 @@ private static Map getCallArgs(KCallable callable, Pe if (parameter.getKind() == Kind.VALUE && parameter.getName() != null && parameter.getName().equals(property.getName())) { - args.put(parameter, value); + + args.put(parameter, KotlinValueBoxing.getWrapper(parameter).apply(value)); } } return args; 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 99d815b41c..e4875142f7 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -18,11 +18,15 @@ import static org.springframework.asm.Opcodes.*; import static org.springframework.data.mapping.model.BytecodeUtil.*; +import kotlin.reflect.KParameter; +import kotlin.reflect.KParameter.Kind; + import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Member; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.lang.reflect.Parameter; import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.Collections; @@ -48,12 +52,15 @@ import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.SimpleAssociationHandler; import org.springframework.data.mapping.SimplePropertyHandler; +import org.springframework.data.mapping.model.KotlinCopyMethod.KotlinCopyByProperty; import org.springframework.data.util.Optionals; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ConcurrentLruCache; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; /** * A factory that can generate byte code to speed-up dynamic property access. Uses the {@link PersistentEntity}'s @@ -76,6 +83,9 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert private volatile Map, Class>> propertyAccessorClasses = new HashMap<>( 32); + private ConcurrentLruCache, Function> wrapperCache = new ConcurrentLruCache<>( + 256, KotlinValueClassBoxingWrapper::getWrapper); + @Override public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity entity, T bean) { @@ -96,16 +106,23 @@ public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity) constructor.newInstance(args); + + PersistentPropertyAccessor accessor = (PersistentPropertyAccessor) constructor.newInstance(args); + + if (KotlinDetector.isKotlinType(entity.getType())) { + return new KotlinValueClassBoxingWrapper<>(entity, accessor, wrapperCache); + } + + return accessor; } catch (Exception e) { - throw new IllegalArgumentException(String.format("Cannot create persistent property accessor for %s", entity), e); + throw new IllegalArgumentException(String.format("Cannot create persistent property delegate for %s", entity), e); } finally { args[0] = null; } } /** - * Checks whether an accessor class can be generated. + * Checks whether an delegate class can be generated. * * @param entity must not be {@literal null}. * @return {@literal true} if the runtime is equal or greater to Java 1.7, we can access the ClassLoader, the property @@ -205,8 +222,8 @@ private Class> createAccessorClass(PersistentEntit * well. Accessing properties using generated accessors imposes some constraints: *

    *
  • Runtime must be Java 7 or higher
    • - *
  • The generated accessor decides upon generation whether to use field or property access for particular - * properties. It's not possible to change the access method once the accessor class is generated.
    • + *
  • The generated delegate decides upon generation whether to use field or property access for particular + * properties. It's not possible to change the access method once the delegate class is generated.
    • *
  • Property names and their {@link String#hashCode()} must be unique within a {@link PersistentEntity}.
    • *
* These constraints apply to retain the performance gains, otherwise the generated code has to decide which method @@ -260,7 +277,7 @@ private Class> createAccessorClass(PersistentEntit * // … * } * throw new UnsupportedOperationException( - * String.format("No accessor to set property %s", new Object[] { property })); + * String.format("No delegate to set property %s", new Object[] { property })); * } * * public Object getProperty(PersistentProperty property) { @@ -275,7 +292,7 @@ private Class> createAccessorClass(PersistentEntit * return bean.field; * // … * throw new UnsupportedOperationException( - * String.format("No accessor to get property %s", new Object[] { property })); + * String.format("No delegate to get property %s", new Object[] { property })); * } * } * } @@ -776,7 +793,7 @@ private static void visitGetProperty(PersistentEntity entity, visitGetPropertySwitch(entity, persistentProperties, internalClassName, mv); mv.visitLabel(l1); - visitThrowUnsupportedOperationException(mv, "No accessor to get property %s"); + visitThrowUnsupportedOperationException(mv, "No delegate to get property %s"); mv.visitLocalVariable(THIS_REF, referenceName(internalClassName), null, l0, l1, 0); mv.visitLocalVariable("property", referenceName(PERSISTENT_PROPERTY), @@ -911,7 +928,7 @@ private static void visitGetProperty0(PersistentEntity entity, PersistentP * // … * } * throw new UnsupportedOperationException( - * String.format("No accessor to set property %s", new Object[] { property })); + * String.format("No delegate to set property %s", new Object[] { property })); * } * */ @@ -939,7 +956,7 @@ private static void visitSetProperty(PersistentEntity entity, Label l1 = new Label(); mv.visitLabel(l1); - visitThrowUnsupportedOperationException(mv, "No accessor to set property %s"); + visitThrowUnsupportedOperationException(mv, "No delegate to set property %s"); mv.visitLocalVariable(THIS_REF, referenceName(internalClassName), null, l0, l1, 0); mv.visitLocalVariable("property", "Lorg/springframework/data/mapping/PersistentProperty;", @@ -1431,7 +1448,7 @@ static boolean supportsMutation(PersistentProperty property) { * Check whether the owning type of {@link PersistentProperty} declares a {@literal copy} method or {@literal copy} * method with parameter defaulting. * - * @param type must not be {@literal null}. + * @param property must not be {@literal null}. * @return */ private static boolean hasKotlinCopyMethod(PersistentProperty property) { @@ -1444,4 +1461,62 @@ private static boolean hasKotlinCopyMethod(PersistentProperty property) { return false; } + + /** + * Wrapper to encapsulate Kotlin's value class boxing when properties are nullable. + * + * @param entity + * @param delegate + * @param wrapperCache + * @param + */ + record KotlinValueClassBoxingWrapper (PersistentEntity entity, PersistentPropertyAccessor delegate, + ConcurrentLruCache, Function> wrapperCache) + implements + PersistentPropertyAccessor { + + @Override + public void setProperty(PersistentProperty property, @Nullable Object value) { + delegate.setProperty(property, wrapperCache.get(property).apply(value)); + } + + static Function getWrapper(PersistentProperty property) { + + Optional kotlinCopyMethod = KotlinCopyMethod.findCopyMethod(property.getOwner().getType()) + .filter(it -> it.supportsProperty(property)); + + if (kotlinCopyMethod.isPresent() + && kotlinCopyMethod.filter(it -> it.forProperty(property).isPresent()).isPresent()) { + KotlinCopyMethod copyMethod = kotlinCopyMethod.get(); + + Optional kParameter = kotlinCopyMethod.stream() + .flatMap(it -> it.getCopyFunction().getParameters().stream()) // + .filter(kf -> kf.getKind() == Kind.VALUE) // + .filter(kf -> StringUtils.hasText(kf.getName())) // + .filter(kf -> kf.getName().equals(property.getName())) // + .findFirst(); + + Function objectObjectFunction = kParameter.map(KotlinValueBoxing::getWrapper) + .orElse(Function.identity()); + KotlinCopyByProperty kotlinCopyByProperty = copyMethod.forProperty(property).get(); + Method copy = copyMethod.getSyntheticCopyMethod(); + + Parameter parameter = copy.getParameters()[kotlinCopyByProperty.getParameterPosition()]; + + return o -> ClassUtils.isAssignableValue(parameter.getType(), o) ? o : objectObjectFunction.apply(o); + } + + return Function.identity(); + } + + @Override + public Object getProperty(PersistentProperty property) { + return delegate.getProperty(property); + } + + @Override + public T getBean() { + return delegate.getBean(); + } + } } diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java index 4b7a22d4b9..6e0542f7ef 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java @@ -31,12 +31,14 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.springframework.core.ResolvableType; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.SimplePropertyHandler; +import org.springframework.data.util.KotlinReflectionUtils; import org.springframework.util.Assert; /** @@ -67,10 +69,9 @@ private KotlinCopyMethod(Method publicCopyMethod, Method syntheticCopyMethod) { } /** - * Attempt to lookup the Kotlin {@code copy} method. Lookup happens in two stages: Find the synthetic copy method and + * Attempt to look up the Kotlin {@code copy} method. Lookup happens in two stages: Find the synthetic copy method and * then attempt to resolve its public variant. * - * @param property the property that must be included in the copy method. * @param type the class. * @return {@link Optional} {@link KotlinCopyMethod}. */ @@ -171,13 +172,27 @@ private static Optional findPublicCopyMethod(Method defaultKotlinMethod) return Optional.empty(); } + boolean usesValueClasses = KotlinReflectionUtils.hasValueClassProperty(type); List constructorArguments = getComponentArguments(primaryConstructor); + Predicate isCopyMethod; - return Arrays.stream(type.getDeclaredMethods()).filter(it -> it.getName().equals("copy") // - && !it.isSynthetic() // - && !Modifier.isStatic(it.getModifiers()) // - && it.getReturnType().equals(type) // - && it.getParameterCount() == constructorArguments.size()) // + if (usesValueClasses) { + String methodName = defaultKotlinMethod.getName(); + Assert.isTrue(methodName.contains("$"), () -> "Cannot find $ marker in method name " + defaultKotlinMethod); + + String methodNameWithHash = methodName.substring(0, methodName.indexOf("$")); + isCopyMethod = it -> it.equals(methodNameWithHash); + } else { + isCopyMethod = it -> it.equals("copy"); + } + + return Arrays.stream(type.getDeclaredMethods()).filter(it -> { + return isCopyMethod.test(it.getName()) // + && !it.isSynthetic() // + && !Modifier.isStatic(it.getModifiers()) // + && it.getReturnType().equals(type) // + && it.getParameterCount() == constructorArguments.size(); + }) // .filter(it -> { KFunction kotlinFunction = ReflectJvmMapping.getKotlinFunction(it); @@ -228,13 +243,17 @@ private static Optional findSyntheticCopyMethod(Class type) { return Optional.empty(); } + boolean usesValueClasses = KotlinReflectionUtils.hasValueClassProperty(type); + + Predicate isCopyMethod = usesValueClasses ? (it -> it.startsWith("copy-") && it.endsWith("$default")) + : (it -> it.equals("copy$default")); + return Arrays.stream(type.getDeclaredMethods()) // - .filter(it -> it.getName().equals("copy$default") // + .filter(it -> isCopyMethod.test(it.getName()) // && Modifier.isStatic(it.getModifiers()) // && it.getReturnType().equals(type)) .filter(Method::isSynthetic) // - .filter(it -> matchesPrimaryConstructor(it.getParameterTypes(), primaryConstructor)) - .findFirst(); + .filter(it -> matchesPrimaryConstructor(it.getParameterTypes(), primaryConstructor)).findFirst(); } /** @@ -259,6 +278,12 @@ private static boolean matchesPrimaryConstructor(Class[] parameterTypes, KFun KParameter kParameter = constructorArguments.get(i); + if (KotlinReflectionUtils.isValueClass(kParameter.getType())) { + // sigh. This can require deep unwrapping because the public vs. the synthetic copy methods use different + // parameter types. + continue; + } + if (!isAssignableFrom(parameterTypes[i + 1], kParameter.getType())) { return false; } diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java b/src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java new file mode 100644 index 0000000000..e879ce82c6 --- /dev/null +++ b/src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java @@ -0,0 +1,69 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model; + +import kotlin.reflect.KClass; +import kotlin.reflect.KFunction; +import kotlin.reflect.KParameter; + +import java.util.function.Function; + +import org.springframework.data.util.KotlinReflectionUtils; + +/** + * Utility to box values into inline class instances. + * + * @author Mark Paluch + * @since 3.2 + */ +class KotlinValueBoxing { + + /** + * @param kParameter the kotlin parameter to wrap. + * @return a wrapper function that potentially wraps (value-boxing) values into value classes if these are nullable or + * used in a lower part of the type hierarchy. + */ + static Function getWrapper(KParameter kParameter) { + return getWrapper(kParameter, true); + } + + /** + * @param kParameter the kotlin parameter to wrap. + * @param domainTypeUsage optional in the domain type require value type casting. Inner/nested ones don't. This is + * because calling the synthetic constructor with an optional parameter requires an inline class while + * optional parameters via reflection are handled within Kotlin-Reflection. + * @return a wrapper function that potentially wraps (value-boxing) values into value classes if these are nullable or + * used in a lower part of the type hierarchy. + */ + private static Function getWrapper(KParameter kParameter, boolean domainTypeUsage) { + + if (KotlinReflectionUtils.isValueClass(kParameter.getType()) && (!domainTypeUsage || kParameter.isOptional())) { + + KClass kClass = (KClass) kParameter.getType().getClassifier(); + KFunction ctor = kClass.getConstructors().iterator().next(); + + // using reflection to construct a value class wrapper. Everything + // else would require too many levels of indirections. + + KParameter nested = ctor.getParameters().get(0); + Function wrapper = getWrapper(nested, false); + + return o -> kClass.isInstance(o) ? o : ctor.call(wrapper.apply(o)); + } + + return Function.identity(); + } +} diff --git a/src/main/java/org/springframework/data/util/KotlinBeanInfoFactory.java b/src/main/java/org/springframework/data/util/KotlinBeanInfoFactory.java new file mode 100644 index 0000000000..8fc946944a --- /dev/null +++ b/src/main/java/org/springframework/data/util/KotlinBeanInfoFactory.java @@ -0,0 +1,86 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.util; + +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KCallable; +import kotlin.reflect.KClass; +import kotlin.reflect.KMutableProperty; +import kotlin.reflect.KProperty; +import kotlin.reflect.jvm.ReflectJvmMapping; + +import java.beans.BeanDescriptor; +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.PropertyDescriptor; +import java.beans.SimpleBeanInfo; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; + +import org.springframework.beans.BeanInfoFactory; +import org.springframework.core.KotlinDetector; +import org.springframework.core.Ordered; + +/** + * {@link BeanInfoFactory} specific to Kotlin types using Kotlin reflection to determine bean properties. + * + * @author Mark Paluch + * @since 3.2 + * @see JvmClassMappingKt + * @see ReflectJvmMapping + */ +public class KotlinBeanInfoFactory implements BeanInfoFactory, Ordered { + + @Override + public BeanInfo getBeanInfo(Class beanClass) throws IntrospectionException { + + if (!KotlinDetector.isKotlinReflectPresent() || !KotlinDetector.isKotlinType(beanClass)) { + return null; + } + + KClass kotlinClass = JvmClassMappingKt.getKotlinClass(beanClass); + List pds = new ArrayList<>(); + + for (KCallable member : kotlinClass.getMembers()) { + + if (member instanceof KProperty property) { + + Method getter = ReflectJvmMapping.getJavaGetter(property); + Method setter = property instanceof KMutableProperty kmp ? ReflectJvmMapping.getJavaSetter(kmp) : null; + + pds.add(new PropertyDescriptor(property.getName(), getter, setter)); + } + } + return new SimpleBeanInfo() { + @Override + public BeanDescriptor getBeanDescriptor() { + return new BeanDescriptor(beanClass); + } + + @Override + public PropertyDescriptor[] getPropertyDescriptors() { + return pds.toArray(new PropertyDescriptor[0]); + } + }; + } + + @Override + public int getOrder() { + return LOWEST_PRECEDENCE - 10; // leave some space for customizations. + } + +} diff --git a/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java b/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java index 56bc7c26e0..9dd7672dd7 100644 --- a/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java +++ b/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java @@ -83,6 +83,41 @@ public static boolean isDataClass(Class type) { return kotlinClass.isData(); } + /** + * Returns whether the given {@link KType} is a {@link KClass#isValue() value} class. + * + * @param type the kotlin type to inspect. + * @return {@code true} the type is a value class. + * @since 3.2 + */ + public static boolean isValueClass(KType type) { + return type.getClassifier()instanceof KClass kc && kc.isValue(); + } + + /** + * Returns whether the given class makes uses Kotlin {@link KClass#isValue() value} classes. + * + * @param type the kotlin type to inspect. + * @return {@code true} when at least one property uses Kotlin value classes. + * @since 3.2 + */ + public static boolean hasValueClassProperty(Class type) { + + if (!KotlinDetector.isKotlinType(type)) { + return false; + } + + KClass kotlinClass = JvmClassMappingKt.getKotlinClass(type); + + for (KCallable member : kotlinClass.getMembers()) { + if (member instanceof KProperty kp && isValueClass(kp.getReturnType())) { + return true; + } + } + + return false; + } + /** * Returns a {@link KFunction} instance corresponding to the given Java {@link Method} instance, or {@code null} if * this method cannot be represented by a Kotlin function. diff --git a/src/main/resources/META-INF/spring.factories b/src/main/resources/META-INF/spring.factories index fc6274377f..506aaa02df 100644 --- a/src/main/resources/META-INF/spring.factories +++ b/src/main/resources/META-INF/spring.factories @@ -1,3 +1,4 @@ org.springframework.data.web.config.SpringDataJacksonModules=org.springframework.data.web.config.SpringDataJacksonConfiguration org.springframework.data.util.CustomCollectionRegistrar=org.springframework.data.util.CustomCollections.VavrCollections, \ org.springframework.data.util.CustomCollections.EclipseCollections +org.springframework.beans.BeanInfoFactory=org.springframework.data.util.KotlinBeanInfoFactory diff --git a/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java b/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java new file mode 100644 index 0000000000..69800fe2ac --- /dev/null +++ b/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java @@ -0,0 +1,170 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model; + +import static org.assertj.core.api.Assertions.*; + +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KClass; + +import java.util.function.Function; +import java.util.stream.Stream; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.beans.BeanUtils; +import org.springframework.data.mapping.Parameter; +import org.springframework.data.mapping.context.SampleMappingContext; +import org.springframework.data.mapping.context.SamplePersistentProperty; + +/** + * Kotlin-specific unit tests for {@link ClassGeneratingPropertyAccessorFactory} and + * {@link BeanWrapperPropertyAccessorFactory} + * + * @author John Blum + * @author Oliver Gierke + * @author Mark Paluch + */ +public class KotlinPropertyAccessorFactoryTests { + + private EntityInstantiators instantiators = new EntityInstantiators(); + private SampleMappingContext mappingContext = new SampleMappingContext(); + + @MethodSource("factories") + @ParameterizedTest // GH-2806 + void shouldGeneratePropertyAccessorForTypeWithValueClass(PersistentPropertyAccessorFactory factory) { + + BasicPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(WithMyValueClass.class); + + Object instance = createInstance(entity, parameter -> "foo"); + + var propertyAccessor = factory.getPropertyAccessor(entity, instance); + var persistentProperty = entity.getRequiredPersistentProperty("id"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(persistentProperty)).isEqualTo("foo"); + + propertyAccessor.setProperty(persistentProperty, "bar"); + assertThat(propertyAccessor.getProperty(persistentProperty)).isEqualTo("bar"); + } + + @MethodSource("factories") + @ParameterizedTest // GH-2806 + void shouldGeneratePropertyAccessorForTypeWithNullableValueClass(PersistentPropertyAccessorFactory factory) + throws ReflectiveOperationException { + + BasicPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(WithNestedMyNullableValueClass.class); + + Object instance = createInstance(entity, parameter -> null); + + var propertyAccessor = factory.getPropertyAccessor(entity, instance); + + var expectedDefaultValue = BeanUtils + .instantiateClass(MyNullableValueClass.class.getDeclaredConstructor(String.class), "id"); + var barValue = BeanUtils.instantiateClass(MyNullableValueClass.class.getDeclaredConstructor(String.class), "bar"); + var property = entity.getRequiredPersistentProperty("baz"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(property)).isEqualTo(expectedDefaultValue); + + propertyAccessor.setProperty(property, barValue); + assertThat(propertyAccessor.getProperty(property)).isEqualTo(barValue); + } + + @MethodSource("factories") + @ParameterizedTest // GH-2806 + void shouldGeneratePropertyAccessorForDataClassWithNullableValueClass(PersistentPropertyAccessorFactory factory) + throws ReflectiveOperationException { + + BasicPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(DataClassWithNullableValueClass.class); + + Object instance = createInstance(entity, parameter -> null); + + var propertyAccessor = factory.getPropertyAccessor(entity, instance); + + var expectedDefaultValue = BeanUtils + .instantiateClass(MyNullableValueClass.class.getDeclaredConstructor(String.class), "id"); + var barValue = BeanUtils.instantiateClass(MyNullableValueClass.class.getDeclaredConstructor(String.class), "bar"); + var fullyNullable = entity.getRequiredPersistentProperty("fullyNullable"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(fullyNullable)).isEqualTo(expectedDefaultValue); + + propertyAccessor.setProperty(fullyNullable, barValue); + assertThat(propertyAccessor.getProperty(fullyNullable)).isEqualTo(barValue); + + var innerNullable = entity.getRequiredPersistentProperty("innerNullable"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(innerNullable)).isEqualTo("id"); + + propertyAccessor.setProperty(innerNullable, "bar"); + assertThat(propertyAccessor.getProperty(innerNullable)).isEqualTo("bar"); + } + + @MethodSource("factories") + @ParameterizedTest // GH-2806 + void nestedNullablePropertiesShouldBeSetCorrectly(PersistentPropertyAccessorFactory factory) { + + BasicPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(DataClassWithNestedNullableValueClass.class); + + Object instance = createInstance(entity, parameter -> null); + + var propertyAccessor = factory.getPropertyAccessor(entity, instance); + var nullableNestedNullable = entity.getRequiredPersistentProperty("nullableNestedNullable"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(nullableNestedNullable)).isNull(); + + KClass nested = JvmClassMappingKt.getKotlinClass(MyNestedNullableValueClass.class); + KClass nullable = JvmClassMappingKt.getKotlinClass(MyNullableValueClass.class); + + MyNullableValueClass inner = nullable.getConstructors().iterator().next().call("new-value"); + MyNestedNullableValueClass outer = nested.getConstructors().iterator().next().call(inner); + + propertyAccessor.setProperty(nullableNestedNullable, outer); + assertThat(propertyAccessor.getProperty(nullableNestedNullable)).isInstanceOf(MyNestedNullableValueClass.class) + .hasToString("MyNestedNullableValueClass(id=MyNullableValueClass(id=new-value))"); + + var nestedNullable = entity.getRequiredPersistentProperty("nestedNullable"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(nestedNullable)).isNull(); + + propertyAccessor.setProperty(nestedNullable, "inner"); + assertThat(propertyAccessor.getProperty(nestedNullable)).isEqualTo("inner"); + } + + private Object createInstance(BasicPersistentEntity entity, + Function, Object> parameterProvider) { + return instantiators.getInstantiatorFor(entity).createInstance(entity, + new ParameterValueProvider() { + @Override + public T getParameterValue(Parameter parameter) { + return (T) parameterProvider.apply(parameter); + } + }); + } + + static Stream factories() { + return Stream.of(new ClassGeneratingPropertyAccessorFactory(), BeanWrapperPropertyAccessorFactory.INSTANCE); + } + +} 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 9742de0e02..317a328d6e 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt @@ -21,9 +21,12 @@ import org.springframework.data.annotation.PersistenceCreator * @author Mark Paluch */ @JvmInline -value class MyInlineValueClass(val id: String) +value class MyValueClass(val id: String) -data class WithMyInlineValueClass(val id: MyInlineValueClass) { +/** + * Simple class. Value class is flattened into `String`. However, `copy` requires a boxed value when called via reflection. + */ +data class WithMyValueClass(val id: MyValueClass) { // ByteCode explanation @@ -36,30 +39,34 @@ data class WithMyInlineValueClass(val id: MyInlineValueClass) { // synthetic constructor that we actually want to use // public synthetic WithMyValueClass(java.lang.String arg0, kotlin.jvm.internal.DefaultConstructorMarker arg1) {} // --------- + + // --------- + // public static WithMyValueClass copy-R7yrDNU$default(WithMyValueClass var0, String var1, int var2, Object var3) { + // --------- } @JvmInline -value class MyNullableInlineClass(val id: String? = "id") +value class MyNullableValueClass(val id: String? = "id") @JvmInline -value class MyNestedNullableInlineClass(val id: MyNullableInlineClass) +value class MyNestedNullableValueClass(val id: MyNullableValueClass) -class WithNestedMyNullableInlineClass( - val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( - MyNullableInlineClass("foo") - ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") +class WithNestedMyNullableValueClass( + var id: MyNestedNullableValueClass? = MyNestedNullableValueClass( + MyNullableValueClass("foo") + ), var baz: MyNullableValueClass? = MyNullableValueClass("id") // ByteCode explanation // --------- - // private WithNestedMyNullableInlineClass(MyNestedNullableInlineClass id, MyNullableInlineClass baz) {} + // private WithNestedMyNullableValueClass(MyNestedNullableValueClass id, MyNullableValueClass baz) {} // --------- // --------- // default constructor, detected by Discoverers.KOTLIN - // note that these constructors use boxed variants ("MyNestedNullableInlineClass") + // note that these constructors use boxed variants ("MyNestedNullableValueClass") - // public synthetic WithNestedMyNullableInlineClass(MyNestedNullableInlineClass id, MyNullableInlineClass baz, DefaultConstructorMarker $constructor_marker) {} + // public synthetic WithNestedMyNullableValueClass(MyNestedNullableValueClass id, MyNullableValueClass baz, DefaultConstructorMarker $constructor_marker) {} // --------- // --------- @@ -67,52 +74,106 @@ class WithNestedMyNullableInlineClass( // provide the defaulting mask. This constructor only gets generated when parameters are nullable, otherwise // Kotlin doesn't create this constructor - // public synthetic WithNestedMyNullableInlineClass(MyNestedNullableInlineClass var1, MyNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} + // public synthetic WithNestedMyNullableValueClass(MyNestedNullableValueClass var1, MyNullableValueClass var2, int var3, DefaultConstructorMarker var4) {} + // --------- +) + +/** + * Nullability on the domain class means that public and static copy methods uses both the boxed type. + */ +data class DataClassWithNullableValueClass( + + val fullyNullable: MyNullableValueClass? = MyNullableValueClass("id"), + val innerNullable: MyNullableValueClass = MyNullableValueClass("id") + + // ByteCode + + // private final MyNullableValueClass fullyNullable; + // private final String innerNullable; + + // --------- + // private DataClassWithNullableValueClass(MyNullableValueClass fullyNullable, String innerNullable) + // --------- + + // --------- + // public DataClassWithNullableValueClass(MyNullableValueClass var1, MyNullableValueClass var2, int var3, DefaultConstructorMarker var4) { + // --------- + + // --------- + // public final DataClassWithNullableValueClass copy-bwh045w (@Nullable MyNullableValueClass fullyNullable, @NotNull String innerNullable) { + // --------- + + // --------- + // public static DataClassWithNullableValueClass copy-bwh045w$default(DataClassWithNullableValueClass var0, MyNullableValueClass var1, MyNullableValueClass var2, int var3, Object var4) {} + // --------- +) + +/** + * Nullability on a nested level (domain class property isn't nullable, the inner value in the value class is) means that public copy method uses the value component type while the static copy method uses the boxed type. + * + * This creates a skew in getter vs. setter. The setter would be required to set the boxed value while the getter returns plain String. Sigh. + */ +data class DataClassWithNestedNullableValueClass( + val nullableNestedNullable: MyNestedNullableValueClass?, + val nestedNullable: MyNestedNullableValueClass + + // ByteCode + + // --------- + // public DataClassWithNestedNullableValueClass(MyNestedNullableValueClass nullableNestedNullable, String nestedNullable, DefaultConstructorMarker $constructor_marker) { + // --------- + + // --------- + // public final DataClassWithNestedNullableValueClass copy-W2GYjxM(@Nullable MyNestedNullableValueClass nullableNestedNullable, @NotNull String nestedNullable) { … } + // --------- + + // --------- + // public static DataClassWithNestedNullableValueClass copy-W2GYjxM$default(DataClassWithNestedNullableValueClass var0, MyNestedNullableValueClass var1, MyNestedNullableValueClass var2, int var3, Object var4) { // --------- ) -class WithInlineClassPreferredConstructor( - val id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( - MyNullableInlineClass("foo") - ), val baz: MyNullableInlineClass? = MyNullableInlineClass("id") +class WithValueClassPreferredConstructor( + val id: MyNestedNullableValueClass? = MyNestedNullableValueClass( + MyNullableValueClass("foo") + ), val baz: MyNullableValueClass? = MyNullableValueClass("id") ) { @PersistenceCreator constructor( - a: String, id: MyNestedNullableInlineClass? = MyNestedNullableInlineClass( - MyNullableInlineClass("foo") + a: String, id: MyNestedNullableValueClass? = MyNestedNullableValueClass( + MyNullableValueClass("foo") ) - ) : this(id, MyNullableInlineClass(a + "-pref")) { + ) : this(id, MyNullableValueClass(a + "-pref")) { } // ByteCode explanation // --------- - // private WithPreferredConstructor(MyNestedNullableInlineClass id, MyNullableInlineClass baz) {} + // private WithPreferredConstructor(MyNestedNullableValueClass id, MyNullableValueClass baz) {} // --------- // --------- - // public WithPreferredConstructor(MyNestedNullableInlineClass var1, MyNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} + // public WithPreferredConstructor(MyNestedNullableValueClass var1, MyNullableValueClass var2, int var3, DefaultConstructorMarker var4) {} // --------- // --------- - // private WithPreferredConstructor(String a, MyNestedNullableInlineClass id) {} + // private WithPreferredConstructor(String a, MyNestedNullableValueClass id) {} // --------- // --------- // this is the one we need to invoke to pass on the defaulting mask - // public synthetic WithPreferredConstructor(String var1, MyNestedNullableInlineClass var2, int var3, DefaultConstructorMarker var4) {} + // public synthetic WithPreferredConstructor(String var1, MyNestedNullableValueClass var2, int var3, DefaultConstructorMarker var4) {} // --------- // --------- - // public synthetic WithPreferredConstructor(MyNestedNullableInlineClass id, MyNullableInlineClass baz, DefaultConstructorMarker $constructor_marker) { + // public synthetic WithPreferredConstructor(MyNestedNullableValueClass id, MyNullableValueClass baz, DefaultConstructorMarker $constructor_marker) { // --------- // --------- // annotated constructor, detected by Discoverers.KOTLIN // @PersistenceCreator - // public WithPreferredConstructor(String a, MyNestedNullableInlineClass id, DefaultConstructorMarker $constructor_marker) { + // public WithPreferredConstructor(String a, MyNestedNullableValueClass id, DefaultConstructorMarker $constructor_marker) { // --------- } 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 f33a4becab..d1521694cc 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -162,7 +162,7 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { fun `should use default constructor for types using value class`() { every { provider.getParameterValue(any()) } returns "hello" - val instance = construct(WithMyInlineValueClass::class) + val instance = construct(WithMyValueClass::class) assertThat(instance.id.id).isEqualTo("hello") } @@ -172,7 +172,7 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { every { provider.getParameterValue(any()) } returns null - val instance = construct(WithNestedMyNullableInlineClass::class) + val instance = construct(WithNestedMyNullableValueClass::class) assertThat(instance.id?.id?.id).isEqualTo("foo") assertThat(instance.baz?.id).isEqualTo("id") @@ -183,7 +183,7 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { every { provider.getParameterValue(any()) }.returnsMany("Walter", null) - val instance = construct(WithInlineClassPreferredConstructor::class) + val instance = construct(WithValueClassPreferredConstructor::class) assertThat(instance.id?.id?.id).isEqualTo("foo") assertThat(instance.baz?.id).isEqualTo("Walter-pref") diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt index b225c83d61..96a74d3076 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt @@ -29,7 +29,7 @@ import kotlin.reflect.KClass * @author Mark Paluch * See also https://github.com/spring-projects/spring-framework/issues/28638 */ -class ReflectionEntityInstantiatorInlineClassUnitTests { +class ReflectionEntityInstantiatorValueClassUnitTests { val provider = mockk>() @@ -38,8 +38,8 @@ class ReflectionEntityInstantiatorInlineClassUnitTests { every { provider.getParameterValue(any()) }.returnsMany("Walter") - val instance: WithMyInlineValueClass = - construct(WithMyInlineValueClass::class) + val instance: WithMyValueClass = + construct(WithMyValueClass::class) assertThat(instance.id.id).isEqualTo("Walter") } @@ -49,8 +49,8 @@ class ReflectionEntityInstantiatorInlineClassUnitTests { every { provider.getParameterValue(any()) }.returnsMany("Walter", "") - val instance: WithNestedMyNullableInlineClass = - construct(WithNestedMyNullableInlineClass::class) + val instance: WithNestedMyNullableValueClass = + construct(WithNestedMyNullableValueClass::class) assertThat(instance.id?.id?.id).isEqualTo("Walter") assertThat(instance.baz?.id).isEqualTo("") @@ -61,7 +61,7 @@ class ReflectionEntityInstantiatorInlineClassUnitTests { every { provider.getParameterValue(any()) } returns null - val instance: WithNestedMyNullableInlineClass = construct(WithNestedMyNullableInlineClass::class) + val instance: WithNestedMyNullableValueClass = construct(WithNestedMyNullableValueClass::class) assertThat(instance.id?.id?.id).isEqualTo("foo") assertThat(instance.baz?.id).isEqualTo("id") @@ -72,7 +72,7 @@ class ReflectionEntityInstantiatorInlineClassUnitTests { every { provider.getParameterValue(any()) }.returnsMany("Walter", null) - val instance = construct(WithInlineClassPreferredConstructor::class) + val instance = construct(WithValueClassPreferredConstructor::class) assertThat(instance.id?.id?.id).isEqualTo("foo") assertThat(instance.baz?.id).isEqualTo("Walter-pref") diff --git a/src/test/kotlin/org/springframework/data/util/KotlinBeanInfoFactoryUnitTests.kt b/src/test/kotlin/org/springframework/data/util/KotlinBeanInfoFactoryUnitTests.kt new file mode 100644 index 0000000000..5f6fb1de93 --- /dev/null +++ b/src/test/kotlin/org/springframework/data/util/KotlinBeanInfoFactoryUnitTests.kt @@ -0,0 +1,77 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.util + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.BeanUtils + +/** + * Unit tests for [KotlinBeanInfoFactory]. + * @author Mark Paluch + */ +class KotlinBeanInfoFactoryUnitTests { + + @Test + internal fun determinesDataClassProperties() { + + val pds = BeanUtils.getPropertyDescriptors(SimpleDataClass::class.java) + + assertThat(pds).hasSize(2).extracting("name").contains("id", "name") + + for (pd in pds) { + if (pd.name == "id") { + assertThat(pd.readMethod.name).isEqualTo("getId") + assertThat(pd.writeMethod).isNull() + } + + if (pd.name == "name") { + assertThat(pd.readMethod.name).isEqualTo("getName") + assertThat(pd.writeMethod.name).isEqualTo("setName") + } + } + } + + @Test + internal fun determinesInlineClassConsumerProperties() { + + val pds = BeanUtils.getPropertyDescriptors(WithValueClass::class.java) + + assertThat(pds).hasSize(1).extracting("name").containsOnly("address") + assertThat(pds[0].readMethod.name).startsWith("getAddress-") // hashCode suffix + assertThat(pds[0].writeMethod.name).startsWith("setAddress-") // hashCode suffix + } + + @Test + internal fun determinesInlineClassWithDefaultingConsumerProperties() { + + val pds = BeanUtils.getPropertyDescriptors(WithOptionalValueClass::class.java) + + assertThat(pds).hasSize(1).extracting("name").containsOnly("address") + assertThat(pds[0].readMethod.name).startsWith("getAddress-") // hashCode suffix + assertThat(pds[0].writeMethod).isNull() + } + + data class SimpleDataClass(val id: String, var name: String) + + @JvmInline + value class EmailAddress(val address: String) + + data class WithValueClass(var address: EmailAddress) + + data class WithOptionalValueClass(val address: EmailAddress? = EmailAddress("un@known")) + +} From 51a0344fcd1fb0458c8d332ee6d8d20cfcb946ac Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 22 Jun 2023 15:14:26 +0200 Subject: [PATCH 07/11] Add support for value classes with generics. --- pom.xml | 1 + .../data/mapping/model/BeanWrapper.java | 2 +- ...lassGeneratingPropertyAccessorFactory.java | 14 +- .../data/mapping/model/KotlinCopyMethod.java | 7 +- .../model/KotlinInstantiationDelegate.java | 53 +--- .../data/mapping/model/KotlinValueBoxing.java | 69 ------ .../data/mapping/model/KotlinValueUtils.java | 228 ++++++++++++++++++ .../data/util/KotlinReflectionUtils.java | 35 --- .../KotlinPropertyAccessorFactoryTests.java | 86 +++++++ .../data/mapping/model/InlineClasses.kt | 14 ++ 10 files changed, 345 insertions(+), 164 deletions(-) delete mode 100644 src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java create mode 100644 src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java diff --git a/pom.xml b/pom.xml index 06c2edd3b3..ab0329ffb8 100644 --- a/pom.xml +++ b/pom.xml @@ -33,6 +33,7 @@ 2.11.7 1.4.24 spring.data.commons + 1.8 diff --git a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java index 004e87cd7d..d3169d3e27 100644 --- a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java +++ b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java @@ -190,7 +190,7 @@ private static Map getCallArgs(KCallable callable, Pe if (parameter.getKind() == Kind.VALUE && parameter.getName() != null && parameter.getName().equals(property.getName())) { - args.put(parameter, KotlinValueBoxing.getWrapper(parameter).apply(value)); + args.put(parameter, KotlinValueUtils.getValueHierarchy(parameter).wrap(value)); } } return args; 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 e4875142f7..d0a9f1824a 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -53,6 +53,7 @@ import org.springframework.data.mapping.SimpleAssociationHandler; import org.springframework.data.mapping.SimplePropertyHandler; import org.springframework.data.mapping.model.KotlinCopyMethod.KotlinCopyByProperty; +import org.springframework.data.mapping.model.KotlinValueUtils.ValueBoxing; import org.springframework.data.util.Optionals; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; @@ -84,7 +85,7 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert 32); private ConcurrentLruCache, Function> wrapperCache = new ConcurrentLruCache<>( - 256, KotlinValueClassBoxingWrapper::getWrapper); + 256, KotlinValueClassBoxingAdapter::getWrapper); @Override public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity entity, T bean) { @@ -110,7 +111,7 @@ public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity accessor = (PersistentPropertyAccessor) constructor.newInstance(args); if (KotlinDetector.isKotlinType(entity.getType())) { - return new KotlinValueClassBoxingWrapper<>(entity, accessor, wrapperCache); + return new KotlinValueClassBoxingAdapter<>(entity, accessor, wrapperCache); } return accessor; @@ -1463,14 +1464,14 @@ private static boolean hasKotlinCopyMethod(PersistentProperty property) { } /** - * Wrapper to encapsulate Kotlin's value class boxing when properties are nullable. + * Adapter to encapsulate Kotlin's value class boxing when properties are nullable. * * @param entity * @param delegate * @param wrapperCache * @param */ - record KotlinValueClassBoxingWrapper (PersistentEntity entity, PersistentPropertyAccessor delegate, + record KotlinValueClassBoxingAdapter (PersistentEntity entity, PersistentPropertyAccessor delegate, ConcurrentLruCache, Function> wrapperCache) implements PersistentPropertyAccessor { @@ -1496,14 +1497,13 @@ static Function getWrapper(PersistentProperty property) { .filter(kf -> kf.getName().equals(property.getName())) // .findFirst(); - Function objectObjectFunction = kParameter.map(KotlinValueBoxing::getWrapper) - .orElse(Function.identity()); + ValueBoxing vh = kParameter.map(KotlinValueUtils::getValueHierarchy).orElse(null); KotlinCopyByProperty kotlinCopyByProperty = copyMethod.forProperty(property).get(); Method copy = copyMethod.getSyntheticCopyMethod(); Parameter parameter = copy.getParameters()[kotlinCopyByProperty.getParameterPosition()]; - return o -> ClassUtils.isAssignableValue(parameter.getType(), o) ? o : objectObjectFunction.apply(o); + return o -> ClassUtils.isAssignableValue(parameter.getType(), o) || vh == null ? o : vh.wrap(o); } return Function.identity(); diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java index 6e0542f7ef..0794dbdadf 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java @@ -38,7 +38,6 @@ import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.SimplePropertyHandler; -import org.springframework.data.util.KotlinReflectionUtils; import org.springframework.util.Assert; /** @@ -172,7 +171,7 @@ private static Optional findPublicCopyMethod(Method defaultKotlinMethod) return Optional.empty(); } - boolean usesValueClasses = KotlinReflectionUtils.hasValueClassProperty(type); + boolean usesValueClasses = KotlinValueUtils.hasValueClassProperty(type); List constructorArguments = getComponentArguments(primaryConstructor); Predicate isCopyMethod; @@ -243,7 +242,7 @@ private static Optional findSyntheticCopyMethod(Class type) { return Optional.empty(); } - boolean usesValueClasses = KotlinReflectionUtils.hasValueClassProperty(type); + boolean usesValueClasses = KotlinValueUtils.hasValueClassProperty(type); Predicate isCopyMethod = usesValueClasses ? (it -> it.startsWith("copy-") && it.endsWith("$default")) : (it -> it.equals("copy$default")); @@ -278,7 +277,7 @@ private static boolean matchesPrimaryConstructor(Class[] parameterTypes, KFun KParameter kParameter = constructorArguments.get(i); - if (KotlinReflectionUtils.isValueClass(kParameter.getType())) { + if (KotlinValueUtils.isValueClass(kParameter.getType())) { // sigh. This can require deep unwrapping because the public vs. the synthetic copy methods use different // parameter types. continue; diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java index 96e920ba92..3a3d4b0bc9 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java @@ -15,14 +15,11 @@ */ package org.springframework.data.mapping.model; -import kotlin.jvm.JvmClassMappingKt; -import kotlin.reflect.KClass; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; import kotlin.reflect.jvm.ReflectJvmMapping; import java.lang.reflect.Constructor; -import java.lang.reflect.Type; import java.util.ArrayList; import java.util.List; import java.util.function.Function; @@ -32,6 +29,7 @@ import org.springframework.data.mapping.Parameter; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PreferredConstructor; +import org.springframework.data.mapping.model.KotlinValueUtils.ValueBoxing; import org.springframework.data.util.ReflectionUtils; import org.springframework.lang.Nullable; @@ -68,34 +66,10 @@ public KotlinInstantiationDelegate(PreferredConstructor preferredConstruct this.hasDefaultConstructorMarker = hasDefaultConstructorMarker(constructorToInvoke.getParameters()); for (KParameter kParameter : kParameters) { - wrappers.add(getWrapper(kParameter, true)); - } - } - - /** - * @param kParameter the kotlin parameter to wrap. - * @param domainTypeUsage optional in the domain type require value type casting. Inner/nested ones don't. This is - * because calling the synthetic constructor with an optional parameter requires an inline class while - * optional parameters via reflection are handled within Kotlin-Reflection. - * @return - */ - private Function getWrapper(KParameter kParameter, boolean domainTypeUsage) { - - if (kParameter.getType().getClassifier()instanceof KClass kc && kc.isValue() - && (!domainTypeUsage || kParameter.isOptional())) { - - KFunction ctor = kc.getConstructors().iterator().next(); - - // using reflection to construct a value class wrapper. Everything - // else would require too many levels of indirections. - - KParameter nested = ctor.getParameters().get(0); - Function wrapper = getWrapper(nested, false); - return o -> ctor.call(wrapper.apply(o)); + ValueBoxing valueBoxing = KotlinValueUtils.getValueHierarchy(kParameter); + wrappers.add(valueBoxing::wrap); } - - return Function.identity(); } static boolean hasDefaultConstructorMarker(java.lang.reflect.Parameter[] parameters) { @@ -269,25 +243,8 @@ static boolean parametersMatch(java.lang.reflect.Parameter constructorParameter, // candidate can be also a wrapper - Class aClass = unwrapValueClass(candidateParameter.getType()); + Class componentType = KotlinValueUtils.getValueHierarchy(candidateParameter.getType()).getActualType(); - return constructorParameter.getType().equals(aClass); + return constructorParameter.getType().equals(componentType); } - - private static Class unwrapValueClass(Class type) { - - KClass kotlinClass = JvmClassMappingKt.getKotlinClass(type); - if (kotlinClass != null && kotlinClass.isValue()) { - - KFunction next = kotlinClass.getConstructors().iterator().next(); - KParameter kParameter = next.getParameters().get(0); - Type javaType = ReflectJvmMapping.getJavaType(kParameter.getType()); - if (javaType instanceof Class) { - return unwrapValueClass((Class) javaType); - } - } - - return type; - } - } diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java b/src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java deleted file mode 100644 index e879ce82c6..0000000000 --- a/src/main/java/org/springframework/data/mapping/model/KotlinValueBoxing.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright 2023 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.mapping.model; - -import kotlin.reflect.KClass; -import kotlin.reflect.KFunction; -import kotlin.reflect.KParameter; - -import java.util.function.Function; - -import org.springframework.data.util.KotlinReflectionUtils; - -/** - * Utility to box values into inline class instances. - * - * @author Mark Paluch - * @since 3.2 - */ -class KotlinValueBoxing { - - /** - * @param kParameter the kotlin parameter to wrap. - * @return a wrapper function that potentially wraps (value-boxing) values into value classes if these are nullable or - * used in a lower part of the type hierarchy. - */ - static Function getWrapper(KParameter kParameter) { - return getWrapper(kParameter, true); - } - - /** - * @param kParameter the kotlin parameter to wrap. - * @param domainTypeUsage optional in the domain type require value type casting. Inner/nested ones don't. This is - * because calling the synthetic constructor with an optional parameter requires an inline class while - * optional parameters via reflection are handled within Kotlin-Reflection. - * @return a wrapper function that potentially wraps (value-boxing) values into value classes if these are nullable or - * used in a lower part of the type hierarchy. - */ - private static Function getWrapper(KParameter kParameter, boolean domainTypeUsage) { - - if (KotlinReflectionUtils.isValueClass(kParameter.getType()) && (!domainTypeUsage || kParameter.isOptional())) { - - KClass kClass = (KClass) kParameter.getType().getClassifier(); - KFunction ctor = kClass.getConstructors().iterator().next(); - - // using reflection to construct a value class wrapper. Everything - // else would require too many levels of indirections. - - KParameter nested = ctor.getParameters().get(0); - Function wrapper = getWrapper(nested, false); - - return o -> kClass.isInstance(o) ? o : ctor.call(wrapper.apply(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 new file mode 100644 index 0000000000..731c889274 --- /dev/null +++ b/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java @@ -0,0 +1,228 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model; + +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KCallable; +import kotlin.reflect.KClass; +import kotlin.reflect.KFunction; +import kotlin.reflect.KParameter; +import kotlin.reflect.KProperty; +import kotlin.reflect.KType; +import kotlin.reflect.KTypeParameter; + +import org.springframework.core.KotlinDetector; +import org.springframework.lang.Nullable; + +/** + * Utilities for Kotlin Value class support. + * + * @author Mark Paluch + * @since 3.2 + */ +class KotlinValueUtils { + + /** + * Returns whether the given {@link KType} is a {@link KClass#isValue() value} class. + * + * @param type the kotlin type to inspect. + * @return {@code true} the type is a value class. + */ + public static boolean isValueClass(KType type) { + return type.getClassifier()instanceof KClass kc && kc.isValue(); + } + + /** + * Returns whether the given class makes uses Kotlin {@link KClass#isValue() value} classes. + * + * @param type the kotlin type to inspect. + * @return {@code true} when at least one property uses Kotlin value classes. + */ + public static boolean hasValueClassProperty(Class type) { + + if (!KotlinDetector.isKotlinType(type)) { + return false; + } + + KClass kotlinClass = JvmClassMappingKt.getKotlinClass(type); + + for (KCallable member : kotlinClass.getMembers()) { + if (member instanceof KProperty kp && isValueClass(kp.getReturnType())) { + return true; + } + } + + return false; + } + + /** + * Creates a value hierarchy across value types from a given {@link KParameter}. + * + * @param parameter the parameter that references the value class hierarchy. + * @return + */ + public static ValueBoxing getValueHierarchy(KParameter parameter) { + return ValueBoxing.of(parameter); + } + + /** + * Creates a value hierarchy across value types from a given {@link KParameter}. + * + * @param cls the entrypoint of the type hierarchy + * @return + */ + public static ValueBoxing getValueHierarchy(Class cls) { + + KClass kotlinClass = JvmClassMappingKt.getKotlinClass(cls); + return ValueBoxing.of(kotlinClass); + } + + /** + * Utility to represent Kotlin value class boxing. + */ + static class ValueBoxing { + + private final KClass kClass; + + private final KFunction wrapperConstructor; + + private final boolean applyBoxing; + + private final @Nullable ValueBoxing next; + + /** + * @param type the referenced type. + * @param optional whether the type is optional. + */ + private ValueBoxing(KType type, boolean optional) { + this((KClass) type.getClassifier(), optional, true); + } + + private ValueBoxing(KClass kClass, boolean optional, boolean domainTypeUsage) { + + KFunction wrapperConstructor = null; + ValueBoxing next = null; + + boolean applyBoxing = (optional || !domainTypeUsage); + if (kClass.isValue()) { + + wrapperConstructor = kClass.getConstructors().iterator().next(); + KParameter nested = wrapperConstructor.getParameters().get(0); + KType nestedType = nested.getType(); + KClass nestedClass; + + // bound flattening + if (nestedType.getClassifier()instanceof KTypeParameter ktp) { + nestedClass = getUpperBound(ktp); + } else { + nestedClass = (KClass) nestedType.getClassifier(); + } + + next = new ValueBoxing(nestedClass, nested.isOptional(), false); + } + + this.kClass = kClass; + this.wrapperConstructor = wrapperConstructor; + this.next = next; + this.applyBoxing = applyBoxing; + } + + private static KClass getUpperBound(KTypeParameter typeParameter) { + + for (KType upperBound : typeParameter.getUpperBounds()) { + if (upperBound.getClassifier()instanceof KClass kc) { + return kc; + } + } + + throw new IllegalArgumentException("No upper bounds found"); + } + + /** + * Creates a new {@link ValueBoxing} for a {@link KParameter}. + * + * @param parameter + * @return + */ + static ValueBoxing of(KParameter parameter) { + return new ValueBoxing(parameter.getType(), parameter.isOptional()); + } + + /** + * Creates a new {@link ValueBoxing} for a {@link KClass} assuming the class is the uppermost entrypoint and not a + * value class. + * + * @param kotlinClass + * @return + */ + public static ValueBoxing of(KClass kotlinClass) { + return new ValueBoxing(kotlinClass, false, !kotlinClass.isValue()); + } + + public Class getActualType() { + + if (isValueClass() && hasNext()) { + return next.getActualType(); + } + + return JvmClassMappingKt.getJavaClass(kClass); + } + + public boolean isValueClass() { + return kClass.isValue(); + } + + public boolean hasNext() { + return next != null; + } + + @Nullable + public ValueBoxing getNext() { + return next; + } + + @Nullable + public Object wrap(@Nullable Object o) { + + if (applyBoxing) { + return next == null || kClass.isInstance(o) ? o : wrapperConstructor.call(next.wrap(o)); + } + + return o; + } + + @Override + public String toString() { + + StringBuilder sb = new StringBuilder(); + + ValueBoxing hierarchy = this; + while (hierarchy != null) { + + if (sb.length() != 0) { + sb.append(" -> "); + } + + sb.append(hierarchy.kClass.getSimpleName()); + hierarchy = hierarchy.next; + } + + return sb.toString(); + } + + } + +} diff --git a/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java b/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java index 9dd7672dd7..56bc7c26e0 100644 --- a/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java +++ b/src/main/java/org/springframework/data/util/KotlinReflectionUtils.java @@ -83,41 +83,6 @@ public static boolean isDataClass(Class type) { return kotlinClass.isData(); } - /** - * Returns whether the given {@link KType} is a {@link KClass#isValue() value} class. - * - * @param type the kotlin type to inspect. - * @return {@code true} the type is a value class. - * @since 3.2 - */ - public static boolean isValueClass(KType type) { - return type.getClassifier()instanceof KClass kc && kc.isValue(); - } - - /** - * Returns whether the given class makes uses Kotlin {@link KClass#isValue() value} classes. - * - * @param type the kotlin type to inspect. - * @return {@code true} when at least one property uses Kotlin value classes. - * @since 3.2 - */ - public static boolean hasValueClassProperty(Class type) { - - if (!KotlinDetector.isKotlinType(type)) { - return false; - } - - KClass kotlinClass = JvmClassMappingKt.getKotlinClass(type); - - for (KCallable member : kotlinClass.getMembers()) { - if (member instanceof KProperty kp && isValueClass(kp.getReturnType())) { - return true; - } - } - - return false; - } - /** * Returns a {@link KFunction} instance corresponding to the given Java {@link Method} instance, or {@code null} if * this method cannot be represented by a Kotlin function. 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 69800fe2ac..e5e73598cc 100644 --- a/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java +++ b/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java @@ -19,6 +19,7 @@ import kotlin.jvm.JvmClassMappingKt; import kotlin.reflect.KClass; +import kotlin.reflect.jvm.internal.KotlinReflectionInternalError; import java.util.function.Function; import java.util.stream.Stream; @@ -106,6 +107,15 @@ void shouldGeneratePropertyAccessorForDataClassWithNullableValueClass(Persistent assertThat(propertyAccessor).isNotNull(); assertThat(propertyAccessor.getProperty(fullyNullable)).isEqualTo(expectedDefaultValue); + if (factory instanceof BeanWrapperPropertyAccessorFactory) { + + // see https://youtrack.jetbrains.com/issue/KT-57357 + assertThatExceptionOfType(KotlinReflectionInternalError.class) + .isThrownBy(() -> propertyAccessor.setProperty(fullyNullable, barValue)) + .withMessageContaining("This callable does not support a default call"); + return; + } + propertyAccessor.setProperty(fullyNullable, barValue); assertThat(propertyAccessor.getProperty(fullyNullable)).isEqualTo(barValue); @@ -139,6 +149,15 @@ void nestedNullablePropertiesShouldBeSetCorrectly(PersistentPropertyAccessorFact MyNullableValueClass inner = nullable.getConstructors().iterator().next().call("new-value"); MyNestedNullableValueClass outer = nested.getConstructors().iterator().next().call(inner); + if (factory instanceof BeanWrapperPropertyAccessorFactory) { + + // see https://youtrack.jetbrains.com/issue/KT-57357 + assertThatExceptionOfType(KotlinReflectionInternalError.class) + .isThrownBy(() -> propertyAccessor.setProperty(nullableNestedNullable, outer)) + .withMessageContaining("This callable does not support a default call"); + return; + } + propertyAccessor.setProperty(nullableNestedNullable, outer); assertThat(propertyAccessor.getProperty(nullableNestedNullable)).isInstanceOf(MyNestedNullableValueClass.class) .hasToString("MyNestedNullableValueClass(id=MyNullableValueClass(id=new-value))"); @@ -152,6 +171,73 @@ void nestedNullablePropertiesShouldBeSetCorrectly(PersistentPropertyAccessorFact assertThat(propertyAccessor.getProperty(nestedNullable)).isEqualTo("inner"); } + @MethodSource("factories") + @ParameterizedTest // GH-2806 + void genericInlineClassesShouldWork(PersistentPropertyAccessorFactory factory) { + + BasicPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(WithGenericValue.class); + + Object instance = createInstance(entity, parameter -> "aaa"); + + var propertyAccessor = factory.getPropertyAccessor(entity, instance); + var string = entity.getRequiredPersistentProperty("string"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(string)).isEqualTo("aaa"); + + if (factory instanceof BeanWrapperPropertyAccessorFactory) { + + // see https://youtrack.jetbrains.com/issue/KT-57357 + assertThatExceptionOfType(KotlinReflectionInternalError.class) + .isThrownBy(() -> propertyAccessor.setProperty(string, "string")) + .withMessageContaining("This callable does not support a default call"); + return; + } + + propertyAccessor.setProperty(string, "string"); + assertThat(propertyAccessor.getProperty(string)).isEqualTo("string"); + + var charseq = entity.getRequiredPersistentProperty("charseq"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(charseq)).isEqualTo("aaa"); + propertyAccessor.setProperty(charseq, "charseq"); + assertThat(propertyAccessor.getProperty(charseq)).isEqualTo("charseq"); + + var recursive = entity.getRequiredPersistentProperty("recursive"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(recursive)).isEqualTo("aaa"); + propertyAccessor.setProperty(recursive, "recursive"); + assertThat(propertyAccessor.getProperty(recursive)).isEqualTo("recursive"); + } + + @MethodSource("factories") + @ParameterizedTest // GH-2806 + void genericNullableInlineClassesShouldWork(PersistentPropertyAccessorFactory factory) { + + BasicPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(WithGenericNullableValue.class); + + KClass genericClass = JvmClassMappingKt.getKotlinClass(MyGenericValue.class); + MyGenericValue inner = genericClass.getConstructors().iterator().next().call("initial-value"); + MyGenericValue outer = genericClass.getConstructors().iterator().next().call(inner); + + MyGenericValue newInner = genericClass.getConstructors().iterator().next().call("new-value"); + MyGenericValue newOuter = genericClass.getConstructors().iterator().next().call(newInner); + + Object instance = createInstance(entity, parameter -> outer); + + var propertyAccessor = factory.getPropertyAccessor(entity, instance); + var recursive = entity.getRequiredPersistentProperty("recursive"); + + assertThat(propertyAccessor).isNotNull(); + assertThat(propertyAccessor.getProperty(recursive)).isEqualTo(outer); + propertyAccessor.setProperty(recursive, newOuter); + assertThat(propertyAccessor.getProperty(recursive)).isEqualTo(newOuter); + } + 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 317a328d6e..5213bac5d5 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt @@ -51,6 +51,20 @@ value class MyNullableValueClass(val id: String? = "id") @JvmInline value class MyNestedNullableValueClass(val id: MyNullableValueClass) +@JvmInline +value class MyGenericValue(val id: T) + +@JvmInline +value class MyGenericBoundValue(val id: T) + +data class WithGenericValue( + val string: MyGenericBoundValue, + val charseq: MyGenericBoundValue, + val recursive: MyGenericValue> +) + +data class WithGenericNullableValue(val recursive: MyGenericValue>?) + class WithNestedMyNullableValueClass( var id: MyNestedNullableValueClass? = MyNestedNullableValueClass( MyNullableValueClass("foo") From 1d3c27ece2e37b256a5949f9d140d56a8f5b29de Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 22 Jun 2023 15:30:33 +0200 Subject: [PATCH 08/11] Cleanup unwanted changes. --- ...lassGeneratingPropertyAccessorFactory.java | 41 +++++++++++-------- .../model/ReflectionEntityInstantiator.java | 1 + 2 files changed, 26 insertions(+), 16 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 d0a9f1824a..095de97251 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -84,8 +84,8 @@ public class ClassGeneratingPropertyAccessorFactory implements PersistentPropert private volatile Map, Class>> propertyAccessorClasses = new HashMap<>( 32); - private ConcurrentLruCache, Function> wrapperCache = new ConcurrentLruCache<>( - 256, KotlinValueClassBoxingAdapter::getWrapper); + private final ConcurrentLruCache, Function> wrapperCache = new ConcurrentLruCache<>( + 256, KotlinValueBoxingAdapter::getWrapper); @Override public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity entity, T bean) { @@ -111,19 +111,19 @@ public PersistentPropertyAccessor getPropertyAccessor(PersistentEntity accessor = (PersistentPropertyAccessor) constructor.newInstance(args); if (KotlinDetector.isKotlinType(entity.getType())) { - return new KotlinValueClassBoxingAdapter<>(entity, accessor, wrapperCache); + return new KotlinValueBoxingAdapter<>(entity, accessor, wrapperCache); } return accessor; } catch (Exception e) { - throw new IllegalArgumentException(String.format("Cannot create persistent property delegate for %s", entity), e); + throw new IllegalArgumentException(String.format("Cannot create persistent property accessor for %s", entity), e); } finally { args[0] = null; } } /** - * Checks whether an delegate class can be generated. + * Checks whether an accessor class can be generated. * * @param entity must not be {@literal null}. * @return {@literal true} if the runtime is equal or greater to Java 1.7, we can access the ClassLoader, the property @@ -223,8 +223,8 @@ private Class> createAccessorClass(PersistentEntit * well. Accessing properties using generated accessors imposes some constraints: *
    *
  • Runtime must be Java 7 or higher
    • - *
  • The generated delegate decides upon generation whether to use field or property access for particular - * properties. It's not possible to change the access method once the delegate class is generated.
    • + *
  • The generated accessor decides upon generation whether to use field or property access for particular + * properties. It's not possible to change the access method once the accessor class is generated.
    • *
  • Property names and their {@link String#hashCode()} must be unique within a {@link PersistentEntity}.
    • *
* These constraints apply to retain the performance gains, otherwise the generated code has to decide which method @@ -278,7 +278,7 @@ private Class> createAccessorClass(PersistentEntit * // … * } * throw new UnsupportedOperationException( - * String.format("No delegate to set property %s", new Object[] { property })); + * String.format("No accessor to set property %s", new Object[] { property })); * } * * public Object getProperty(PersistentProperty property) { @@ -293,7 +293,7 @@ private Class> createAccessorClass(PersistentEntit * return bean.field; * // … * throw new UnsupportedOperationException( - * String.format("No delegate to get property %s", new Object[] { property })); + * String.format("No accessor to get property %s", new Object[] { property })); * } * } * } @@ -794,7 +794,7 @@ private static void visitGetProperty(PersistentEntity entity, visitGetPropertySwitch(entity, persistentProperties, internalClassName, mv); mv.visitLabel(l1); - visitThrowUnsupportedOperationException(mv, "No delegate to get property %s"); + visitThrowUnsupportedOperationException(mv, "No accessor to get property %s"); mv.visitLocalVariable(THIS_REF, referenceName(internalClassName), null, l0, l1, 0); mv.visitLocalVariable("property", referenceName(PERSISTENT_PROPERTY), @@ -929,7 +929,7 @@ private static void visitGetProperty0(PersistentEntity entity, PersistentP * // … * } * throw new UnsupportedOperationException( - * String.format("No delegate to set property %s", new Object[] { property })); + * String.format("No accessor to set property %s", new Object[] { property })); * } * */ @@ -957,7 +957,7 @@ private static void visitSetProperty(PersistentEntity entity, Label l1 = new Label(); mv.visitLabel(l1); - visitThrowUnsupportedOperationException(mv, "No delegate to set property %s"); + visitThrowUnsupportedOperationException(mv, "No accessor to set property %s"); mv.visitLocalVariable(THIS_REF, referenceName(internalClassName), null, l0, l1, 0); mv.visitLocalVariable("property", "Lorg/springframework/data/mapping/PersistentProperty;", @@ -1466,12 +1466,13 @@ private static boolean hasKotlinCopyMethod(PersistentProperty property) { /** * Adapter to encapsulate Kotlin's value class boxing when properties are nullable. * - * @param entity - * @param delegate - * @param wrapperCache + * @param entity the entity that could use value class properties. + * @param delegate the property accessor to delegate to. + * @param wrapperCache cache for wrapping functions. * @param + * @since 3.2 */ - record KotlinValueClassBoxingAdapter (PersistentEntity entity, PersistentPropertyAccessor delegate, + record KotlinValueBoxingAdapter (PersistentEntity entity, PersistentPropertyAccessor delegate, ConcurrentLruCache, Function> wrapperCache) implements PersistentPropertyAccessor { @@ -1481,6 +1482,14 @@ public void setProperty(PersistentProperty property, @Nullable Object value) delegate.setProperty(property, wrapperCache.get(property).apply(value)); } + /** + * Create a wrapper function if the {@link PersistentProperty} uses value classes. + * + * @param property the persistent property to inspect. + * @return a wrapper function to wrap a value class component into the hierarchy of value classes or + * {@link Function#identity()} if wrapping is not necessary. + * @see KotlinValueUtils#getValueHierarchy(Class) + */ static Function getWrapper(PersistentProperty property) { Optional kotlinCopyMethod = KotlinCopyMethod.findCopyMethod(property.getOwner().getType()) diff --git a/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java b/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java index 840ddd2af6..ba841daf59 100644 --- a/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java +++ b/src/main/java/org/springframework/data/mapping/model/ReflectionEntityInstantiator.java @@ -57,6 +57,7 @@ public , P extends PersistentPrope return instantiateClass(entity); } + // workaround as classes using value classes cannot be instantiated through BeanUtils. if (KotlinDetector.isKotlinReflectPresent() && KotlinReflectionUtils.isSupportedKotlinClass(entity.getType()) && creator instanceof PreferredConstructor constructor) { From 8a6ba2ed56ac9f36c43e96a45dfb25af44a69318 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 27 Jun 2023 16:41:48 +0200 Subject: [PATCH 09/11] Refine wrapping rules for copy and constructor usage. --- pom.xml | 6 + .../data/mapping/model/BeanWrapper.java | 2 +- ...lassGeneratingPropertyAccessorFactory.java | 4 +- .../InstanceCreatorMetadataDiscoverer.java | 37 ++- .../model/KotlinInstantiationDelegate.java | 8 +- .../data/mapping/model/KotlinValueUtils.java | 239 +++++++++++++++--- .../model/PreferredConstructorDiscoverer.java | 1 - ...nceCreatorMetadataDiscovererUnitTests.java | 43 ++++ .../KotlinPropertyAccessorFactoryTests.java | 25 +- .../data/mapping/model/InlineClasses.kt | 66 +++++ ...ssGeneratingEntityInstantiatorUnitTests.kt | 42 +++ .../model/KotlinValueUtilsUnitTests.kt | 192 ++++++++++++++ ...PreferredConstructorDiscovererUnitTests.kt | 29 ++- ...ionEntityInstantiatorDataClassUnitTests.kt | 10 + ...nEntityInstantiatorInlineClassUnitTests.kt | 12 - 15 files changed, 645 insertions(+), 71 deletions(-) create mode 100644 src/test/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscovererUnitTests.java create mode 100644 src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt diff --git a/pom.xml b/pom.xml index ab0329ffb8..c99cf2a2b8 100644 --- a/pom.xml +++ b/pom.xml @@ -127,6 +127,12 @@ test + + org.jetbrains.kotlin + kotlin-maven-plugin + ${kotlin} + + diff --git a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java index d3169d3e27..3bd3ee60bc 100644 --- a/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java +++ b/src/main/java/org/springframework/data/mapping/model/BeanWrapper.java @@ -190,7 +190,7 @@ private static Map getCallArgs(KCallable callable, Pe if (parameter.getKind() == Kind.VALUE && parameter.getName() != null && parameter.getName().equals(property.getName())) { - args.put(parameter, KotlinValueUtils.getValueHierarchy(parameter).wrap(value)); + args.put(parameter, KotlinValueUtils.getCopyValueHierarchy(parameter).wrap(value)); } } return args; 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 095de97251..b6e26ca4ca 100644 --- a/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java +++ b/src/main/java/org/springframework/data/mapping/model/ClassGeneratingPropertyAccessorFactory.java @@ -1488,7 +1488,7 @@ public void setProperty(PersistentProperty property, @Nullable Object value) * @param property the persistent property to inspect. * @return a wrapper function to wrap a value class component into the hierarchy of value classes or * {@link Function#identity()} if wrapping is not necessary. - * @see KotlinValueUtils#getValueHierarchy(Class) + * @see KotlinValueUtils#getCopyValueHierarchy(KParameter) */ static Function getWrapper(PersistentProperty property) { @@ -1506,7 +1506,7 @@ static Function getWrapper(PersistentProperty property) { .filter(kf -> kf.getName().equals(property.getName())) // .findFirst(); - ValueBoxing vh = kParameter.map(KotlinValueUtils::getValueHierarchy).orElse(null); + ValueBoxing vh = kParameter.map(KotlinValueUtils::getCopyValueHierarchy).orElse(null); KotlinCopyByProperty kotlinCopyByProperty = copyMethod.forProperty(property).get(); Method copy = copyMethod.getSyntheticCopyMethod(); diff --git a/src/main/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscoverer.java b/src/main/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscoverer.java index ceabbc9d24..fd06866ebf 100644 --- a/src/main/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscoverer.java +++ b/src/main/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscoverer.java @@ -15,6 +15,11 @@ */ package org.springframework.data.mapping.model; +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KCallable; +import kotlin.reflect.KClass; +import kotlin.reflect.KProperty; + import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Constructor; @@ -24,6 +29,7 @@ import java.util.List; import org.springframework.core.DefaultParameterNameDiscoverer; +import org.springframework.core.KotlinDetector; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.data.annotation.PersistenceCreator; @@ -55,7 +61,8 @@ class InstanceCreatorMetadataDiscoverer { * @return */ @Nullable - public static > InstanceCreatorMetadata

discover(PersistentEntity entity) { + public static > InstanceCreatorMetadata

discover( + PersistentEntity entity) { Constructor[] declaredConstructors = entity.getType().getDeclaredConstructors(); Method[] declaredMethods = entity.getType().getDeclaredMethods(); @@ -78,6 +85,34 @@ public static > InstanceCreatorMetadata

di } } + if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isKotlinType(entity.getType())) { + + KClass kClass = JvmClassMappingKt.getKotlinClass(entity.getType()); + // We use box-impl as factory for classes. + if (kClass.isValue()) { + + String propertyName = ""; + for (KCallable member : kClass.getMembers()) { + if (member instanceof KProperty) { + propertyName = member.getName(); + break; + } + } + + for (Method declaredMethod : entity.getType().getDeclaredMethods()) { + if (declaredMethod.getName().equals("box-impl") && declaredMethod.isSynthetic() + && declaredMethod.getParameterCount() == 1) { + + Annotation[][] parameterAnnotations = declaredMethod.getParameterAnnotations(); + List> types = entity.getTypeInformation().getParameterTypes(declaredMethod); + + return new FactoryMethod<>(declaredMethod, + new Parameter<>(propertyName, (TypeInformation) types.get(0), parameterAnnotations[0], entity)); + } + } + } + } + return PreferredConstructorDiscoverer.discover(entity); } diff --git a/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java index 3a3d4b0bc9..c3b1bc09ac 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinInstantiationDelegate.java @@ -67,7 +67,7 @@ public KotlinInstantiationDelegate(PreferredConstructor preferredConstruct for (KParameter kParameter : kParameters) { - ValueBoxing valueBoxing = KotlinValueUtils.getValueHierarchy(kParameter); + ValueBoxing valueBoxing = KotlinValueUtils.getConstructorValueHierarchy(kParameter); wrappers.add(valueBoxing::wrap); } } @@ -242,9 +242,9 @@ static boolean parametersMatch(java.lang.reflect.Parameter constructorParameter, } // candidate can be also a wrapper + Class componentOrWrapperType = KotlinValueUtils.getConstructorValueHierarchy(candidateParameter.getType()) + .getActualType(); - Class componentType = KotlinValueUtils.getValueHierarchy(candidateParameter.getType()).getActualType(); - - return constructorParameter.getType().equals(componentType); + return constructorParameter.getType().equals(componentOrWrapperType); } } 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 731c889274..02ea07b779 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java +++ b/src/main/java/org/springframework/data/mapping/model/KotlinValueUtils.java @@ -16,6 +16,7 @@ package org.springframework.data.mapping.model; import kotlin.jvm.JvmClassMappingKt; +import kotlin.jvm.internal.Reflection; import kotlin.reflect.KCallable; import kotlin.reflect.KClass; import kotlin.reflect.KFunction; @@ -23,9 +24,16 @@ import kotlin.reflect.KProperty; import kotlin.reflect.KType; import kotlin.reflect.KTypeParameter; +import kotlin.reflect.jvm.ReflectJvmMapping; + +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.springframework.core.KotlinDetector; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * Utilities for Kotlin Value class support. @@ -69,25 +77,135 @@ public static boolean hasValueClassProperty(Class type) { } /** - * Creates a value hierarchy across value types from a given {@link KParameter}. + * Creates a value hierarchy across value types from a given {@link KParameter} for COPY method usage. * * @param parameter the parameter that references the value class hierarchy. * @return */ - public static ValueBoxing getValueHierarchy(KParameter parameter) { - return ValueBoxing.of(parameter); + public static ValueBoxing getCopyValueHierarchy(KParameter parameter) { + return new ValueBoxing(BoxingRules.COPY, parameter); } /** - * Creates a value hierarchy across value types from a given {@link KParameter}. + * Creates a value hierarchy across value types from a given {@link KParameter} for constructor usage. * - * @param cls the entrypoint of the type hierarchy - * @return + * @param parameter the parameter that references the value class hierarchy. + * @return the {@link ValueBoxing} type hierarchy. */ - public static ValueBoxing getValueHierarchy(Class cls) { + public static ValueBoxing getConstructorValueHierarchy(KParameter parameter) { + return new ValueBoxing(BoxingRules.CONSTRUCTOR, parameter); + } + + /** + * Creates a value hierarchy across value types from a given {@link KParameter} for constructor usage. + * + * @param cls the entrypoint of the type hierarchy. + * @return the {@link ValueBoxing} type hierarchy. + */ + public static ValueBoxing getConstructorValueHierarchy(Class cls) { KClass kotlinClass = JvmClassMappingKt.getKotlinClass(cls); - return ValueBoxing.of(kotlinClass); + return new ValueBoxing(BoxingRules.CONSTRUCTOR, Reflection.typeOf(kotlinClass), kotlinClass, false); + } + + /** + * Boxing rules for value class wrappers. + */ + enum BoxingRules { + + /** + * When used in the constructor. Constructor boxing depends on nullability of the declared property, whether the + * component uses defaulting, nullability of the value component and whether the component is a primitive. + */ + CONSTRUCTOR { + @Override + public boolean shouldApplyBoxing(KType type, boolean optional, KParameter component) { + + Type javaType = ReflectJvmMapping.getJavaType(component.getType()); + boolean isPrimitive = javaType instanceof Class c && c.isPrimitive(); + + if (type.isMarkedNullable() || optional) { + return (isPrimitive && type.isMarkedNullable()) || component.getType().isMarkedNullable(); + } + + return false; + } + }, + + /** + * When used in a copy method. Copy method boxing depends on nullability of the declared property, nullability of + * the value component and whether the component is a primitive. + */ + COPY { + @Override + public boolean shouldApplyBoxing(KType type, boolean optional, KParameter component) { + + KType copyType = expandUnderlyingType(type); + + if (copyType.getClassifier()instanceof KClass kc && kc.isValue() || copyType.isMarkedNullable()) { + return true; + } + + return false; + } + + private static KType expandUnderlyingType(KType kotlinType) { + + if (!(kotlinType.getClassifier()instanceof KClass kc) || !kc.isValue()) { + return kotlinType; + } + + List> properties = getProperties(kc); + if (properties.isEmpty()) { + return kotlinType; + } + + KType underlyingType = properties.get(0).getReturnType(); + KType componentType = ValueBoxing.resolveType(underlyingType); + KType expandedUnderlyingType = expandUnderlyingType(componentType); + + if (!kotlinType.isMarkedNullable()) { + return expandedUnderlyingType; + } + + if (expandedUnderlyingType.isMarkedNullable()) { + return kotlinType; + } + + Type javaType = ReflectJvmMapping.getJavaType(expandedUnderlyingType); + boolean isPrimitive = javaType instanceof Class c && c.isPrimitive(); + + if (isPrimitive) { + return kotlinType; + } + + return expandedUnderlyingType; + } + + static List> getProperties(KClass kClass) { + + if (kClass.isValue()) { + + for (KCallable member : kClass.getMembers()) { + if (member instanceof KProperty kp) { + return Collections.singletonList(kp); + } + } + } + + List> properties = new ArrayList<>(); + for (KCallable member : kClass.getMembers()) { + if (member instanceof KProperty kp) { + properties.add(kp); + } + } + + return properties; + } + }; + + public abstract boolean shouldApplyBoxing(KType type, boolean optional, KParameter component); + } /** @@ -104,24 +222,30 @@ static class ValueBoxing { private final @Nullable ValueBoxing next; /** - * @param type the referenced type. - * @param optional whether the type is optional. + * Creates a new {@link ValueBoxing} for a {@link KParameter}. + * + * @param rules boxing rules to apply. + * @param parameter the copy or constructor parameter. */ - private ValueBoxing(KType type, boolean optional) { - this((KClass) type.getClassifier(), optional, true); + @SuppressWarnings("ConstantConditions") + private ValueBoxing(BoxingRules rules, KParameter parameter) { + this(rules, parameter.getType(), (KClass) parameter.getType().getClassifier(), parameter.isOptional()); } - private ValueBoxing(KClass kClass, boolean optional, boolean domainTypeUsage) { + private ValueBoxing(BoxingRules rules, KType type, KClass kClass, boolean optional) { KFunction wrapperConstructor = null; ValueBoxing next = null; + boolean applyBoxing; - boolean applyBoxing = (optional || !domainTypeUsage); if (kClass.isValue()) { wrapperConstructor = kClass.getConstructors().iterator().next(); KParameter nested = wrapperConstructor.getParameters().get(0); KType nestedType = nested.getType(); + + applyBoxing = rules.shouldApplyBoxing(type, optional, nested); + KClass nestedClass; // bound flattening @@ -131,7 +255,11 @@ private ValueBoxing(KClass kClass, boolean optional, boolean domainTypeUsage) nestedClass = (KClass) nestedType.getClassifier(); } - next = new ValueBoxing(nestedClass, nested.isOptional(), false); + Assert.notNull(nestedClass, () -> String.format("Cannot resolve nested class from type %s", nestedType)); + + next = new ValueBoxing(rules, nestedType, nestedClass, nested.isOptional()); + } else { + applyBoxing = false; } this.kClass = kClass; @@ -143,6 +271,7 @@ private ValueBoxing(KClass kClass, boolean optional, boolean domainTypeUsage) private static KClass getUpperBound(KTypeParameter typeParameter) { for (KType upperBound : typeParameter.getUpperBounds()) { + if (upperBound.getClassifier()instanceof KClass kc) { return kc; } @@ -151,54 +280,94 @@ private static KClass getUpperBound(KTypeParameter typeParameter) { throw new IllegalArgumentException("No upper bounds found"); } - /** - * Creates a new {@link ValueBoxing} for a {@link KParameter}. - * - * @param parameter - * @return - */ - static ValueBoxing of(KParameter parameter) { - return new ValueBoxing(parameter.getType(), parameter.isOptional()); + static KType resolveType(KType type) { + + if (type.getClassifier()instanceof KTypeParameter ktp) { + + for (KType upperBound : ktp.getUpperBounds()) { + + if (upperBound.getClassifier()instanceof KClass kc) { + return upperBound; + } + } + } + + return type; } /** - * Creates a new {@link ValueBoxing} for a {@link KClass} assuming the class is the uppermost entrypoint and not a - * value class. - * - * @param kotlinClass - * @return + * @return the expanded component type that is used as value. */ - public static ValueBoxing of(KClass kotlinClass) { - return new ValueBoxing(kotlinClass, false, !kotlinClass.isValue()); - } - public Class getActualType() { if (isValueClass() && hasNext()) { - return next.getActualType(); + return getNext().getActualType(); + } + + return JvmClassMappingKt.getJavaClass(kClass); + } + + /** + * @return the component or wrapper type to be used. + */ + public Class getParameterType() { + + if (hasNext() && getNext().appliesBoxing()) { + return next.getParameterType(); } return JvmClassMappingKt.getJavaClass(kClass); } + /** + * @return {@code true} if the value hierarchy applies boxing. + */ + public boolean appliesBoxing() { + return applyBoxing; + } + public boolean isValueClass() { return kClass.isValue(); } + /** + * @return whether there is another item in the value hierarchy. + */ public boolean hasNext() { return next != null; } - @Nullable + /** + * Returns the next {@link ValueBoxing} or throws {@link IllegalStateException} if there is no next. Make sure to + * check {@link #hasNext()} prior to calling this method. + * + * @return the next {@link ValueBoxing}. + * @throws IllegalStateException if there is no next item. + */ public ValueBoxing getNext() { + + if (next == null) { + throw new IllegalStateException("No next ValueBoxing available"); + } + return next; } + /** + * Apply wrapping into the boxing wrapper type if applicable. + * + * @param o + * @return + */ @Nullable public Object wrap(@Nullable Object o) { if (applyBoxing) { - return next == null || kClass.isInstance(o) ? o : wrapperConstructor.call(next.wrap(o)); + return o == null || kClass.isInstance(o) ? o : wrapperConstructor.call(next.wrap(o)); + } + + if (hasNext()) { + return next.wrap(o); } return o; diff --git a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java index 0e5cfda24f..2d1e321506 100644 --- a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java +++ b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java @@ -180,7 +180,6 @@ > PreferredConstructor discover(TypeInf Class rawOwningType = type.getType(); - // Kotlin can rewrite annotated constructors into synthetic ones so we need to inspect that first. Optional> first = Arrays.stream(rawOwningType.getDeclaredConstructors()) // .filter(it -> it.isSynthetic() && AnnotationUtils.findAnnotation(it, PersistenceCreator.class) != null) diff --git a/src/test/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscovererUnitTests.java b/src/test/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscovererUnitTests.java new file mode 100644 index 0000000000..b9bd483fef --- /dev/null +++ b/src/test/java/org/springframework/data/mapping/model/InstanceCreatorMetadataDiscovererUnitTests.java @@ -0,0 +1,43 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.Test; +import org.springframework.data.mapping.InstanceCreatorMetadata; +import org.springframework.data.mapping.model.AbstractPersistentPropertyUnitTests.SamplePersistentProperty; +import org.springframework.data.util.TypeInformation; + +/** + * Unit tests for {@link InstanceCreatorMetadata}. + * + * @author Mark Paluch + */ +class InstanceCreatorMetadataDiscovererUnitTests { + + @Test + void shouldDiscoverConstructorForKotlinValueType() { + + InstanceCreatorMetadata metadata = InstanceCreatorMetadataDiscoverer + .discover(new BasicPersistentEntity( + (TypeInformation) TypeInformation.of(MyNullableValueClass.class))); + + assertThat(metadata).isNotNull(); + assertThat(metadata.hasParameters()).isTrue(); + assertThat(metadata.getParameters().get(0).getName()).isEqualTo("id"); + } +} 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 e5e73598cc..1296cc807e 100644 --- a/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java +++ b/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java @@ -59,6 +59,14 @@ void shouldGeneratePropertyAccessorForTypeWithValueClass(PersistentPropertyAcces assertThat(propertyAccessor).isNotNull(); assertThat(propertyAccessor.getProperty(persistentProperty)).isEqualTo("foo"); + if (factory instanceof BeanWrapperPropertyAccessorFactory) { + + // Sigh. Reflection requires a wrapped value while copy accepts the inlined type. + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> propertyAccessor.setProperty(persistentProperty, "bar")); + return; + } + propertyAccessor.setProperty(persistentProperty, "bar"); assertThat(propertyAccessor.getProperty(persistentProperty)).isEqualTo("bar"); } @@ -178,7 +186,14 @@ void genericInlineClassesShouldWork(PersistentPropertyAccessorFactory factory) { BasicPersistentEntity entity = mappingContext .getRequiredPersistentEntity(WithGenericValue.class); - Object instance = createInstance(entity, parameter -> "aaa"); + KClass genericClass = JvmClassMappingKt.getKotlinClass(MyGenericValue.class); + MyGenericValue inner = genericClass.getConstructors().iterator().next().call("initial-value"); + MyGenericValue outer = genericClass.getConstructors().iterator().next().call(inner); + + MyGenericValue newInner = genericClass.getConstructors().iterator().next().call("new-value"); + MyGenericValue newOuter = genericClass.getConstructors().iterator().next().call(newInner); + + Object instance = createInstance(entity, parameter -> parameter.getName().equals("recursive") ? outer : "aaa"); var propertyAccessor = factory.getPropertyAccessor(entity, instance); var string = entity.getRequiredPersistentProperty("string"); @@ -208,9 +223,11 @@ void genericInlineClassesShouldWork(PersistentPropertyAccessorFactory factory) { var recursive = entity.getRequiredPersistentProperty("recursive"); assertThat(propertyAccessor).isNotNull(); - assertThat(propertyAccessor.getProperty(recursive)).isEqualTo("aaa"); - propertyAccessor.setProperty(recursive, "recursive"); - assertThat(propertyAccessor.getProperty(recursive)).isEqualTo("recursive"); + assertThat(propertyAccessor.getProperty(recursive)).isEqualTo(outer); + propertyAccessor.setProperty(recursive, newOuter); + + // huh? why is that? + assertThat(propertyAccessor.getProperty(recursive)).isEqualTo(newInner); } @MethodSource("factories") 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 5213bac5d5..1e555c9f60 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/InlineClasses.kt @@ -16,6 +16,7 @@ package org.springframework.data.mapping.model import org.springframework.data.annotation.PersistenceCreator +import java.time.LocalDate /** * @author Mark Paluch @@ -58,13 +59,78 @@ value class MyGenericValue(val id: T) value class MyGenericBoundValue(val id: T) data class WithGenericValue( + // ctor: WithGenericValue(CharSequence string, CharSequence charseq, Object recursive, DefaultConstructorMarker $constructor_marker) val string: MyGenericBoundValue, val charseq: MyGenericBoundValue, val recursive: MyGenericValue> + + // copy: copy-XQwFSJ0$default(WithGenericValue var0, CharSequence var1, CharSequence var2, MyGenericValue var3, int var4, Object var5) { ) data class WithGenericNullableValue(val recursive: MyGenericValue>?) +@JvmInline +value class PrimitiveNullableValue(val id: Int?) + +data class WithDefaultPrimitiveValue( + val pvd: PrimitiveValue = PrimitiveValue(1) +) + +/** + * Copy method for nullable value component type uses wrappers while constructor uses the component type. + */ +data class WithPrimitiveNullableValue( + // ctor: public WithPrimitiveNullableValue(Integer nv, PrimitiveNullableValue nvn, Integer nvd, PrimitiveNullableValue nvdn, DefaultConstructorMarker $constructor_marker) { + val nv: PrimitiveNullableValue, + val nvn: PrimitiveNullableValue?, + val nvd: PrimitiveNullableValue = PrimitiveNullableValue(1), + val nvdn: PrimitiveNullableValue? = PrimitiveNullableValue(1), + + // copy: copy-lcs_1S0$default(WithPrimitiveNullableValue var0, PrimitiveNullableValue var1, PrimitiveNullableValue var2, PrimitiveNullableValue var3, PrimitiveNullableValue var4, int var5, Object var6) +) + +@JvmInline +value class PrimitiveValue(val id: Int) + +data class WithPrimitiveValue( + + // ctor: int,org.springframework.data.mapping.model.KotlinValueUtilsUnitTests$PrimitiveValue,int,org.springframework.data.mapping.model.KotlinValueUtilsUnitTests$PrimitiveValue,kotlin.jvm.internal.DefaultConstructorMarker + val nv: PrimitiveValue, + val nvn: PrimitiveValue?, + val nvd: PrimitiveValue = PrimitiveValue(1), + val nvdn: PrimitiveValue? = PrimitiveValue(1), + + // copy: copy-XQwFSJ0$default(WithPrimitiveValue var0, int var1, PrimitiveValue var2, int var3, PrimitiveValue var4, int var5, Object var6) +) + +@JvmInline +value class StringValue(val id: String) + +data class WithStringValue( + + // ctor: WithStringValue(String nv, String nvn, String nvd, String nvdn) + val nv: StringValue, + val nvn: StringValue?, + val nvd: StringValue = StringValue("1"), + val nvdn: StringValue? = StringValue("1"), + + // copy: copy-QB2wzyg$default(WithStringValue var0, String var1, String var2, String var3, String var4, int var5, Object var6) +) + + +data class Inner(val id: LocalDate, val foo: String) + +@JvmInline +value class Outer(val id: Inner = Inner(LocalDate.MAX, "")) + +data class WithCustomInner( + + // ctor: private WithCustomInner(Inner nv) + val nv: Outer + + // copy: copy(org.springframework.data.mapping.model.WithCustomInner,org.springframework.data.mapping.model.Inner,int,java.lang.Object) +) + class WithNestedMyNullableValueClass( var id: MyNestedNullableValueClass? = MyNestedNullableValueClass( MyNullableValueClass("foo") 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 d1521694cc..f606b260fd 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -189,6 +189,48 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { assertThat(instance.baz?.id).isEqualTo("Walter-pref") } + @Test // GH-2806 + fun `should instantiate type with value class defaulting`() { + + every { provider.getParameterValue(any()) }.returns(1) + + val instance = construct(WithDefaultPrimitiveValue::class) + + assertThat(instance.pvd.id).isEqualTo(1) + } + + @Test // GH-2806 + fun `should instantiate type with nullable primitive value class defaulting`() { + + every { provider.getParameterValue(any()) }.returnsMany( + 1, + PrimitiveNullableValue(2), + 3, + PrimitiveNullableValue(4) + ) + + val instance = construct(WithPrimitiveNullableValue::class) + + assertThat(instance.nv.id).isEqualTo(1) + assertThat(instance.nvn!!.id).isEqualTo(2) + assertThat(instance.nvd.id).isEqualTo(3) + assertThat(instance.nvdn!!.id).isEqualTo(4) + } + + @Test // GH-2806 + fun `should instantiate type with nullable value class defaulting`() { + + // String nv, String nvn, String nvd, String nvdn, LocalDate dnv, LocalDate dnvn, LocalDate dnvd, LocalDate dnvdn + every { provider.getParameterValue(any()) }.returnsMany("1", "2", "3", "4") + + val instance = construct(WithStringValue::class) + + assertThat(instance.nv.id).isEqualTo("1") + assertThat(instance.nvn!!.id).isEqualTo("2") + assertThat(instance.nvd.id).isEqualTo("3") + assertThat(instance.nvdn!!.id).isEqualTo("4") + } + private fun construct(typeToCreate: KClass): T { val entity = diff --git a/src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt new file mode 100644 index 0000000000..e415c3e799 --- /dev/null +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt @@ -0,0 +1,192 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mapping.model; + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import kotlin.reflect.KParameter +import kotlin.reflect.jvm.javaConstructor + +/** + * Unit tests for [KotlinValueUtils]. + * @author Mark Paluch + */ +class KotlinValueUtilsUnitTests { + + @Test // GH-2806 + internal fun considersCustomInnerConstructorRules() { + + val ctor = WithCustomInner::class.constructors.iterator().next(); + assertThat(ctor.javaConstructor.toString()).contains("(org.springframework.data.mapping.model.Inner,kotlin.jvm.internal.DefaultConstructorMarker)") + + val vh = KotlinValueUtils.getConstructorValueHierarchy( + ctor.parameters.iterator().next() + ) + + assertThat(vh.actualType).isEqualTo(Inner::class.java) + assertThat(vh.appliesBoxing()).isFalse + } + + @Test // GH-2806 + internal fun considersCustomInnerCopyRules() { + + val copy = KotlinCopyMethod.findCopyMethod(WithCustomInner::class.java).get(); + assertThat(copy.syntheticCopyMethod.toString()).contains("(org.springframework.data.mapping.model.WithCustomInner,org.springframework.data.mapping.model.Inner,int,java.lang.Object)") + + val vh = KotlinValueUtils.getCopyValueHierarchy( + copy.copyFunction.parameters.get(1) + ) + + assertThat(vh.parameterType).isEqualTo(Outer::class.java) + assertThat(vh.actualType).isEqualTo(Inner::class.java) + assertThat(vh.appliesBoxing()).isFalse + } + + @Test // GH-2806 + internal fun inlinesTypesToStringConstructorRules() { + + val ctor = WithStringValue::class.constructors.iterator().next(); + assertThat(ctor.javaConstructor.toString()).contains("(java.lang.String,java.lang.String,java.lang.String,java.lang.String,kotlin.jvm.internal.DefaultConstructorMarker)") + + for (kParam in ctor.parameters) { + + val vh = KotlinValueUtils.getConstructorValueHierarchy(kParam) + + assertThat(vh.actualType).isEqualTo(String::class.java) + assertThat(vh.appliesBoxing()).describedAs(kParam.toString()).isFalse + assertThat(vh.wrap("1")).describedAs(kParam.toString()).isEqualTo("1") + } + } + + @Test // GH-2806 + internal fun inlinesTypesToStringCopyRules() { + + val copy = KotlinCopyMethod.findCopyMethod(WithStringValue::class.java).get(); + assertThat(copy.syntheticCopyMethod.toString()).contains("(org.springframework.data.mapping.model.WithStringValue,java.lang.String,java.lang.String,java.lang.String,java.lang.String,int,java.lang.Object)") + + for (kParam in copy.copyFunction.parameters) { + + if (kParam.kind == KParameter.Kind.INSTANCE) { + continue + } + + val vh = KotlinValueUtils.getCopyValueHierarchy(kParam) + + assertThat(vh.actualType).isEqualTo(String::class.java) + assertThat(vh.appliesBoxing()).describedAs(kParam.toString()).isFalse + assertThat(vh.wrap("1")).describedAs(kParam.toString()).isEqualTo("1") + } + } + + @Test // GH-2806 + internal fun inlinesTypesToIntConstructorRules() { + + val ctor = WithPrimitiveValue::class.constructors.iterator().next(); + assertThat(ctor.javaConstructor.toString()).contains("(int,org.springframework.data.mapping.model.PrimitiveValue,int,org.springframework.data.mapping.model.PrimitiveValue,kotlin.jvm.internal.DefaultConstructorMarker)") + + val iterator = ctor.parameters.iterator() + + val nv = KotlinValueUtils.getConstructorValueHierarchy(iterator.next()); + assertThat(nv.actualType).isEqualTo(Int::class.java) + assertThat(nv.appliesBoxing()).isFalse + + val nvn = KotlinValueUtils.getConstructorValueHierarchy(iterator.next()); + assertThat(nvn.actualType).isEqualTo(Int::class.java) + assertThat(nvn.appliesBoxing()).isTrue + assertThat(nvn.parameterType).isEqualTo(PrimitiveValue::class.java) + + val nvd = KotlinValueUtils.getConstructorValueHierarchy(iterator.next()); + assertThat(nvd.actualType).isEqualTo(Int::class.java) + assertThat(nvd.appliesBoxing()).isFalse + + val nvdn = KotlinValueUtils.getConstructorValueHierarchy(iterator.next()); + assertThat(nvdn.actualType).isEqualTo(Int::class.java) + assertThat(nvn.parameterType).isEqualTo(PrimitiveValue::class.java) + assertThat(nvdn.appliesBoxing()).isTrue + } + + @Test // GH-2806 + internal fun inlinesTypesToIntCopyRules() { + + val copy = KotlinCopyMethod.findCopyMethod(WithPrimitiveValue::class.java).get(); + assertThat(copy.syntheticCopyMethod.toString()).contains("(org.springframework.data.mapping.model.WithPrimitiveValue,int,org.springframework.data.mapping.model.PrimitiveValue,int,org.springframework.data.mapping.model.PrimitiveValue,int,java.lang.Object)") + + val parameters = copy.copyFunction.parameters; + + val nv = KotlinValueUtils.getConstructorValueHierarchy(parameters[1]); + assertThat(nv.actualType).isEqualTo(Int::class.java) + assertThat(nv.appliesBoxing()).isFalse + + val nvn = KotlinValueUtils.getConstructorValueHierarchy(parameters[2]); + assertThat(nvn.actualType).isEqualTo(Int::class.java) + assertThat(nvn.appliesBoxing()).isTrue + assertThat(nvn.parameterType).isEqualTo(PrimitiveValue::class.java) + + val nvd = KotlinValueUtils.getConstructorValueHierarchy(parameters[3]); + assertThat(nvd.actualType).isEqualTo(Int::class.java) + assertThat(nvd.appliesBoxing()).isFalse + + val nvdn = KotlinValueUtils.getConstructorValueHierarchy(parameters[4]); + assertThat(nvdn.actualType).isEqualTo(Int::class.java) + assertThat(nvn.parameterType).isEqualTo(PrimitiveValue::class.java) + assertThat(nvdn.appliesBoxing()).isTrue + } + + @Test // GH-2806 + internal fun inlinesGenericTypesConstructorRules() { + + val ctor = WithGenericValue::class.constructors.iterator().next(); + assertThat(ctor.javaConstructor.toString()).contains("(java.lang.CharSequence,java.lang.CharSequence,java.lang.Object,kotlin.jvm.internal.DefaultConstructorMarker)") + + val iterator = ctor.parameters.iterator() + + val string = KotlinValueUtils.getConstructorValueHierarchy(iterator.next()); + assertThat(string.actualType).isEqualTo(CharSequence::class.java) + assertThat(string.appliesBoxing()).isFalse + + val charseq = KotlinValueUtils.getConstructorValueHierarchy(iterator.next()); + assertThat(charseq.actualType).isEqualTo(CharSequence::class.java) + assertThat(charseq.appliesBoxing()).isFalse + + val recursive = KotlinValueUtils.getConstructorValueHierarchy(iterator.next()); + assertThat(recursive.actualType).isEqualTo(Any::class.java) + assertThat(recursive.appliesBoxing()).isFalse + } + + @Test // GH-2806 + internal fun inlinesGenericTypesCopyRules() { + + val copy = KotlinCopyMethod.findCopyMethod(WithGenericValue::class.java).get(); + assertThat(copy.syntheticCopyMethod.toString()).contains("(org.springframework.data.mapping.model.WithGenericValue,java.lang.CharSequence,java.lang.CharSequence,org.springframework.data.mapping.model.MyGenericValue,int,java.lang.Object)") + + val parameters = copy.copyFunction.parameters; + + val string = KotlinValueUtils.getConstructorValueHierarchy(parameters[1]); + assertThat(string.actualType).isEqualTo(CharSequence::class.java) + assertThat(string.appliesBoxing()).isFalse + + val charseq = KotlinValueUtils.getConstructorValueHierarchy(parameters[2]); + assertThat(charseq.actualType).isEqualTo(CharSequence::class.java) + assertThat(charseq.appliesBoxing()).isFalse + + val recursive = KotlinValueUtils.getConstructorValueHierarchy(parameters[3]); + assertThat(recursive.actualType).isEqualTo(Object::class.java) + assertThat(recursive.parameterType).isEqualTo(MyGenericValue::class.java) + assertThat(recursive.appliesBoxing()).isFalse + } + +} + diff --git a/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt index e5b9e43ffa..4d091d91c7 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt @@ -38,7 +38,8 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should reject two constructors`() { - val constructor = PreferredConstructorDiscoverer.discover(TwoConstructors::class.java) + val constructor = + PreferredConstructorDiscoverer.discover(TwoConstructors::class.java) assertThat(constructor!!.parameters.size).isEqualTo(1) } @@ -46,7 +47,10 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1170 fun `should fall back to no-args constructor if no primary constructor available`() { - val constructor = PreferredConstructorDiscoverer.discover(TwoConstructorsWithoutDefault::class.java) + val constructor = + PreferredConstructorDiscoverer.discover( + TwoConstructorsWithoutDefault::class.java + ) assertThat(constructor!!.parameters).isEmpty() } @@ -54,7 +58,9 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should discover annotated constructor`() { - val constructor = PreferredConstructorDiscoverer.discover(AnnotatedConstructors::class.java) + val constructor = PreferredConstructorDiscoverer.discover( + AnnotatedConstructors::class.java + ) assertThat(constructor!!.parameters.size).isEqualTo(2) } @@ -62,7 +68,8 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should discover default constructor`() { - val constructor = PreferredConstructorDiscoverer.discover(DefaultConstructor::class.java) + val constructor = + PreferredConstructorDiscoverer.discover(DefaultConstructor::class.java) assertThat(constructor!!.parameters.size).isEqualTo(1) } @@ -70,7 +77,10 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should discover default annotated constructor`() { - val constructor = PreferredConstructorDiscoverer.discover(TwoDefaultConstructorsAnnotated::class.java) + val constructor = + PreferredConstructorDiscoverer.discover( + TwoDefaultConstructorsAnnotated::class.java + ) assertThat(constructor!!.parameters.size).isEqualTo(3) } @@ -88,8 +98,7 @@ class PreferredConstructorDiscovererUnitTests { assertThat(constructor).isNull() } - // See https://github.com/spring-projects/spring-data-commons/issues/2374 - /*@Test // DATACMNS-1800, gh-2215 + @Test // DATACMNS-1800, GH-2215 @ExperimentalUnsignedTypes fun `should discover constructor for class using unsigned types`() { @@ -99,7 +108,7 @@ class PreferredConstructorDiscovererUnitTests { ) assertThat(constructor).isNotNull() - } */ + } data class Simple(val firstname: String) @@ -138,13 +147,11 @@ class PreferredConstructorDiscovererUnitTests { ) } - /* - See https://github.com/spring-projects/spring-data-commons/issues/2374 @ExperimentalUnsignedTypes data class UnsignedTypesEntity( val id: String, val a: UInt = 5u, val b: Int = 5, val c: Double = 1.5 - ) */ + ) } diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt index 9bb7ead0ac..a34460c829 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt @@ -56,6 +56,16 @@ class ReflectionEntityInstantiatorDataClassUnitTests { assertThat(instance.lastname).isEqualTo("White") } + @Test // GH-2806 + fun `should instantiate type with value class defaulting`() { + + every { provider.getParameterValue(any()) }.returns(1) + + val instance = construct(WithDefaultPrimitiveValue::class) + + assertThat(instance.pvd.id).isEqualTo(1) + } + private fun construct(typeToCreate: KClass): T { val entity = mockk>() diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt index 96a74d3076..c7db9e3c38 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt @@ -44,18 +44,6 @@ class ReflectionEntityInstantiatorValueClassUnitTests { assertThat(instance.id.id).isEqualTo("Walter") } - @Test // GH-2806 - fun `should create instance with defaulting with value`() { - - every { provider.getParameterValue(any()) }.returnsMany("Walter", "") - - val instance: WithNestedMyNullableValueClass = - construct(WithNestedMyNullableValueClass::class) - - assertThat(instance.id?.id?.id).isEqualTo("Walter") - assertThat(instance.baz?.id).isEqualTo("") - } - @Test // GH-2806 fun `should create instance with defaulting without value`() { From 186dbac5d06e3e43001d9b78a761ed52099a43ae Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 28 Jun 2023 14:48:43 +0200 Subject: [PATCH 10/11] Add documentation. --- src/main/asciidoc/object-mapping.adoc | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/main/asciidoc/object-mapping.adoc b/src/main/asciidoc/object-mapping.adoc index c2213c3ac0..804212fac6 100644 --- a/src/main/asciidoc/object-mapping.adoc +++ b/src/main/asciidoc/object-mapping.adoc @@ -436,3 +436,27 @@ You can exclude properties by annotating these with `@Transient`. 2. How to represent properties in your data store? Using the same field/column name for different values typically leads to corrupt data so you should annotate least one of the properties using an explicit field/column name. 3. Using `@AccessType(PROPERTY)` cannot be used as the super-property cannot be set. + +[[mapping.kotlin.value.classes]] +=== Kotlin Value Classes + +Kotlin Value Classes are designed for a more expressive domain model to make underlying concepts explicit. +Spring Data can read and write types that define properties using Value Classes. + +Consider the following domain model: + +==== +[source,kotlin] +---- +@JvmInline +value class EmailAddress(val theAddress: String) <1> + +data class Contact(val id: String, val name:String, val emailAddress: EmailAddress) <2> +---- + +<1> A simple value class with a non-nullable value type. +<2> Data class defining a property using the `EmailAddress` value class. +==== + +NOTE: Non-nullable properties using non-primitive value types are flattened in the compiled class to the value type. +Nullable primitive value types or nullable value-in-value types are represented with their wrapper type and that affects how value types are represented in the database. From 9cc9476be583aef9517659dedfa3de56b6e71ce1 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 28 Jun 2023 14:58:59 +0200 Subject: [PATCH 11/11] Polishing. Switch to older ticket number. --- .../KotlinPropertyAccessorFactoryTests.java | 12 ++++++------ ...ClassGeneratingEntityInstantiatorUnitTests.kt | 8 ++++---- .../mapping/model/KotlinValueUtilsUnitTests.kt | 16 ++++++++-------- ...ectionEntityInstantiatorDataClassUnitTests.kt | 2 +- ...tionEntityInstantiatorInlineClassUnitTests.kt | 6 +++--- 5 files changed, 22 insertions(+), 22 deletions(-) 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 1296cc807e..0750748b1c 100644 --- a/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java +++ b/src/test/java/org/springframework/data/mapping/model/KotlinPropertyAccessorFactoryTests.java @@ -45,7 +45,7 @@ public class KotlinPropertyAccessorFactoryTests { private SampleMappingContext mappingContext = new SampleMappingContext(); @MethodSource("factories") - @ParameterizedTest // GH-2806 + @ParameterizedTest // GH-1947 void shouldGeneratePropertyAccessorForTypeWithValueClass(PersistentPropertyAccessorFactory factory) { BasicPersistentEntity entity = mappingContext @@ -72,7 +72,7 @@ void shouldGeneratePropertyAccessorForTypeWithValueClass(PersistentPropertyAcces } @MethodSource("factories") - @ParameterizedTest // GH-2806 + @ParameterizedTest // GH-1947 void shouldGeneratePropertyAccessorForTypeWithNullableValueClass(PersistentPropertyAccessorFactory factory) throws ReflectiveOperationException { @@ -96,7 +96,7 @@ void shouldGeneratePropertyAccessorForTypeWithNullableValueClass(PersistentPrope } @MethodSource("factories") - @ParameterizedTest // GH-2806 + @ParameterizedTest // GH-1947 void shouldGeneratePropertyAccessorForDataClassWithNullableValueClass(PersistentPropertyAccessorFactory factory) throws ReflectiveOperationException { @@ -137,7 +137,7 @@ void shouldGeneratePropertyAccessorForDataClassWithNullableValueClass(Persistent } @MethodSource("factories") - @ParameterizedTest // GH-2806 + @ParameterizedTest // GH-1947 void nestedNullablePropertiesShouldBeSetCorrectly(PersistentPropertyAccessorFactory factory) { BasicPersistentEntity entity = mappingContext @@ -180,7 +180,7 @@ void nestedNullablePropertiesShouldBeSetCorrectly(PersistentPropertyAccessorFact } @MethodSource("factories") - @ParameterizedTest // GH-2806 + @ParameterizedTest // GH-1947 void genericInlineClassesShouldWork(PersistentPropertyAccessorFactory factory) { BasicPersistentEntity entity = mappingContext @@ -231,7 +231,7 @@ void genericInlineClassesShouldWork(PersistentPropertyAccessorFactory factory) { } @MethodSource("factories") - @ParameterizedTest // GH-2806 + @ParameterizedTest // GH-1947 void genericNullableInlineClassesShouldWork(PersistentPropertyAccessorFactory factory) { BasicPersistentEntity entity = mappingContext 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 f606b260fd..5eb67944db 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -178,7 +178,7 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { assertThat(instance.baz?.id).isEqualTo("id") } - @Test // GH-2806 + @Test // GH-1947 fun `should use annotated constructor for types using nullable value class`() { every { provider.getParameterValue(any()) }.returnsMany("Walter", null) @@ -189,7 +189,7 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { assertThat(instance.baz?.id).isEqualTo("Walter-pref") } - @Test // GH-2806 + @Test // GH-1947 fun `should instantiate type with value class defaulting`() { every { provider.getParameterValue(any()) }.returns(1) @@ -199,7 +199,7 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { assertThat(instance.pvd.id).isEqualTo(1) } - @Test // GH-2806 + @Test // GH-1947 fun `should instantiate type with nullable primitive value class defaulting`() { every { provider.getParameterValue(any()) }.returnsMany( @@ -217,7 +217,7 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { assertThat(instance.nvdn!!.id).isEqualTo(4) } - @Test // GH-2806 + @Test // GH-1947 fun `should instantiate type with nullable value class defaulting`() { // String nv, String nvn, String nvd, String nvdn, LocalDate dnv, LocalDate dnvn, LocalDate dnvd, LocalDate dnvdn diff --git a/src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt index e415c3e799..79efef7acd 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/KotlinValueUtilsUnitTests.kt @@ -26,7 +26,7 @@ import kotlin.reflect.jvm.javaConstructor */ class KotlinValueUtilsUnitTests { - @Test // GH-2806 + @Test // GH-1947 internal fun considersCustomInnerConstructorRules() { val ctor = WithCustomInner::class.constructors.iterator().next(); @@ -40,7 +40,7 @@ class KotlinValueUtilsUnitTests { assertThat(vh.appliesBoxing()).isFalse } - @Test // GH-2806 + @Test // GH-1947 internal fun considersCustomInnerCopyRules() { val copy = KotlinCopyMethod.findCopyMethod(WithCustomInner::class.java).get(); @@ -55,7 +55,7 @@ class KotlinValueUtilsUnitTests { assertThat(vh.appliesBoxing()).isFalse } - @Test // GH-2806 + @Test // GH-1947 internal fun inlinesTypesToStringConstructorRules() { val ctor = WithStringValue::class.constructors.iterator().next(); @@ -71,7 +71,7 @@ class KotlinValueUtilsUnitTests { } } - @Test // GH-2806 + @Test // GH-1947 internal fun inlinesTypesToStringCopyRules() { val copy = KotlinCopyMethod.findCopyMethod(WithStringValue::class.java).get(); @@ -91,7 +91,7 @@ class KotlinValueUtilsUnitTests { } } - @Test // GH-2806 + @Test // GH-1947 internal fun inlinesTypesToIntConstructorRules() { val ctor = WithPrimitiveValue::class.constructors.iterator().next(); @@ -118,7 +118,7 @@ class KotlinValueUtilsUnitTests { assertThat(nvdn.appliesBoxing()).isTrue } - @Test // GH-2806 + @Test // GH-1947 internal fun inlinesTypesToIntCopyRules() { val copy = KotlinCopyMethod.findCopyMethod(WithPrimitiveValue::class.java).get(); @@ -145,7 +145,7 @@ class KotlinValueUtilsUnitTests { assertThat(nvdn.appliesBoxing()).isTrue } - @Test // GH-2806 + @Test // GH-1947 internal fun inlinesGenericTypesConstructorRules() { val ctor = WithGenericValue::class.constructors.iterator().next(); @@ -166,7 +166,7 @@ class KotlinValueUtilsUnitTests { assertThat(recursive.appliesBoxing()).isFalse } - @Test // GH-2806 + @Test // GH-1947 internal fun inlinesGenericTypesCopyRules() { val copy = KotlinCopyMethod.findCopyMethod(WithGenericValue::class.java).get(); diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt index a34460c829..44c9a69260 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorDataClassUnitTests.kt @@ -56,7 +56,7 @@ class ReflectionEntityInstantiatorDataClassUnitTests { assertThat(instance.lastname).isEqualTo("White") } - @Test // GH-2806 + @Test // GH-1947 fun `should instantiate type with value class defaulting`() { every { provider.getParameterValue(any()) }.returns(1) diff --git a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt index c7db9e3c38..87e4424d73 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/ReflectionEntityInstantiatorInlineClassUnitTests.kt @@ -33,7 +33,7 @@ class ReflectionEntityInstantiatorValueClassUnitTests { val provider = mockk>() - @Test // GH-2806 + @Test // GH-1947 fun `should create instance`() { every { provider.getParameterValue(any()) }.returnsMany("Walter") @@ -44,7 +44,7 @@ class ReflectionEntityInstantiatorValueClassUnitTests { assertThat(instance.id.id).isEqualTo("Walter") } - @Test // GH-2806 + @Test // GH-1947 fun `should create instance with defaulting without value`() { every { provider.getParameterValue(any()) } returns null @@ -55,7 +55,7 @@ class ReflectionEntityInstantiatorValueClassUnitTests { assertThat(instance.baz?.id).isEqualTo("id") } - @Test // GH-2806 + @Test // GH-1947 fun `should use annotated constructor for types using nullable value class`() { every { provider.getParameterValue(any()) }.returnsMany("Walter", null)