diff --git a/pom.xml b/pom.xml index 87a56d1644..d3947be3c0 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 2.2.0.BUILD-SNAPSHOT + 2.2.0.DATACMNS-1402-SNAPSHOT Spring Data Core diff --git a/src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java index b82d5ea472..cffd852d12 100644 --- a/src/main/java/org/springframework/data/convert/KotlinClassGeneratingEntityInstantiator.java +++ b/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 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,10 +217,8 @@ private

, 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> parameters = preferredConstructor.getParameters(); @@ -225,28 +226,30 @@ private

, T> Object[] extractInvocationArguments // Prepare user-space arguments for (int i = 0; i < userParameterCount; i++) { - int slot = i / Integer.SIZE; - int offset = slot * Integer.SIZE; - Parameter parameter = parameters.get(i); - Class 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 parameter = parameters.get(index); + Class 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]; 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 fd458bda39..2f77a0319c 100644 --- a/src/main/java/org/springframework/data/mapping/model/KotlinDefaultMask.java +++ b/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. diff --git a/src/test/java/org/springframework/data/convert/ParameterizedKotlinInstantiatorUnitTests.java b/src/test/java/org/springframework/data/convert/ParameterizedKotlinInstantiatorUnitTests.java new file mode 100644 index 0000000000..d07fa11c0b --- /dev/null +++ b/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 entity; + private final int propertyCount; + private final int propertyUnderTestIndex; + private final String propertyUnderTestName; + private final EntityInstantiator entityInstantiator; + + public ParameterizedKotlinInstantiatorUnitTests(PersistentEntity 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 parameters() { + + SampleMappingContext context = new SampleMappingContext(); + + KotlinClassGeneratingEntityInstantiator generatingInstantiator = new KotlinClassGeneratingEntityInstantiator(); + ReflectionEntityInstantiator reflectionInstantiator = ReflectionEntityInstantiator.INSTANCE; + + List 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 createFixture(SampleMappingContext context, Class entityType, int propertyCount, EntityInstantiator entityInstantiator) { + + BasicPersistentEntity 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 { + + @Override + public T getParameterValue(Parameter 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 { + + @Override + public T getParameterValue(Parameter parameter) { + + if (!parameter.getName().equals(propertyUnderTestName)) { + return (T) parameter.getName(); + } + return null; + } + } +} diff --git a/src/test/kotlin/org/springframework/data/mapping/model/With32Args.kt b/src/test/kotlin/org/springframework/data/mapping/model/With32Args.kt new file mode 100644 index 0000000000..09cff72a41 --- /dev/null +++ b/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 = "" +) diff --git a/src/test/kotlin/org/springframework/data/mapping/model/With33Args.kt b/src/test/kotlin/org/springframework/data/mapping/model/With33Args.kt new file mode 100644 index 0000000000..36be17a252 --- /dev/null +++ b/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 = "" +)