Skip to content

Commit 6481cbe

Browse files
mp911deodrotbohm
mp911de
authored and
odrotbohm
committed
DATACMNS-1402 - Fix invocation of default Kotlin constructor.
We now correctly calculate the number of defaulting masks used to represent constructor arguments. Previously, we've been one off which caused that Kotlin classes with 32/33 parameters weren't able to be instantiated. We also now reuse KotlinDefaultMask to apply defaulting calculation and removed code duplicates.
1 parent 3935d1f commit 6481cbe

File tree

5 files changed

+233
-17
lines changed

5 files changed

+233
-17
lines changed

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

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.springframework.data.mapping.PersistentProperty;
2929
import org.springframework.data.mapping.PreferredConstructor;
3030
import org.springframework.data.mapping.PreferredConstructor.Parameter;
31+
import org.springframework.data.mapping.model.KotlinDefaultMask;
3132
import org.springframework.data.mapping.model.MappingInstantiationException;
3233
import org.springframework.data.mapping.model.ParameterValueProvider;
3334
import org.springframework.data.util.ReflectionUtils;
@@ -114,7 +115,7 @@ private static Constructor<?> resolveDefaultConstructor(PersistentEntity<?, ?> e
114115

115116
// candidates must contain at least two additional parameters (int, DefaultConstructorMarker).
116117
// Number of defaulting masks derives from the original constructor arg count
117-
int syntheticParameters = (constructor.getParameterCount() / Integer.SIZE) + 1
118+
int syntheticParameters = KotlinDefaultMask.getMaskCount(constructor.getParameterCount())
118119
+ /* DefaultConstructorMarker */ 1;
119120

120121
if (constructor.getParameterCount() + syntheticParameters != candidate.getParameterCount()) {
@@ -172,6 +173,7 @@ private static boolean parametersMatch(java.lang.reflect.Parameter[] constructor
172173
static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantiator {
173174

174175
private final ObjectInstantiator instantiator;
176+
private final KFunction<?> constructor;
175177
private final List<KParameter> kParameters;
176178
private final Constructor<?> synthetic;
177179

@@ -185,6 +187,7 @@ static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantia
185187
}
186188

187189
this.instantiator = instantiator;
190+
this.constructor = kotlinConstructor;
188191
this.kParameters = kotlinConstructor.getParameters();
189192
this.synthetic = constructor.getConstructor();
190193
}
@@ -214,39 +217,39 @@ private <P extends PersistentProperty<P>, T> Object[] extractInvocationArguments
214217
throw new IllegalArgumentException("PreferredConstructor must not be null!");
215218
}
216219

217-
int[] defaulting = new int[(synthetic.getParameterCount() / Integer.SIZE) + 1];
218-
219220
Object[] params = allocateArguments(
220-
synthetic.getParameterCount() + defaulting.length + /* DefaultConstructorMarker */1);
221+
synthetic.getParameterCount() + KotlinDefaultMask.getMaskCount(synthetic.getParameterCount()) + /* DefaultConstructorMarker */1);
221222
int userParameterCount = kParameters.size();
222223

223224
List<Parameter<Object, P>> parameters = preferredConstructor.getParameters();
224225

225226
// Prepare user-space arguments
226227
for (int i = 0; i < userParameterCount; i++) {
227228

228-
int slot = i / Integer.SIZE;
229-
int offset = slot * Integer.SIZE;
230-
231229
Parameter<Object, P> parameter = parameters.get(i);
232-
Class<Object> type = parameter.getType().getType();
233-
Object param = provider.getParameterValue(parameter);
230+
params[i] = provider.getParameterValue(parameter);
231+
}
234232

235-
KParameter kParameter = kParameters.get(i);
233+
KotlinDefaultMask defaultMask = KotlinDefaultMask.from(constructor, it -> {
236234

237-
// what about null and parameter is mandatory? What if parameter is non-null?
238-
if (kParameter.isOptional() && param == null) {
235+
int index = kParameters.indexOf(it);
239236

240-
defaulting[slot] = defaulting[slot] | (1 << (i - offset));
237+
Parameter<Object, P> parameter = parameters.get(index);
238+
Class<Object> type = parameter.getType().getType();
241239

240+
if (it.isOptional() && params[index] == null) {
242241
if (type.isPrimitive()) {
243-
param = ReflectionUtils.getPrimitiveDefault(type);
242+
243+
// apply primitive defaulting to prevent NPE on primitive downcast
244+
params[index] = ReflectionUtils.getPrimitiveDefault(type);
244245
}
246+
return false;
245247
}
246248

247-
params[i] = param;
248-
}
249+
return true;
250+
});
249251

