DATACMNS-1200 - Fix entity instantiation of Kotlin types using primitives with default values.

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-commons</artifactId>
-	<version>2.1.0.BUILD-SNAPSHOT</version>
+	<version>2.1.0.DATACMNS-1200-SNAPSHOT</version>
 
 	<name>Spring Data Core</name>
 

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<? extends Parameter<Object, ?>> 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.
 		 * <p>

src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java

@@ -20,13 +20,15 @@
 import kotlin.reflect.jvm.ReflectJvmMapping;
 
 import java.lang.reflect.Constructor;
+import java.util.Arrays;
 import java.util.List;
 import java.util.stream.IntStream;
 
 import org.springframework.data.mapping.PersistentEntity;
 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 <T, E extends PersistentEntity<? extends T, P>, P extends PersistentProperty<P>> T createInstance(E entity,
 				ParameterValueProvider<P> provider) {
 
-			PreferredConstructor<? extends T, P> 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 <P extends PersistentProperty<P>, T> Object[] extractInvocationArguments(
+				@Nullable PreferredConstructor<? extends T, P> preferredConstructor, ParameterValueProvider<P> provider) {
 
 			if (preferredConstructor == null) {
 				throw new IllegalArgumentException("PreferredConstructor must not be null!");
@@ -217,15 +229,19 @@ public <T, E extends PersistentEntity<? extends T, P>, P extends PersistentPrope
 				int slot = i / 32;
 				int offset = slot * 32;
 
-				Object param = provider.getParameterValue(parameters.get(i));
+				Parameter<Object, P> parameter = parameters.get(i);
+				Class<Object> 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 <T, E extends PersistentEntity<? extends T, P>, 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));
 		}
 	}
 }

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

@@ -15,13 +15,17 @@
  */
 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;
 import java.util.Optional;
 
 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<PersistentEntity<?, ?>> 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.
 	 *

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() {

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<WithBoolean, SamplePersistentProperty>
+		val constructor = PreferredConstructorDiscoverer.discover<WithBoolean, SamplePersistentProperty>(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 <init>(kotlin.Boolean)") //
+				.hasCauseInstanceOf(IllegalArgumentException::class.java)
+	}
+
+	@Test // DATACMNS-1200
+	fun `should apply primitive defaulting for absent parameters`() {
+
+		val entity = this.entity as PersistentEntity<WithPrimitiveDefaulting, SamplePersistentProperty>
+		val constructor = PreferredConstructorDiscoverer.discover<WithPrimitiveDefaulting, SamplePersistentProperty>(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)
 }