DATACMNS-1402 - Fix invocation of default Kotlin constructor

pom.xml

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

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

@@ -28,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.KotlinDefaultMask;
 import org.springframework.data.mapping.model.MappingInstantiationException;
 import org.springframework.data.mapping.model.ParameterValueProvider;
 import org.springframework.data.util.ReflectionUtils;
@@ -114,7 +115,7 @@ private static Constructor<?> resolveDefaultConstructor(PersistentEntity<?, ?> e
 
 				// candidates must contain at least two additional parameters (int, DefaultConstructorMarker).
 				// Number of defaulting masks derives from the original constructor arg count
-				int syntheticParameters = (constructor.getParameterCount() / Integer.SIZE) + 1
+				int syntheticParameters = KotlinDefaultMask.getMaskCount(constructor.getParameterCount())
 						+ /* DefaultConstructorMarker */ 1;
 
 				if (constructor.getParameterCount() + syntheticParameters != candidate.getParameterCount()) {
@@ -172,6 +173,7 @@ private static boolean parametersMatch(java.lang.reflect.Parameter[] constructor
 	static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantiator {
 
 		private final ObjectInstantiator instantiator;
+		private final KFunction<?> constructor;
 		private final List<KParameter> kParameters;
 		private final Constructor<?> synthetic;
 
@@ -185,6 +187,7 @@ static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantia
 			}
 
 			this.instantiator = instantiator;
+			this.constructor = kotlinConstructor;
 			this.kParameters = kotlinConstructor.getParameters();
 			this.synthetic = constructor.getConstructor();
 		}
@@ -214,39 +217,39 @@ private <P extends PersistentProperty<P>, T> Object[] extractInvocationArguments
 				throw new IllegalArgumentException("PreferredConstructor must not be null!");
 			}
 
-			int[] defaulting = new int[(synthetic.getParameterCount() / Integer.SIZE) + 1];
-
 			Object[] params = allocateArguments(
-					synthetic.getParameterCount() + defaulting.length + /* DefaultConstructorMarker */1);
+					synthetic.getParameterCount() + KotlinDefaultMask.getMaskCount(synthetic.getParameterCount()) + /* DefaultConstructorMarker */1);
 			int userParameterCount = kParameters.size();
 
 			List<Parameter<Object, P>> parameters = preferredConstructor.getParameters();
 
 			// Prepare user-space arguments
 			for (int i = 0; i < userParameterCount; i++) {
 
-				int slot = i / Integer.SIZE;
-				int offset = slot * Integer.SIZE;
-
 				Parameter<Object, P> parameter = parameters.get(i);
-				Class<Object> type = parameter.getType().getType();
-				Object param = provider.getParameterValue(parameter);
+				params[i] = provider.getParameterValue(parameter);
+			}
 
-				KParameter kParameter = kParameters.get(i);
+			KotlinDefaultMask defaultMask = KotlinDefaultMask.from(constructor, it -> {
 
-				// what about null and parameter is mandatory? What if parameter is non-null?
-				if (kParameter.isOptional() && param == null) {
+				int index = kParameters.indexOf(it);
 
-					defaulting[slot] = defaulting[slot] | (1 << (i - offset));
+				Parameter<Object, P> parameter = parameters.get(index);
+				Class<Object> type = parameter.getType().getType();
 
+				if (it.isOptional() && params[index] == null) {
 					if (type.isPrimitive()) {
-						param = ReflectionUtils.getPrimitiveDefault(type);
+
+						// apply primitive defaulting to prevent NPE on primitive downcast
+						params[index] = ReflectionUtils.getPrimitiveDefault(type);
 					}
+					return false;
 				}
 
-				params[i] = param;
-			}
+				return true;
+			});
 
