diff --git a/pom.xml b/pom.xml index 2473b93f59..c06ded7c9f 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 2.1.0.BUILD-SNAPSHOT + 2.1.0.DATACMNS-1200-SNAPSHOT Spring Data Core diff --git a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java index 144b3e005b..a2092ba2e3 100644 --- a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java @@ -23,6 +23,7 @@ import java.security.PrivilegedAction; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.springframework.asm.ClassWriter; @@ -399,6 +400,7 @@ private void visitCreateMethod(ClassWriter cw, PersistentEntity entity, Constructor ctor = constructor.getConstructor(); Class[] parameterTypes = ctor.getParameterTypes(); + List> parameters = constructor.getParameters(); for (int i = 0; i < parameterTypes.length; i++) { @@ -409,6 +411,11 @@ private void visitCreateMethod(ClassWriter cw, PersistentEntity entity, mv.visitInsn(AALOAD); if (parameterTypes[i].isPrimitive()) { + + mv.visitInsn(DUP); + String parameterName = parameters.size() > i ? parameters.get(i).getName() : null; + + insertAssertNotNull(mv, parameterName == null ? String.format("at index %d", i) : parameterName); insertUnboxInsns(mv, Type.getType(parameterTypes[i]).toString().charAt(0), ""); } else { mv.visitTypeInsn(CHECKCAST, Type.getInternalName(parameterTypes[i])); @@ -438,6 +445,20 @@ private static void visitArrayIndex(MethodVisitor mv, int idx) { mv.visitLdcInsn(idx); } + /** + * Insert not {@literal null} assertion for a parameter. + * + * @param mv the method visitor into which instructions should be inserted + * @param parameterName name of the parameter to create the appropriate assertion message. + */ + private void insertAssertNotNull(MethodVisitor mv, String parameterName) { + + // Assert.notNull(property) + mv.visitLdcInsn(String.format("Parameter %s must not be null!", parameterName)); + mv.visitMethodInsn(INVOKESTATIC, "org/springframework/util/Assert", "notNull", + String.format("(%s%s)V", String.format("L%s;", JAVA_LANG_OBJECT), "Ljava/lang/String;"), false); + } + /** * Insert any necessary cast and value call to convert from a boxed type to a primitive value. *

diff --git a/src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java index 1bdb23a3ab..9b35c17e13 100644 --- a/src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java @@ -20,6 +20,7 @@ import kotlin.reflect.jvm.ReflectJvmMapping; import java.lang.reflect.Constructor; +import java.util.Arrays; import java.util.List; import java.util.stream.IntStream; @@ -27,6 +28,7 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.mapping.PreferredConstructor.Parameter; +import org.springframework.data.mapping.model.MappingInstantiationException; import org.springframework.data.mapping.model.ParameterValueProvider; import org.springframework.data.util.ReflectionUtils; import org.springframework.lang.Nullable; @@ -197,7 +199,17 @@ static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantia public , P extends PersistentProperty

> T createInstance(E entity, ParameterValueProvider

provider) { - PreferredConstructor preferredConstructor = entity.getPersistenceConstructor(); + Object[] params = extractInvocationArguments(entity.getPersistenceConstructor(), provider); + + try { + return (T) instantiator.newInstance(params); + } catch (Exception e) { + throw new MappingInstantiationException(entity, Arrays.asList(params), e); + } + } + + private

, T> Object[] extractInvocationArguments( + @Nullable PreferredConstructor preferredConstructor, ParameterValueProvider

provider) { if (preferredConstructor == null) { throw new IllegalArgumentException("PreferredConstructor must not be null!"); @@ -217,15 +229,19 @@ public , P extends PersistentPrope int slot = i / 32; int offset = slot * 32; - Object param = provider.getParameterValue(parameters.get(i)); + Parameter parameter = parameters.get(i); + Class type = parameter.getType().getType(); + Object param = provider.getParameterValue(parameter); KParameter kParameter = kParameters.get(i); // what about null and parameter is mandatory? What if parameter is non-null? - if (kParameter.isOptional()) { + if (kParameter.isOptional() && param == null) { + + defaulting[slot] = defaulting[slot] | (1 << (i - offset)); - if (param == null) { - defaulting[slot] = defaulting[slot] | (1 << (i - offset)); + if (type.isPrimitive()) { + param = getPrimitiveDefault(type); } } @@ -237,7 +253,44 @@ public , P extends PersistentPrope params[userParameterCount + i] = defaulting[i]; } - return (T) instantiator.newInstance(params); + return params; + } + + private static Object getPrimitiveDefault(Class type) { + + if (type == Byte.TYPE || type == Byte.class) { + return (byte) 0; + } + + if (type == Short.TYPE || type == Short.class) { + return (short) 0; + } + + if (type == Integer.TYPE || type == Integer.class) { + return 0; + } + + if (type == Long.TYPE || type == Long.class) { + return 0L; + } + + if (type == Float.TYPE || type == Float.class) { + return 0F; + } + + if (type == Double.TYPE || type == Double.class) { + return 0D; + } + + if (type == Character.TYPE || type == Character.class) { + return '\u0000'; + } + + if (type == Boolean.TYPE) { + return Boolean.FALSE; + } + + throw new IllegalArgumentException(String.format("Primitive type %s not supported!", type)); } } } diff --git a/src/main/java/org/springframework/data/mapping/model/MappingInstantiationException.java b/src/main/java/org/springframework/data/mapping/model/MappingInstantiationException.java index 68e7a82998..37bbe99611 100644 --- a/src/main/java/org/springframework/data/mapping/model/MappingInstantiationException.java +++ b/src/main/java/org/springframework/data/mapping/model/MappingInstantiationException.java @@ -15,6 +15,9 @@ */ package org.springframework.data.mapping.model; +import kotlin.reflect.KFunction; +import kotlin.reflect.jvm.ReflectJvmMapping; + import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.List; @@ -22,6 +25,7 @@ import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PreferredConstructor; +import org.springframework.data.util.ReflectionUtils; import org.springframework.lang.Nullable; import org.springframework.util.ObjectUtils; @@ -88,12 +92,28 @@ private static String buildExceptionMessage(Optional> ent } return String.format(TEXT_TEMPLATE, it.getType().getName(), - constructor.map(c -> c.getConstructor().toString()).orElse("NO_CONSTRUCTOR"), // + constructor.map(c -> toString(c)).orElse("NO_CONSTRUCTOR"), // String.join(",", toStringArgs)); }).orElse(defaultMessage); } + private static String toString(PreferredConstructor preferredConstructor) { + + Constructor constructor = preferredConstructor.getConstructor(); + + if (ReflectionUtils.isSupportedKotlinClass(constructor.getDeclaringClass())) { + + KFunction kotlinFunction = ReflectJvmMapping.getKotlinFunction(constructor); + + if (kotlinFunction != null) { + return kotlinFunction.toString(); + } + } + + return constructor.toString(); + } + /** * Returns the type of the entity that was attempted to instantiate. * diff --git a/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java b/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java index 242b1a7010..bb3f263cde 100755 --- a/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java +++ b/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java @@ -264,6 +264,17 @@ public void instantiateObjectCtor1ParamInt() { }); } + @Test // DATACMNS-1200 + public void instantiateObjectCtor1ParamIntWithoutValue() { + + doReturn(ObjectCtor1ParamInt.class).when(entity).getType(); + doReturn(PreferredConstructorDiscoverer.discover(ObjectCtor1ParamInt.class))// + .when(entity).getPersistenceConstructor(); + + assertThatThrownBy(() -> this.instance.createInstance(entity, provider)) // + .hasCauseInstanceOf(IllegalArgumentException.class); + } + @Test // DATACMNS-578, DATACMNS-1126 @SuppressWarnings("unchecked") public void instantiateObjectCtor7ParamsString5IntsString() { diff --git a/src/test/kotlin/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiatorUnitTests.kt b/src/test/kotlin/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiatorUnitTests.kt index 0c3f820c08..14e0da6b44 100644 --- a/src/test/kotlin/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiatorUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiatorUnitTests.kt @@ -25,8 +25,10 @@ import org.mockito.Mock import org.mockito.junit.MockitoJUnitRunner import org.springframework.data.mapping.PersistentEntity import org.springframework.data.mapping.context.SamplePersistentProperty +import org.springframework.data.mapping.model.MappingInstantiationException import org.springframework.data.mapping.model.ParameterValueProvider import org.springframework.data.mapping.model.PreferredConstructorDiscoverer +import java.lang.IllegalArgumentException /** * Unit tests for [KotlinClassGeneratingEntityInstantiator] creating instances using Kotlin data classes. @@ -79,6 +81,42 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { Assertions.assertThat(instance.prop34).isEqualTo("White") } + @Test // DATACMNS-1200 + fun `absent primitive value should cause MappingInstantiationException`() { + + val entity = this.entity as PersistentEntity + val constructor = PreferredConstructorDiscoverer.discover(WithBoolean::class.java) + + doReturn(constructor).whenever(entity).persistenceConstructor + doReturn(constructor.constructor.declaringClass).whenever(entity).type + + Assertions.assertThatThrownBy { KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) } // + .isInstanceOf(MappingInstantiationException::class.java) // + .hasMessageContaining("fun (kotlin.Boolean)") // + .hasCauseInstanceOf(IllegalArgumentException::class.java) + } + + @Test // DATACMNS-1200 + fun `should apply primitive defaulting for absent parameters`() { + + val entity = this.entity as PersistentEntity + val constructor = PreferredConstructorDiscoverer.discover(WithPrimitiveDefaulting::class.java) + + doReturn(constructor).whenever(entity).persistenceConstructor + doReturn(constructor.constructor.declaringClass).whenever(entity).type + + val instance: WithPrimitiveDefaulting = KotlinClassGeneratingEntityInstantiator().createInstance(entity, provider) + + Assertions.assertThat(instance.aByte).isEqualTo(0) + Assertions.assertThat(instance.aShort).isEqualTo(0) + Assertions.assertThat(instance.anInt).isEqualTo(0) + Assertions.assertThat(instance.aLong).isEqualTo(0L) + Assertions.assertThat(instance.aFloat).isEqualTo(0.0f) + Assertions.assertThat(instance.aDouble).isEqualTo(0.0) + Assertions.assertThat(instance.aChar).isEqualTo('a') + Assertions.assertThat(instance.aBool).isTrue() + } + data class Contact(val firstname: String, val lastname: String) data class ContactWithDefaulting(val prop0: String, val prop1: String = "White", val prop2: String, @@ -94,5 +132,10 @@ class KotlinClassGeneratingEntityInstantiatorUnitTests { 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) }