252+
int[] defaulting = defaultMask.getDefaulting();
250253
// append nullability masks to creation arguments
251254
for (int i = 0; i < defaulting.length; i++) {
252255
params[userParameterCount + i] = defaulting[i];

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,19 @@
2424
import kotlin.reflect.KParameter;
2525
import kotlin.reflect.KParameter.Kind;
2626
import lombok.AccessLevel;
27+
import lombok.Getter;
2728
import lombok.RequiredArgsConstructor;
2829

2930
/**
3031
* Value object representing defaulting masks used for Kotlin methods applying parameter defaulting.
32+
*
33+
* @author Mark Paluch
34+
* @since 2.1
3135
*/
3236
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
33-
class KotlinDefaultMask {
37+
public class KotlinDefaultMask {
3438

39+
@Getter
3540
private final int[] defaulting;
3641

3742
/**
@@ -46,6 +51,16 @@ public void forEach(IntConsumer maskCallback) {
4651
}
4752
}
4853

54+
/**
55+
* Return the number of defaulting masks required to represent the number of {@code arguments}.
56+
*
57+
* @param arguments number of method arguments.
58+
* @return the number of defaulting masks required.
59+
*/
60+
public static int getMaskCount(int arguments) {
61+
return ((arguments - 1) / Integer.SIZE) + 1;
62+
}
63+
4964
/**
5065
* Creates defaulting mask(s) used to invoke Kotlin {@literal default} methods that conditionally apply parameter
5166
* values.

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/*
2+
* Copyright 2018 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.convert;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
20+
import java.util.ArrayList;
21+
import java.util.List;
22+
import java.util.stream.Collectors;
23+
import java.util.stream.IntStream;
24+
25+
import org.junit.Test;
26+
import org.junit.runner.RunWith;
27+
import org.junit.runners.Parameterized;
28+
import org.junit.runners.Parameterized.Parameters;
29+
import org.springframework.data.mapping.PersistentEntity;
30+
import org.springframework.data.mapping.PreferredConstructor.Parameter;
31+
import org.springframework.data.mapping.context.SampleMappingContext;
32+
import org.springframework.data.mapping.context.SamplePersistentProperty;
33+
import org.springframework.data.mapping.model.BasicPersistentEntity;
34+
import org.springframework.data.mapping.model.ParameterValueProvider;
35+
import org.springframework.data.mapping.model.With32Args;
36+
import org.springframework.data.mapping.model.With33Args;
37+
import org.springframework.test.util.ReflectionTestUtils;
38+
39+
/**
40+
* Unit test to verify correct object instantiation using Kotlin defaulting via {@link KotlinClassGeneratingEntityInstantiator}.
41+
*
42+
* @author Mark Paluch
43+
*/
44+
@RunWith(Parameterized.class)
45+
public class ParameterizedKotlinInstantiatorUnitTests {
46+
47+
private final String valueToSet = "THE VALUE";
48+
private final PersistentEntity<Object, SamplePersistentProperty> entity;
49+
private final int propertyCount;
50+
private final int propertyUnderTestIndex;
51+
private final String propertyUnderTestName;
52+
private final EntityInstantiator entityInstantiator;
53+
54+
public ParameterizedKotlinInstantiatorUnitTests(PersistentEntity<Object, SamplePersistentProperty> entity, int propertyCount, int propertyUnderTestIndex, String propertyUnderTestName, EntityInstantiator entityInstantiator, String label) {
55+
this.entity = entity;
56+
this.propertyCount = propertyCount;
57+
this.propertyUnderTestIndex = propertyUnderTestIndex;
58+
this.propertyUnderTestName = propertyUnderTestName;
59+
this.entityInstantiator = entityInstantiator;
60+
}
61+
62+
@Parameters(name = "{5}")
63+
public static List<Object[]> parameters() {
64+
65+
SampleMappingContext context = new SampleMappingContext();
66+
67+
KotlinClassGeneratingEntityInstantiator generatingInstantiator = new KotlinClassGeneratingEntityInstantiator();
68+
ReflectionEntityInstantiator reflectionInstantiator = ReflectionEntityInstantiator.INSTANCE;
69+
70+
List<Object[]> fixtures = new ArrayList<>();
71+
fixtures.addAll(createFixture(context, With32Args.class, 32, generatingInstantiator));
72+
fixtures.addAll(createFixture(context, With32Args.class, 32, reflectionInstantiator));
73+
fixtures.addAll(createFixture(context, With33Args.class, 33, generatingInstantiator));
74+
fixtures.addAll(createFixture(context, With33Args.class, 33, reflectionInstantiator));
75+
76+
return fixtures;
77+
}
78+
79+
private static List<Object[]> createFixture(SampleMappingContext context, Class<?> entityType, int propertyCount, EntityInstantiator entityInstantiator) {
80+
81+
BasicPersistentEntity<Object, SamplePersistentProperty> persistentEntity = context.getPersistentEntity(entityType);
82+
83+
return IntStream.range(0, propertyCount).mapToObj(i -> {
84+
85+
return new Object[]{persistentEntity, propertyCount, i, Integer.toString(i), entityInstantiator, String.format("Property %d for %s using %s", i, entityType.getSimpleName(), entityInstantiator.getClass().getSimpleName())};
86+
}).collect(Collectors.toList());
87+
}
88+
89+
@Test // DATACMNS-1402
90+
public void shouldCreateInstanceWithSinglePropertySet() {
91+
92+
Object instance = entityInstantiator.createInstance(entity, new SingleParameterValueProvider());
93+
94+
for (int i = 0; i < propertyCount; i++) {
95+
96+
Object value = ReflectionTestUtils.getField(instance, Integer.toString(i));
97+
98+
if (propertyUnderTestIndex == i) {
99+
assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo(valueToSet);
100+
} else {
101+
assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo("");
102+
}
103+
}
104+
}
105+
106+
@Test // DATACMNS-1402
107+
public void shouldCreateInstanceWithAllExceptSinglePropertySet() {
108+
109+
Object instance = entityInstantiator.createInstance(entity, new AllButParameterValueProvider());
110+
111+
for (int i = 0; i < propertyCount; i++) {
112+
113+
Object value = ReflectionTestUtils.getField(instance, Integer.toString(i));
114+
115+
if (propertyUnderTestIndex == i) {
116+
assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo("");
117+
} else {
118+
assertThat(value).describedAs("Property " + i + " of " + entity).isEqualTo(Integer.toString(i));
119+
}
120+
}
121+
}
122+
123+
/**
124+
* Return the value to set for the property to test.
125+
*/
126+
class SingleParameterValueProvider implements ParameterValueProvider<SamplePersistentProperty> {
127+
128+
@Override
129+
public <T> T getParameterValue(Parameter<T, SamplePersistentProperty> parameter) {
130+
131+
if (parameter.getName().equals(propertyUnderTestName)) {
132+
return (T) valueToSet;
133+
}
134+
return null;
135+
}
136+
}
137+
138+
/**
139+
* Return the property name as value for all properties except the one to test.
140+
*/
141+
class AllButParameterValueProvider implements ParameterValueProvider<SamplePersistentProperty> {
142+
143+
@Override
144+
public <T> T getParameterValue(Parameter<T, SamplePersistentProperty> parameter) {
145+
146+
if (!parameter.getName().equals(propertyUnderTestName)) {
147+
return (T) parameter.getName();
148+
}
149+
return null;
150+
}
151+
}
152+
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright 2018 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.mapping.model
17+
18+
data class With32Args(val `0`: String = "", val `1`: String = "", val `2`: String = "", val `3`: String = "", val `4`: String = "", val `5`: String = "",
19+
val `6`: String = "", val `7`: String = "", val `8`: String = "", val `9`: String = "", val `10`: String = "", val `11`: String = "", val `12`: String = "",
20+
val `13`: String = "", val `14`: String = "", val `15`: String = "", val `16`: String = "", val `17`: String = "", val `18`: String = "", val `19`: String = "",
21+
val `20`: String = "", val `21`: String = "", val `22`: String = "", val `23`: String = "", val `24`: String = "", val `25`: String = "", val `26`: String = "",
22+
val `27`: String = "", val `28`: String = "", val `29`: String = "", val `30`: String = "", val `31`: String = ""
23+
)

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright 2018 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.mapping.model
17+
18+
data class With33Args(val `0`: String = "", val `1`: String = "", val `2`: String = "", val `3`: String = "", val `4`: String = "", val `5`: String = "",
19+
val `6`: String = "", val `7`: String = "", val `8`: String = "", val `9`: String = "", val `10`: String = "", val `11`: String = "", val `12`: String = "",
20+
val `13`: String = "", val `14`: String = "", val `15`: String = "", val `16`: String = "", val `17`: String = "", val `18`: String = "", val `19`: String = "",
21+
val `20`: String = "", val `21`: String = "", val `22`: String = "", val `23`: String = "", val `24`: String = "", val `25`: String = "", val `26`: String = "",
22+
val `27`: String = "", val `28`: String = "", val `29`: String = "", val `30`: String = "", val `31`: String = "", val `32`: String = ""
23+
)

0 commit comments

Comments
 (0)