+			int[] defaulting = defaultMask.getDefaulting();
 			// append nullability masks to creation arguments
 			for (int i = 0; i < defaulting.length; i++) {
 				params[userParameterCount + i] = defaulting[i];

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

@@ -24,14 +24,19 @@
 import kotlin.reflect.KParameter;
 import kotlin.reflect.KParameter.Kind;
 import lombok.AccessLevel;
+import lombok.Getter;
 import lombok.RequiredArgsConstructor;
 
 /**
  * Value object representing defaulting masks used for Kotlin methods applying parameter defaulting.
+ *
+ * @author Mark Paluch
+ * @since 2.1
  */
 @RequiredArgsConstructor(access = AccessLevel.PRIVATE)
-class KotlinDefaultMask {
+public class KotlinDefaultMask {
 
+	@Getter
 	private final int[] defaulting;
 
 	/**
@@ -46,6 +51,16 @@ 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.
+	 */
+	public static int getMaskCount(int arguments) {
+		return ((arguments - 1) / Integer.SIZE) + 1;
+	}
+
 	/**
 	 * Creates defaulting mask(s) used to invoke Kotlin {@literal default} methods that conditionally apply parameter
 	 * values.

src/test/java/org/springframework/data/convert/ParameterizedKotlinInstantiatorUnitTests.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright 2018 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
+ *
+ *      http://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.convert;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.PreferredConstructor.Parameter;
+import org.springframework.data.mapping.context.SampleMappingContext;
+import org.springframework.data.mapping.context.SamplePersistentProperty;
+import org.springframework.data.mapping.model.BasicPersistentEntity;
+import org.springframework.data.mapping.model.ParameterValueProvider;
+import org.springframework.data.mapping.model.With32Args;
+import org.springframework.data.mapping.model.With33Args;
+import org.springframework.test.util.ReflectionTestUtils;
+
+/**
+ * Unit test to verify correct object instantiation using Kotlin defaulting via {@link KotlinClassGeneratingEntityInstantiator}.
+ *
+ * @author Mark Paluch
+ */
+@RunWith(Parameterized.class)
+public class ParameterizedKotlinInstantiatorUnitTests {
+
+	private final String valueToSet = "THE VALUE";
+	private final PersistentEntity<Object, SamplePersistentProperty> entity;
+	private final int propertyCount;
+	private final int propertyUnderTestIndex;
+	private final String propertyUnderTestName;
+	private final EntityInstantiator entityInstantiator;
+
+	public ParameterizedKotlinInstantiatorUnitTests(PersistentEntity<Object, SamplePersistentProperty> entity, int propertyCount, int propertyUnderTestIndex, String propertyUnderTestName, EntityInstantiator entityInstantiator, String label) {
+		this.entity = entity;
+		this.propertyCount = propertyCount;
+		this.propertyUnderTestIndex = propertyUnderTestIndex;
+		this.propertyUnderTestName = propertyUnderTestName;
+		this.entityInstantiator = entityInstantiator;
+	}
+
+	@Parameters(name = "{5}")
+	public static List<Object[]> parameters() {
+
+		SampleMappingContext context = new SampleMappingContext();
+
+		KotlinClassGeneratingEntityInstantiator generatingInstantiator = new KotlinClassGeneratingEntityInstantiator();
+		ReflectionEntityInstantiator reflectionInstantiator = ReflectionEntityInstantiator.INSTANCE;
+
+		List<Object[]> fixtures = new ArrayList<>();
+		fixtures.addAll(createFixture(context, With32Args.class, 32, generatingInstantiator));
+		fixtures.addAll(createFixture(context, With32Args.class, 32, reflectionInstantiator));
+		fixtures.addAll(createFixture(context, With33Args.class, 33, generatingInstantiator));
+		fixtures.addAll(createFixture(context, With33Args.class, 33, reflectionInstantiator));
+
+		return fixtures;
+	}
+
+	private static List<Object[]> createFixture(SampleMappingContext context, Class<?> entityType, int propertyCount, EntityInstantiator entityInstantiator) {
+
+		BasicPersistentEntity<Object, SamplePersistentProperty> persistentEntity = context.getPersistentEntity(entityType);
+
+		return IntStream.range(0, propertyCount).mapToObj(i -> {
+
+			return new Object[]{persistentEntity, propertyCount, i, Integer.toString(i), entityInstantiator, String.format("Property %d for %s using %s", i, entityType.getSimpleName(), entityInstantiator.getClass().getSimpleName())};
+		}).collect(Collectors.toList());
+	}
+
+	@Test // DATACMNS-1402
+	public void shouldCreateInstanceWithSinglePropertySet() {
+
+		Object instance = entityInstantiator.createInstance(entity, new SingleParameterValueProvider());
+
+		for (int i = 0; i < propertyCount; i++) {
+
+			Object value = ReflectionTestUtils.getField(instance, Integer.toString(i));
+
+			if (propertyUnderTestIndex == i) {
+				assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo(valueToSet);
+			} else {
+				assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo("");
+			}
+		}
+	}
+
+	@Test // DATACMNS-1402
+	public void shouldCreateInstanceWithAllExceptSinglePropertySet() {
+
+		Object instance = entityInstantiator.createInstance(entity, new AllButParameterValueProvider());
+
+		for (int i = 0; i < propertyCount; i++) {
+
+			Object value = ReflectionTestUtils.getField(instance, Integer.toString(i));
+
+			if (propertyUnderTestIndex == i) {
+				assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo("");
+			} else {
+				assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo(Integer.toString(i));
+			}
+		}
+	}
+
+	/**
+	 * Return the value to set for the property to test.
+	 */
+	class SingleParameterValueProvider implements ParameterValueProvider<SamplePersistentProperty> {
+
+		@Override
+		public <T> T getParameterValue(Parameter<T, SamplePersistentProperty> parameter) {
+
+			if (parameter.getName().equals(propertyUnderTestName)) {
+				return (T) valueToSet;
+			}
+			return null;
+		}
+	}
+
+	/**
+	 * Return the property name as value for all properties except the one to test.
+	 */
+	class AllButParameterValueProvider implements ParameterValueProvider<SamplePersistentProperty> {
+
+		@Override
+		public <T> T getParameterValue(Parameter<T, SamplePersistentProperty> parameter) {
+
+			if (!parameter.getName().equals(propertyUnderTestName)) {
+				return (T) parameter.getName();
+			}
+			return null;
+		}
+	}
+}

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

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2018 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
+ *
+ *      http://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
+
+data class With32Args(val `0`: String = "", val `1`: String = "", val `2`: String = "", val `3`: String = "", val `4`: String = "", val `5`: String = "",
+					  val `6`: String = "", val `7`: String = "", val `8`: String = "", val `9`: String = "", val `10`: String = "", val `11`: String = "", val `12`: String = "",
+					  val `13`: String = "", val `14`: String = "", val `15`: String = "", val `16`: String = "", val `17`: String = "", val `18`: String = "", val `19`: String = "",
+					  val `20`: String = "", val `21`: String = "", val `22`: String = "", val `23`: String = "", val `24`: String = "", val `25`: String = "", val `26`: String = "",
+					  val `27`: String = "", val `28`: String = "", val `29`: String = "", val `30`: String = "", val `31`: String = ""
+)

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

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2018 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
+ *
+ *      http://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
+
+data class With33Args(val `0`: String = "", val `1`: String = "", val `2`: String = "", val `3`: String = "", val `4`: String = "", val `5`: String = "",
+					  val `6`: String = "", val `7`: String = "", val `8`: String = "", val `9`: String = "", val `10`: String = "", val `11`: String = "", val `12`: String = "",
+					  val `13`: String = "", val `14`: String = "", val `15`: String = "", val `16`: String = "", val `17`: String = "", val `18`: String = "", val `19`: String = "",
+					  val `20`: String = "", val `21`: String = "", val `22`: String = "", val `23`: String = "", val `24`: String = "", val `25`: String = "", val `26`: String = "",
+					  val `27`: String = "", val `28`: String = "", val `29`: String = "", val `30`: String = "", val `31`: String = "", val `32`: String = ""
+)