From fb25fbe1a54d973fd202739189a0e922699b9e47 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 25 Jul 2017 10:22:10 +0200 Subject: [PATCH 1/4] DATACMNS-1126 - Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index ea6303c6eb..779ba95083 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 2.0.0.BUILD-SNAPSHOT + 2.0.0.DATACMNS-1126-SNAPSHOT Spring Data Core From 700775fe044d8dd26e21dfa0e6371509f31cddb3 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 26 Jul 2017 12:07:20 +0200 Subject: [PATCH 2/4] DATACMNS-1126 - Add Kotlin constructor support. We now discover Kotlin constructors and apply parameter defaulting to allow construction of immutable value objects. Constructor discovery uses primary constructors by default and considers PersistenceConstructor annotations. ClassGeneratingEntityInstantiator can instantiate Kotlin classes with default parameters by resolving the synthetic constructor. Null values translate to parameter defaults. class Person(val firstname: String = "Walter") { } class Address(val street: String, val city: String) { @PersistenceConstructor constructor(street: String = "Unknown", city: String = "Unknown", country: String) : this(street, city) } --- pom.xml | 43 +++ .../ClassGeneratingEntityInstantiator.java | 247 ++++++++++++++++-- .../model/PreferredConstructorDiscoverer.java | 33 +++ .../data/util/ReflectionUtils.java | 49 ++-- ...ingEntityInstantiatorDataClassUnitTests.kt | 98 +++++++ ...ionEntityInstantiatorDataClassUnitTests.kt | 77 ++++++ ...PreferredConstructorDiscovererUnitTests.kt | 90 +++++++ 7 files changed, 606 insertions(+), 31 deletions(-) create mode 100644 src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt create mode 100644 src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt create mode 100644 src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt diff --git a/pom.xml b/pom.xml index 779ba95083..d27b5a8d80 100644 --- a/pom.xml +++ b/pom.xml @@ -231,6 +231,49 @@ test + + + org.jetbrains.kotlin + kotlin-stdlib + ${kotlin} + true + + + + org.jetbrains.kotlin + kotlin-reflect + ${kotlin} + true + + + + org.jetbrains.kotlin + kotlin-test + ${kotlin} + test + + + + com.nhaarman + mockito-kotlin + 1.5.0 + test + + + org.jetbrains.kotlin + kotlin-stdlib + + + org.jetbrains.kotlin + kotlin-reflect + + + org.mockito + mockito-core + + + + org.scala-lang diff --git a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java index deed67e841..ef738e7c7f 100644 --- a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java @@ -17,6 +17,10 @@ import static org.springframework.asm.Opcodes.*; +import kotlin.reflect.KFunction; +import kotlin.reflect.KParameter; +import kotlin.reflect.jvm.ReflectJvmMapping; + import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; import java.security.AccessController; @@ -26,6 +30,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.IntStream; import org.springframework.asm.ClassWriter; import org.springframework.asm.MethodVisitor; @@ -37,6 +42,7 @@ 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.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -47,6 +53,8 @@ * {@link PersistentEntity}'s {@link PreferredConstructor} to instantiate an instance of the entity by dynamically * generating factory methods with appropriate constructor invocations via ASM. If we cannot generate byte code for a * type, we gracefully fall-back to the {@link ReflectionEntityInstantiator}. + *

+ * Adopts to Kotlin constructors using parameter defaulting. * * @author Thomas Darimont * @author Oliver Gierke @@ -57,6 +65,19 @@ */ public class ClassGeneratingEntityInstantiator implements EntityInstantiator { + private static final int ARG_CACHE_SIZE = 100; + + private static final ThreadLocal OBJECT_POOL = ThreadLocal.withInitial(() -> { + + Object[][] cached = new Object[ARG_CACHE_SIZE][]; + + for (int i = 0; i < ARG_CACHE_SIZE; i++) { + cached[i] = new Object[i]; + } + + return cached; + }); + private final ObjectInstantiatorClassGenerator generator; private volatile Map, EntityInstantiator> entityInstantiators = new HashMap<>(32); @@ -120,7 +141,19 @@ private EntityInstantiator createEntityInstantiator(PersistentEntity entit } try { - return new EntityInstantiatorAdapter(createObjectInstantiator(entity)); + + if (ReflectionUtils.isKotlinClass(entity.getType())) { + + PreferredConstructor defaultConstructor = new DefaultingKotlinConstructorResolver(entity) + .getDefaultConstructor(); + + if (defaultConstructor != null) { + return new DefaultingKotlinClassEntityInstantiator(createObjectInstantiator(entity, defaultConstructor), + entity.getPersistenceConstructor()); + } + } + + return new EntityInstantiatorAdapter(createObjectInstantiator(entity, entity.getPersistenceConstructor())); } catch (Throwable ex) { return ReflectionEntityInstantiator.INSTANCE; } @@ -151,22 +184,28 @@ private boolean shouldUseReflectionEntityInstantiator(PersistentEntity ent } /** - * Creates a dynamically generated {@link ObjectInstantiator} for the given {@link PersistentEntity}. There will - * always be exactly one {@link ObjectInstantiator} instance per {@link PersistentEntity}. - *

+ * Creates a dynamically generated {@link ObjectInstantiator} for the given {@link PersistentEntity} and + * {@link PreferredConstructor}. There will always be exactly one {@link ObjectInstantiator} instance per + * {@link PersistentEntity}. * * @param entity + * @param constructor * @return */ - private ObjectInstantiator createObjectInstantiator(PersistentEntity entity) { + private ObjectInstantiator createObjectInstantiator(PersistentEntity entity, + @Nullable PreferredConstructor constructor) { try { - return (ObjectInstantiator) this.generator.generateCustomInstantiatorClass(entity).newInstance(); + return (ObjectInstantiator) this.generator.generateCustomInstantiatorClass(entity, constructor).newInstance(); } catch (Exception e) { throw new RuntimeException(e); } } + private static Object[] allocateArguments(int argumentCount) { + return argumentCount < ARG_CACHE_SIZE ? OBJECT_POOL.get()[argumentCount] : new Object[argumentCount]; + } + /** * Adapter to forward an invocation of the {@link EntityInstantiator} API to an {@link ObjectInstantiator}. * @@ -216,7 +255,7 @@ public , P extends PersistentPrope private

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

provider) { - if (provider == null || constructor == null || !constructor.hasParameters()) { + if (constructor == null || !constructor.hasParameters()) { return EMPTY_ARRAY; } @@ -230,6 +269,180 @@ private

, T> Object[] extractInvocationArguments } } + /** + * Resolves a {@link PreferredConstructor} to a synthetic Kotlin constructor accepting the same user-space parameters + * suffixed by Kotlin-specifics required for defaulting and the {@code kotlin.jvm.internal.DefaultConstructorMarker}. + * + * @since 2.0 + * @author Mark Paluch + */ + static class DefaultingKotlinConstructorResolver { + + @Nullable private final PreferredConstructor defaultConstructor; + + @SuppressWarnings("unchecked") + DefaultingKotlinConstructorResolver(PersistentEntity entity) { + + Constructor hit = resolveDefaultConstructor(entity); + PreferredConstructor persistenceConstructor = entity.getPersistenceConstructor(); + + if (hit != null && persistenceConstructor != null) { + this.defaultConstructor = new PreferredConstructor<>(hit, + persistenceConstructor.getParameters().toArray(new Parameter[0])); + } else { + this.defaultConstructor = null; + } + } + + @Nullable + private static Constructor resolveDefaultConstructor(PersistentEntity entity) { + + if (entity.getPersistenceConstructor() == null) { + return null; + } + + Constructor hit = null; + Constructor constructor = entity.getPersistenceConstructor().getConstructor(); + + for (Constructor candidate : entity.getType().getDeclaredConstructors()) { + + // use only synthetic constructors + if (!candidate.isSynthetic()) { + continue; + } + + // with a parameter count greater zero + if (constructor.getParameterCount() == 0) { + continue; + } + + // candidates must contain at least two additional parameters (int, DefaultConstructorMarker) + if (constructor.getParameterCount() + 2 > candidate.getParameterCount()) { + continue; + } + + java.lang.reflect.Parameter[] constructorParameters = constructor.getParameters(); + java.lang.reflect.Parameter[] candidateParameters = candidate.getParameters(); + + if (!candidateParameters[candidateParameters.length - 1].getType().getName() + .equals("kotlin.jvm.internal.DefaultConstructorMarker")) { + continue; + } + + if (parametersMatch(constructorParameters, candidateParameters)) { + hit = candidate; + break; + } + } + + return hit; + } + + private static boolean parametersMatch(java.lang.reflect.Parameter[] constructorParameters, + java.lang.reflect.Parameter[] candidateParameters) { + + return IntStream.range(0, constructorParameters.length) + .allMatch(i -> constructorParameters[i].getType().equals(candidateParameters[i].getType())); + } + + @Nullable + PreferredConstructor getDefaultConstructor() { + return defaultConstructor; + } + } + + /** + * Entity instantiator for Kotlin constructors that apply parameter defaulting. Kotlin constructors that apply + * argument defaulting are marked with {@link kotlin.jvm.internal.DefaultConstructorMarker} and accept additional + * parameters besides the regular (user-space) parameters. Additional parameters are: + *

    + *
  • defaulting bitmask ({@code int}), a bit mask slot for each 32 parameters
    • + *
  • {@code DefaultConstructorMarker} (usually null)
    • + *
+ * Defaulting bitmask + *

+ * The defaulting bitmask is a 32 bit integer representing which positional argument should be defaulted. Defaulted + * arguments are passed as {@literal null} and require the appropriate positional bit set ( {@code 1 << 2} for the 2. + * argument)). Since the bitmask represents only 32 bit states, it requires additional masks (slots) if more than 32 + * arguments are represented. + * + * @author Mark Paluch + * @since 2.0 + */ + static class DefaultingKotlinClassEntityInstantiator implements EntityInstantiator { + + private final ObjectInstantiator instantiator; + private final List kParameters; + private final Constructor synthetic; + + DefaultingKotlinClassEntityInstantiator(ObjectInstantiator instantiator, PreferredConstructor constructor) { + + KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(constructor.getConstructor()); + + if (kotlinConstructor == null) { + throw new IllegalArgumentException( + "No corresponding Kotlin constructor found for " + constructor.getConstructor()); + } + + this.instantiator = instantiator; + this.kParameters = kotlinConstructor.getParameters(); + this.synthetic = constructor.getConstructor(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.convert.EntityInstantiator#createInstance(org.springframework.data.mapping.PersistentEntity, org.springframework.data.mapping.model.ParameterValueProvider) + */ + @Override + @SuppressWarnings("unchecked") + public , P extends PersistentProperty

> T createInstance(E entity, + ParameterValueProvider

provider) { + + PreferredConstructor preferredConstructor = entity.getPersistenceConstructor(); + Assert.notNull(preferredConstructor, "PreferredConstructor must not be null!"); + + int[] defaulting = new int[(synthetic.getParameterCount() / 32) + 1]; + + Object[] params = allocateArguments( + synthetic.getParameterCount() + defaulting.length + /* DefaultConstructorMarker */1); + int userParameterCount = kParameters.size(); + + List> parameters = preferredConstructor.getParameters(); + + // Prepare user-space arguments + for (int i = 0; i < userParameterCount; i++) { + + int slot = i / 32; + int offset = slot * 32; + + Object param = provider.getParameterValue(parameters.get(i)); + + KParameter kParameter = kParameters.get(i); + + // what about null and parameter is mandatory? What if parameter is non-null? + if (kParameter.isOptional()) { + + if (param == null) { + defaulting[slot] = defaulting[slot] | (1 << (i - offset)); + } + } + + params[i] = param; + } + + // append nullability masks to creation arguments + for (int i = 0; i < defaulting.length; i++) { + params[userParameterCount + i] = defaulting[i]; + } + + try { + return (T) instantiator.newInstance(params); + } finally { + Arrays.fill(params, null); + } + } + } + /** * Needs to be public as otherwise the implementation class generated does not see the interface from the classloader. * @@ -290,7 +503,7 @@ static class ObjectInstantiatorClassGenerator { private final ByteArrayClassLoader classLoader; - private ObjectInstantiatorClassGenerator() { + ObjectInstantiatorClassGenerator() { this.classLoader = AccessController.doPrivileged( (PrivilegedAction) () -> new ByteArrayClassLoader(ClassUtils.getDefaultClassLoader())); @@ -300,12 +513,14 @@ private ObjectInstantiatorClassGenerator() { * Generate a new class for the given {@link PersistentEntity}. * * @param entity + * @param constructor * @return */ - public Class generateCustomInstantiatorClass(PersistentEntity entity) { + public Class generateCustomInstantiatorClass(PersistentEntity entity, + @Nullable PreferredConstructor constructor) { String className = generateClassName(entity); - byte[] bytecode = generateBytecode(className, entity); + byte[] bytecode = generateBytecode(className, entity, constructor); return classLoader.loadClass(className, bytecode); } @@ -323,9 +538,11 @@ private String generateClassName(PersistentEntity entity) { * * @param internalClassName * @param entity + * @param constructor * @return */ - public byte[] generateBytecode(String internalClassName, PersistentEntity entity) { + public byte[] generateBytecode(String internalClassName, PersistentEntity entity, + @Nullable PreferredConstructor constructor) { ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS); @@ -334,7 +551,7 @@ public byte[] generateBytecode(String internalClassName, PersistentEntity visitDefaultConstructor(cw); - visitCreateMethod(cw, entity); + visitCreateMethod(cw, entity, constructor); cw.visitEnd(); @@ -357,8 +574,10 @@ private void visitDefaultConstructor(ClassWriter cw) { * * @param cw * @param entity + * @param constructor */ - private void visitCreateMethod(ClassWriter cw, PersistentEntity entity) { + private void visitCreateMethod(ClassWriter cw, PersistentEntity entity, + @Nullable PreferredConstructor constructor) { String entityTypeResourcePath = Type.getInternalName(entity.getType()); @@ -368,8 +587,6 @@ private void visitCreateMethod(ClassWriter cw, PersistentEntity entity) { mv.visitTypeInsn(NEW, entityTypeResourcePath); mv.visitInsn(DUP); - PreferredConstructor constructor = entity.getPersistenceConstructor(); - if (constructor != null) { Constructor ctor = constructor.getConstructor(); diff --git a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java index eb8ed3fcfd..17efcb4faf 100644 --- a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java +++ b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java @@ -15,6 +15,11 @@ */ package org.springframework.data.mapping.model; +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KFunction; +import kotlin.reflect.full.KClasses; +import kotlin.reflect.jvm.ReflectJvmMapping; + import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; import java.util.List; @@ -26,6 +31,7 @@ import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.mapping.PreferredConstructor.Parameter; import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; @@ -73,6 +79,33 @@ protected PreferredConstructorDiscoverer(TypeInformation type, @Nullable Pers int numberOfArgConstructors = 0; Class rawOwningType = type.getType(); + if (ReflectionUtils.isKotlinClass(type.getType())) { + + for (Constructor candidate : rawOwningType.getDeclaredConstructors()) { + + PreferredConstructor preferredConstructor = buildPreferredConstructor(candidate, type, entity); + + // Synthetic constructors should not be considered + if (preferredConstructor.getConstructor().isSynthetic()) { + continue; + } + + // Explicitly defined constructor trumps all + if (preferredConstructor.isExplicitlyAnnotated()) { + this.constructor = preferredConstructor; + return; + } + } + + KFunction primaryConstructor = KClasses + .getPrimaryConstructor(JvmClassMappingKt.getKotlinClass(type.getType())); + Constructor javaConstructor = ReflectJvmMapping.getJavaConstructor(primaryConstructor); + if (javaConstructor != null) { + this.constructor = buildPreferredConstructor(javaConstructor, type, entity); + return; + } + } + for (Constructor candidate : rawOwningType.getDeclaredConstructors()) { PreferredConstructor preferredConstructor = buildPreferredConstructor(candidate, type, entity); diff --git a/src/main/java/org/springframework/data/util/ReflectionUtils.java b/src/main/java/org/springframework/data/util/ReflectionUtils.java index 726a487321..7e798053c0 100644 --- a/src/main/java/org/springframework/data/util/ReflectionUtils.java +++ b/src/main/java/org/springframework/data/util/ReflectionUtils.java @@ -40,19 +40,23 @@ /** * Spring Data specific reflection utility methods and classes. - * + * * @author Oliver Gierke * @author Thomas Darimont * @author Christoph Strobl + * @author Mark Paluch * @since 1.5 */ @UtilityClass public class ReflectionUtils { + private static final boolean KOTLIN_IS_PRESENT = ClassUtils.isPresent("kotlin.Unit", + BeanUtils.class.getClassLoader()); + /** * Creates an instance of the class with the given fully qualified name or returns the given default instance if the * class cannot be loaded or instantiated. - * + * * @param classname the fully qualified class name to create an instance for. * @param defaultInstance the instance to fall back to in case the given class cannot be loaded or instantiated. * @return @@ -70,7 +74,7 @@ public static T createInstanceIfPresent(String classname, T defaultInstance) /** * A {@link FieldFilter} that has a description. - * + * * @author Oliver Gierke */ public interface DescribedFieldFilter extends FieldFilter { @@ -78,7 +82,7 @@ public interface DescribedFieldFilter extends FieldFilter { /** * Returns the description of the field filter. Used in exceptions being thrown in case uniqueness shall be enforced * on the field filter. - * + * * @return */ String getDescription(); @@ -86,7 +90,7 @@ public interface DescribedFieldFilter extends FieldFilter { /** * A {@link FieldFilter} for a given annotation. - * + * * @author Oliver Gierke */ @RequiredArgsConstructor @@ -94,7 +98,7 @@ public static class AnnotationFieldFilter implements DescribedFieldFilter { private final @NonNull Class annotationType; - /* + /* * (non-Javadoc) * @see org.springframework.util.ReflectionUtils.FieldFilter#matches(java.lang.reflect.Field) */ @@ -102,7 +106,7 @@ public boolean matches(Field field) { return AnnotationUtils.getAnnotation(field, annotationType) != null; } - /* + /* * (non-Javadoc) * @see org.springframework.data.util.ReflectionUtils.DescribedFieldFilter#getDescription() */ @@ -113,7 +117,7 @@ public String getDescription() { /** * Finds the first field on the given class matching the given {@link FieldFilter}. - * + * * @param type must not be {@literal null}. * @param filter must not be {@literal null}. * @return the field matching the filter or {@literal null} in case no field could be found. @@ -136,7 +140,7 @@ public String getDescription() { /** * Finds the field matching the given {@link DescribedFieldFilter}. Will make sure there's only one field matching the * filter. - * + * * @see #findField(Class, DescribedFieldFilter, boolean) * @param type must not be {@literal null}. * @param filter must not be {@literal null}. @@ -151,7 +155,7 @@ public static Field findField(Class type, DescribedFieldFilter filter) { /** * Finds the field matching the given {@link DescribedFieldFilter}. Will make sure there's only one field matching the * filter in case {@code enforceUniqueness} is {@literal true}. - * + * * @param type must not be {@literal null}. * @param filter must not be {@literal null}. * @param enforceUniqueness whether to enforce uniqueness of the field @@ -194,7 +198,7 @@ public static Field findField(Class type, DescribedFieldFilter filter, boolea /** * Finds the field of the given name on the given type. - * + * * @param type must not be {@literal null}. * @param name must not be {@literal null} or empty. * @return @@ -213,7 +217,7 @@ public static Field findRequiredField(Class type, String name) { /** * Sets the given field on the given object to the given value. Will make sure the given field is accessible. - * + * * @param field must not be {@literal null}. * @param target must not be {@literal null}. * @param value @@ -226,7 +230,7 @@ public static void setField(Field field, Object target, @Nullable Object value) /** * Finds a constructor on the given type that matches the given constructor arguments. - * + * * @param type must not be {@literal null}. * @param constructorArguments must not be {@literal null}. * @return a {@link Constructor} that is compatible with the given arguments. @@ -243,7 +247,7 @@ public static Optional> findConstructor(Class type, Object... /** * Returns the method with the given name of the given class and parameter types. - * + * * @param type must not be {@literal null}. * @param name must not be {@literal null}. * @param parameterTypes must not be {@literal null}. @@ -269,7 +273,7 @@ public static Method findRequiredMethod(Class type, String name, Class... /** * Returns a {@link Stream} of the return and parameters types of the given {@link Method}. - * + * * @param method must not be {@literal null}. * @return * @since 2.0 @@ -286,7 +290,7 @@ public static Stream> returnTypeAndParameters(Method method) { /** * Returns the {@link Method} with the given name and parameters declared on the given type, if available. - * + * * @param type must not be {@literal null}. * @param name must not be {@literal null} or empty. * @param parameterTypes must not be {@literal null}. @@ -338,4 +342,17 @@ private static boolean argumentsMatch(Class[] parameterTypes, Object[] argume return true; } + + /** + * Return true if the specified class is a Kotlin one. + * + * @return {@literal true} if {@code type} is a Kotlin class. + * @since 2.0 + */ + public static boolean isKotlinClass(Class type) { + + return KOTLIN_IS_PRESENT && Arrays.stream(type.getDeclaredAnnotations()) // + .map(Annotation::annotationType) // + .anyMatch(annotation -> annotation.getName().equals("kotlin.Metadata")); + } } diff --git a/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt new file mode 100644 index 0000000000..f7d5f5a752 --- /dev/null +++ b/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt @@ -0,0 +1,98 @@ +/* + * Copyright 2017 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 com.nhaarman.mockito_kotlin.any +import com.nhaarman.mockito_kotlin.doReturn +import com.nhaarman.mockito_kotlin.whenever +import org.assertj.core.api.Assertions +import org.junit.Test +import org.junit.runner.RunWith +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.ParameterValueProvider +import org.springframework.data.mapping.model.PreferredConstructorDiscoverer + +/** + * Unit tests for [ClassGeneratingEntityInstantiator] creating instances using Kotlin data classes. + * + * @author Mark Paluch + */ +@RunWith(MockitoJUnitRunner::class) +@Suppress("UNCHECKED_CAST") +class ClassGeneratingEntityInstantiatorDataClassUnitTests { + + @Mock lateinit var entity: PersistentEntity<*, *> + @Mock lateinit var provider: ParameterValueProvider + + @Test // DATACMNS-1126 + fun `should create instance`() { + + val entity = this.entity as PersistentEntity + val constructor = PreferredConstructorDiscoverer(Contact::class.java).constructor + + doReturn("Walter", "White").`when`(provider).getParameterValue(any()) + doReturn(constructor).whenever(entity).persistenceConstructor + doReturn(constructor.constructor.declaringClass).whenever(entity).type + + val instance: Contact = ClassGeneratingEntityInstantiator().createInstance(entity, provider) + + Assertions.assertThat(instance.firstname).isEqualTo("Walter") + Assertions.assertThat(instance.lastname).isEqualTo("White") + } + + @Test // DATACMNS-1126 + fun `should create instance and fill in defaults`() { + + val entity = this.entity as PersistentEntity + val constructor = PreferredConstructorDiscoverer(ContactWithDefaulting::class.java).constructor + + doReturn("Walter", null, "Skyler", null, null, null, null, null, null, null, /* 0-9 */ + null, null, null, null, null, null, null, null, null, null, /* 10-19 */ + null, null, null, null, null, null, null, null, null, null, /* 20 - 29 */ + null, "Walter", null, "Junior", null).`when`(provider).getParameterValue(any()) + doReturn(constructor).whenever(entity).persistenceConstructor + doReturn(constructor.constructor.declaringClass).whenever(entity).type + + val instance: ContactWithDefaulting = ClassGeneratingEntityInstantiator().createInstance(entity, provider) + + Assertions.assertThat(instance.prop0).isEqualTo("Walter") + Assertions.assertThat(instance.prop2).isEqualTo("Skyler") + Assertions.assertThat(instance.prop31).isEqualTo("Walter") + Assertions.assertThat(instance.prop32).isEqualTo("White") + Assertions.assertThat(instance.prop33).isEqualTo("Junior") + Assertions.assertThat(instance.prop34).isEqualTo("White") + } + + data class Contact(val firstname: String, val lastname: String) + + data class ContactWithDefaulting(val prop0: String, val prop1: String = "White", val prop2: String, + val prop3: String = "White", val prop4: String = "White", val prop5: String = "White", + val prop6: String = "White", val prop7: String = "White", val prop8: String = "White", + val prop9: String = "White", val prop10: String = "White", val prop11: String = "White", + val prop12: String = "White", val prop13: String = "White", val prop14: String = "White", + val prop15: String = "White", val prop16: String = "White", val prop17: String = "White", + val prop18: String = "White", val prop19: String = "White", val prop20: String = "White", + val prop21: String = "White", val prop22: String = "White", val prop23: String = "White", + val prop24: String = "White", val prop25: String = "White", val prop26: String = "White", + val prop27: String = "White", val prop28: String = "White", val prop29: String = "White", + val prop30: String = "White", val prop31: String = "White", val prop32: String = "White", + val prop33: String, val prop34: String = "White" + ) +} + diff --git a/src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt new file mode 100644 index 0000000000..610660b32c --- /dev/null +++ b/src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt @@ -0,0 +1,77 @@ +/* + * Copyright 2017 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 com.nhaarman.mockito_kotlin.any +import com.nhaarman.mockito_kotlin.doReturn +import com.nhaarman.mockito_kotlin.whenever +import org.assertj.core.api.Assertions +import org.junit.Test +import org.junit.runner.RunWith +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.ParameterValueProvider +import org.springframework.data.mapping.model.PreferredConstructorDiscoverer + +/** + * Unit tests for [ReflectionEntityInstantiator] creating instances using Kotlin data classes. + + * @author Mark Paluch + */ +@RunWith(MockitoJUnitRunner::class) +@Suppress("UNCHECKED_CAST") +class ReflectionEntityInstantiatorDataClassUnitTests { + + @Mock lateinit var entity: PersistentEntity<*, *> + @Mock lateinit var provider: ParameterValueProvider + + @Test // DATACMNS-1126 + fun `should create instance`() { + + val entity = this.entity as PersistentEntity + val constructor = PreferredConstructorDiscoverer(Contact::class.java).constructor + + doReturn("Walter", "White").`when`(provider).getParameterValue(any()) + doReturn(constructor).whenever(entity).persistenceConstructor + + val instance: Contact = ReflectionEntityInstantiator.INSTANCE.createInstance(entity, provider) + + Assertions.assertThat(instance.firstname).isEqualTo("Walter") + Assertions.assertThat(instance.lastname).isEqualTo("White") + } + + @Test // DATACMNS-1126 + fun `should create instance and fill in defaults`() { + + val entity = this.entity as PersistentEntity + val constructor = PreferredConstructorDiscoverer(ContactWithDefaulting::class.java).constructor + + doReturn("Walter", null).`when`(provider).getParameterValue(any()) + doReturn(constructor).whenever(entity).persistenceConstructor + + val instance: ContactWithDefaulting = ReflectionEntityInstantiator.INSTANCE.createInstance(entity, provider) + + Assertions.assertThat(instance.firstname).isEqualTo("Walter") + Assertions.assertThat(instance.lastname).isEqualTo("White") + } + + data class Contact(val firstname: String, val lastname: String) + + data class ContactWithDefaulting(val firstname: String, val lastname: String = "White") +} + diff --git a/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt new file mode 100644 index 0000000000..b50d0054bb --- /dev/null +++ b/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt @@ -0,0 +1,90 @@ +/* + * Copyright 2017 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 + +import org.assertj.core.api.Assertions +import org.junit.Test +import org.springframework.data.annotation.PersistenceConstructor +import org.springframework.data.mapping.model.AbstractPersistentPropertyUnitTests.* + +/** + * Unit tests for [PreferredConstructorDiscoverer]. + * + * @author Mark Paluch + */ +class PreferredConstructorDiscovererUnitTests { + + @Test // DATACMNS-1126 + fun `should discover simple constructor`() { + + val constructor = PreferredConstructorDiscoverer(Simple::class.java).constructor + + Assertions.assertThat(constructor.parameters.size).isEqualTo(1) + } + + @Test // DATACMNS-1126 + fun `should reject two constructors`() { + + val constructor = PreferredConstructorDiscoverer(TwoConstructors::class.java).constructor + + Assertions.assertThat(constructor.parameters.size).isEqualTo(1) + } + + @Test // DATACMNS-1126 + fun `should discover annotated constructor`() { + + val constructor = PreferredConstructorDiscoverer(AnnotatedConstructors::class.java).constructor + + Assertions.assertThat(constructor.parameters.size).isEqualTo(2) + } + + @Test // DATACMNS-1126 + fun `should discover default constructor`() { + + val constructor = PreferredConstructorDiscoverer(DefaultConstructor::class.java).constructor + + Assertions.assertThat(constructor.parameters.size).isEqualTo(1) + } + + @Test // DATACMNS-1126 + fun `should discover default annotated constructor`() { + + val constructor = PreferredConstructorDiscoverer(TwoDefaultConstructorsAnnotated::class.java).constructor + + Assertions.assertThat(constructor.parameters.size).isEqualTo(3) + } + + data class Simple(val firstname: String) + + class TwoConstructors(val firstname: String) { + constructor(firstname: String, lastname: String) : this(firstname) + } + + class AnnotatedConstructors(val firstname: String) { + + @PersistenceConstructor + constructor(firstname: String, lastname: String) : this(firstname) + } + + class DefaultConstructor(val firstname: String = "foo") { + } + + class TwoDefaultConstructorsAnnotated(val firstname: String = "foo", val lastname: String = "bar") { + + @PersistenceConstructor + constructor(firstname: String = "foo", lastname: String = "bar", age: Int) : this(firstname, lastname) + } +} \ No newline at end of file From 592f42d4b7ac8d8971c005516783f2098f89724f Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 27 Jul 2017 14:06:16 +0200 Subject: [PATCH 3/4] DATACMNS-1126 - Address review feedback for constructor discovery. Split preferred constructor discovery in enum types. Refactor PreferredConstructorDiscoverer to interface with static methods for constructor discovery. Adapt tests. --- .../mapping/model/BasicPersistentEntity.java | 28 +-- .../model/PreferredConstructorDiscoverer.java | 149 ++------------ .../PreferredConstructorDiscoverers.java | 183 ++++++++++++++++++ .../data/repository/query/ReturnedType.java | 4 +- ...GeneratingEntityInstantiatorUnitTests.java | 36 ++-- ...ReflectionEntityInstantiatorUnitTests.java | 6 +- ...eferredConstructorDiscovererUnitTests.java | 47 ++--- ...ingEntityInstantiatorDataClassUnitTests.kt | 6 +- ...ionEntityInstantiatorDataClassUnitTests.kt | 6 +- ...PreferredConstructorDiscovererUnitTests.kt | 10 +- 10 files changed, 251 insertions(+), 224 deletions(-) create mode 100644 src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverers.java diff --git a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java index d0d828eede..1448945287 100644 --- a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java @@ -20,34 +20,12 @@ import java.io.Serializable; import java.lang.annotation.Annotation; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.TreeSet; +import java.util.*; import java.util.stream.Collectors; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.data.annotation.TypeAlias; -import org.springframework.data.mapping.Alias; -import org.springframework.data.mapping.Association; -import org.springframework.data.mapping.AssociationHandler; -import org.springframework.data.mapping.IdentifierAccessor; -import org.springframework.data.mapping.MappingException; -import org.springframework.data.mapping.PersistentEntity; -import org.springframework.data.mapping.PersistentProperty; -import org.springframework.data.mapping.PersistentPropertyAccessor; -import org.springframework.data.mapping.PreferredConstructor; -import org.springframework.data.mapping.PropertyHandler; -import org.springframework.data.mapping.SimpleAssociationHandler; -import org.springframework.data.mapping.SimplePropertyHandler; -import org.springframework.data.mapping.TargetAwareIdentifierAccessor; +import org.springframework.data.mapping.*; import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; @@ -112,7 +90,7 @@ public BasicPersistentEntity(TypeInformation information, @Nullable Comparato this.properties = new ArrayList<>(); this.persistentPropertiesCache = new ArrayList<>(); this.comparator = comparator; - this.constructor = new PreferredConstructorDiscoverer<>(this).getConstructor(); + this.constructor = PreferredConstructorDiscoverer.discover(this); this.associations = comparator == null ? new HashSet<>() : new TreeSet<>(new AssociationComparator<>(comparator)); this.propertyCache = new HashMap<>(); diff --git a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java index 17efcb4faf..a71cac255a 100644 --- a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java +++ b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverer.java @@ -15,26 +15,14 @@ */ package org.springframework.data.mapping.model; -import kotlin.jvm.JvmClassMappingKt; -import kotlin.reflect.KFunction; -import kotlin.reflect.full.KClasses; -import kotlin.reflect.jvm.ReflectJvmMapping; - -import java.lang.annotation.Annotation; -import java.lang.reflect.Constructor; -import java.util.List; - -import org.springframework.core.DefaultParameterNameDiscoverer; -import org.springframework.core.ParameterNameDiscoverer; 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.util.ClassTypeInformation; -import org.springframework.data.util.ReflectionUtils; -import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; +import com.mysema.commons.lang.Assert; + /** * Helper class to find a {@link PreferredConstructor}. * @@ -43,135 +31,34 @@ * @author Roman Rodov * @author Mark Paluch */ -public class PreferredConstructorDiscoverer> { - - private final ParameterNameDiscoverer discoverer = new DefaultParameterNameDiscoverer(); - - private @Nullable PreferredConstructor constructor; - - /** - * Creates a new {@link PreferredConstructorDiscoverer} for the given type. - * - * @param type must not be {@literal null}. - */ - public PreferredConstructorDiscoverer(Class type) { - this(ClassTypeInformation.from(type), null); - } +public interface PreferredConstructorDiscoverer> { /** - * Creates a new {@link PreferredConstructorDiscoverer} for the given {@link PersistentEntity}. - * - * @param entity must not be {@literal null}. - */ - public PreferredConstructorDiscoverer(PersistentEntity entity) { - this(entity.getTypeInformation(), entity); - } - - /** - * Creates a new {@link PreferredConstructorDiscoverer} for the given type. + * Discovers the {@link PreferredConstructor} for the given type. * * @param type must not be {@literal null}. - * @param entity + * @return the {@link PreferredConstructor} if found or {@literal null}. */ - protected PreferredConstructorDiscoverer(TypeInformation type, @Nullable PersistentEntity entity) { - - boolean noArgConstructorFound = false; - int numberOfArgConstructors = 0; - Class rawOwningType = type.getType(); - - if (ReflectionUtils.isKotlinClass(type.getType())) { - - for (Constructor candidate : rawOwningType.getDeclaredConstructors()) { - - PreferredConstructor preferredConstructor = buildPreferredConstructor(candidate, type, entity); - - // Synthetic constructors should not be considered - if (preferredConstructor.getConstructor().isSynthetic()) { - continue; - } - - // Explicitly defined constructor trumps all - if (preferredConstructor.isExplicitlyAnnotated()) { - this.constructor = preferredConstructor; - return; - } - } - - KFunction primaryConstructor = KClasses - .getPrimaryConstructor(JvmClassMappingKt.getKotlinClass(type.getType())); - Constructor javaConstructor = ReflectJvmMapping.getJavaConstructor(primaryConstructor); - if (javaConstructor != null) { - this.constructor = buildPreferredConstructor(javaConstructor, type, entity); - return; - } - } - - for (Constructor candidate : rawOwningType.getDeclaredConstructors()) { - - PreferredConstructor preferredConstructor = buildPreferredConstructor(candidate, type, entity); - - // Synthetic constructors should not be considered - if (preferredConstructor.getConstructor().isSynthetic()) { - continue; - } - - // Explicitly defined constructor trumps all - if (preferredConstructor.isExplicitlyAnnotated()) { - this.constructor = preferredConstructor; - return; - } - - // No-arg constructor trumps custom ones - if (this.constructor == null || preferredConstructor.isNoArgConstructor()) { - this.constructor = preferredConstructor; - } - - if (preferredConstructor.isNoArgConstructor()) { - noArgConstructorFound = true; - } else { - numberOfArgConstructors++; - } - } - - if (!noArgConstructorFound && numberOfArgConstructors > 1) { - this.constructor = null; - } - } - - @SuppressWarnings({ "unchecked", "rawtypes" }) - private PreferredConstructor buildPreferredConstructor(Constructor constructor, - TypeInformation typeInformation, @Nullable PersistentEntity entity) { - - List> parameterTypes = typeInformation.getParameterTypes(constructor); - - if (parameterTypes.isEmpty()) { - return new PreferredConstructor<>((Constructor) constructor); - } - - String[] parameterNames = discoverer.getParameterNames(constructor); - - Parameter[] parameters = new Parameter[parameterTypes.size()]; - Annotation[][] parameterAnnotations = constructor.getParameterAnnotations(); - - for (int i = 0; i < parameterTypes.size(); i++) { - - String name = parameterNames == null ? null : parameterNames[i]; - TypeInformation type = parameterTypes.get(i); - Annotation[] annotations = parameterAnnotations[i]; + @Nullable + static > PreferredConstructor discover(Class type) { - parameters[i] = new Parameter(name, type, annotations, entity); - } + Assert.notNull(type, "Type must not be null!"); - return new PreferredConstructor<>((Constructor) constructor, parameters); + return PreferredConstructorDiscoverers.findDiscoverer(type).discover(ClassTypeInformation.from(type), null); } /** - * Returns the discovered {@link PreferredConstructor}. + * Discovers the {@link PreferredConstructorDiscoverer} for the given {@link PersistentEntity}. * - * @return + * @param entity must not be {@literal null}. + * @return the {@link PreferredConstructor} if found or {@literal null}. */ @Nullable - public PreferredConstructor getConstructor() { - return constructor; + static > PreferredConstructor discover(PersistentEntity entity) { + + Assert.notNull(entity, "PersistentEntity must not be null!"); + + return PreferredConstructorDiscoverers.findDiscoverer(entity.getType()).discover(entity.getTypeInformation(), + entity); } } diff --git a/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverers.java b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverers.java new file mode 100644 index 0000000000..9fbafb58d1 --- /dev/null +++ b/src/main/java/org/springframework/data/mapping/model/PreferredConstructorDiscoverers.java @@ -0,0 +1,183 @@ +/* + * Copyright 2011-2017 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; + +import kotlin.jvm.JvmClassMappingKt; +import kotlin.reflect.KFunction; +import kotlin.reflect.full.KClasses; +import kotlin.reflect.jvm.ReflectJvmMapping; + +import java.lang.annotation.Annotation; +import java.lang.reflect.Constructor; +import java.util.Arrays; +import java.util.List; + +import org.springframework.core.DefaultParameterNameDiscoverer; +import org.springframework.core.ParameterNameDiscoverer; +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.util.ReflectionUtils; +import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; + +/** + * Helper class to find a {@link PreferredConstructor}. + * + * @author Oliver Gierke + * @author Christoph Strobl + * @author Roman Rodov + * @author Mark Paluch + * @since 2.0 + */ +enum PreferredConstructorDiscoverers { + + /** + * Discovers a {@link PreferredConstructor} for Java types. + */ + DEFAULT { + + /* + * (non-Javadoc) + * @see org.springframework.data.mapping.model.PreferredConstructorDiscoverers#discover(org.springframework.data.util.TypeInformation, org.springframework.data.mapping.PersistentEntity) + */ + @Nullable + @Override + public > PreferredConstructor discover(TypeInformation type, + @Nullable PersistentEntity entity) { + + boolean noArgConstructorFound = false; + int numberOfArgConstructors = 0; + Class rawOwningType = type.getType(); + PreferredConstructor constructor = null; + + for (Constructor candidate : rawOwningType.getDeclaredConstructors()) { + + PreferredConstructor preferredConstructor = buildPreferredConstructor(candidate, type, entity); + + // Synthetic constructors should not be considered + if (preferredConstructor.getConstructor().isSynthetic()) { + continue; + } + + // Explicitly defined constructor trumps all + if (preferredConstructor.isExplicitlyAnnotated()) { + return preferredConstructor; + } + + // No-arg constructor trumps custom ones + if (constructor == null || preferredConstructor.isNoArgConstructor()) { + constructor = preferredConstructor; + } + + if (preferredConstructor.isNoArgConstructor()) { + noArgConstructorFound = true; + } else { + numberOfArgConstructors++; + } + } + + if (!noArgConstructorFound && numberOfArgConstructors > 1) { + constructor = null; + } + + return constructor; + } + }, + + /** + * Discovers a {@link PreferredConstructor} for Kotlin types. + */ + KOTLIN { + + /* + * (non-Javadoc) + * @see org.springframework.data.mapping.model.PreferredConstructorDiscoverers#discover(org.springframework.data.util.TypeInformation, org.springframework.data.mapping.PersistentEntity) + */ + @Nullable + @Override + public > PreferredConstructor discover(TypeInformation type, + @Nullable PersistentEntity entity) { + + Class rawOwningType = type.getType(); + + return Arrays.stream(rawOwningType.getDeclaredConstructors()) // + .map(it -> buildPreferredConstructor(it, type, entity)) // + .filter(it -> !it.getConstructor().isSynthetic()) // Synthetic constructors should not be considered + .filter(PreferredConstructor::isExplicitlyAnnotated) // Explicitly defined constructor trumps all + .findFirst() // + .orElseGet(() -> { + + KFunction primaryConstructor = KClasses + .getPrimaryConstructor(JvmClassMappingKt.getKotlinClass(type.getType())); + Constructor javaConstructor = ReflectJvmMapping.getJavaConstructor(primaryConstructor); + + return javaConstructor != null ? buildPreferredConstructor(javaConstructor, type, entity) : null; + }); + } + }; + + private static final ParameterNameDiscoverer discoverer = new DefaultParameterNameDiscoverer(); + + /** + * Find the appropriate discoverer for {@code type}. + * + * @param type must not be {@literal null}. + * @return the appropriate discoverer for {@code type}. + */ + public static PreferredConstructorDiscoverers findDiscoverer(Class type) { + return ReflectionUtils.isKotlinClass(type) ? KOTLIN : DEFAULT; + } + + /** + * Discovers a constructor for the given type. + * + * @param type must not be {@literal null}. + * @param entity may be {@literal null}. + * @return the {@link PreferredConstructor} if found or {@literal null}. + */ + @Nullable + public abstract > PreferredConstructor discover(TypeInformation type, + @Nullable PersistentEntity entity); + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private static > PreferredConstructor buildPreferredConstructor( + Constructor constructor, TypeInformation typeInformation, @Nullable PersistentEntity entity) { + + List> parameterTypes = typeInformation.getParameterTypes(constructor); + + if (parameterTypes.isEmpty()) { + return new PreferredConstructor<>((Constructor) constructor); + } + + String[] parameterNames = discoverer.getParameterNames(constructor); + + Parameter[] parameters = new Parameter[parameterTypes.size()]; + Annotation[][] parameterAnnotations = constructor.getParameterAnnotations(); + + for (int i = 0; i < parameterTypes.size(); i++) { + + String name = parameterNames == null ? null : parameterNames[i]; + TypeInformation type = parameterTypes.get(i); + Annotation[] annotations = parameterAnnotations[i]; + + parameters[i] = new Parameter(name, type, annotations, entity); + } + + return new PreferredConstructor<>((Constructor) constructor, parameters); + } +} diff --git a/src/main/java/org/springframework/data/repository/query/ReturnedType.java b/src/main/java/org/springframework/data/repository/query/ReturnedType.java index aae5ac89b2..7921ec18b2 100644 --- a/src/main/java/org/springframework/data/repository/query/ReturnedType.java +++ b/src/main/java/org/springframework/data/repository/query/ReturnedType.java @@ -45,6 +45,7 @@ * * @author Oliver Gierke * @author Christoph Strobl + * @author Mark Paluch * @since 1.12 */ @RequiredArgsConstructor(access = AccessLevel.PRIVATE) @@ -296,8 +297,7 @@ private List detectConstructorParameterNames(Class type) { return Collections.emptyList(); } - PreferredConstructorDiscoverer discoverer = new PreferredConstructorDiscoverer(type); - PreferredConstructor constructor = discoverer.getConstructor(); + PreferredConstructor constructor = PreferredConstructorDiscoverer.discover(type); if (constructor == null) { return Collections.emptyList(); diff --git a/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java b/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java index 1b63ab5a30..5a578b875c 100755 --- a/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java +++ b/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java @@ -53,8 +53,6 @@ public class ClassGeneratingEntityInstantiatorUnitTests

entity; @Mock ParameterValueProvider

provider; - @Mock PreferredConstructor constructor; - @Mock Parameter parameter; @Test public void instantiatesSimpleObjectCorrectly() { @@ -72,11 +70,10 @@ public void instantiatesArrayCorrectly() { this.instance.createInstance(entity, provider); } - @Test + @Test // DATACMNS-1126 public void instantiatesTypeWithPreferredConstructorUsingParameterValueProvider() { - PreferredConstructor constructor = new PreferredConstructorDiscoverer(Foo.class) - .getConstructor(); + PreferredConstructor constructor = PreferredConstructorDiscoverer.discover(Foo.class); doReturn(Foo.class).when(entity).getType(); doReturn(constructor).when(entity).getPersistenceConstructor(); @@ -150,23 +147,21 @@ public void capturesContextOnInstantiationException() throws Exception { } } - @Test // DATACMNS-578 - @SuppressWarnings({ "unchecked", "rawtypes" }) + @Test // DATACMNS-578, DATACMNS-1126 public void instantiateObjCtorDefault() { doReturn(ObjCtorDefault.class).when(entity).getType(); - doReturn(new PreferredConstructorDiscoverer<>(ObjCtorDefault.class).getConstructor())// + doReturn(PreferredConstructorDiscoverer.discover(ObjCtorDefault.class))// .when(entity).getPersistenceConstructor(); IntStream.range(0, 2).forEach(i -> assertThat(this.instance.createInstance(entity, provider)).isInstanceOf(ObjCtorDefault.class)); } - @Test // DATACMNS-578 - @SuppressWarnings({ "unchecked", "rawtypes" }) + @Test // DATACMNS-578, DATACMNS-1126 public void instantiateObjCtorNoArgs() { doReturn(ObjCtorNoArgs.class).when(entity).getType(); - doReturn(new PreferredConstructorDiscoverer<>(ObjCtorNoArgs.class).getConstructor())// + doReturn(PreferredConstructorDiscoverer.discover(ObjCtorNoArgs.class))// .when(entity).getPersistenceConstructor(); IntStream.range(0, 2).forEach(i -> { @@ -178,12 +173,11 @@ public void instantiateObjCtorNoArgs() { }); } - @Test // DATACMNS-578 - @SuppressWarnings("unchecked") + @Test // DATACMNS-578, DATACMNS-1126 public void instantiateObjCtor1ParamString() { doReturn(ObjCtor1ParamString.class).when(entity).getType(); - doReturn(new PreferredConstructorDiscoverer<>(ObjCtor1ParamString.class).getConstructor())// + doReturn(PreferredConstructorDiscoverer.discover(ObjCtor1ParamString.class))// .when(entity).getPersistenceConstructor(); doReturn("FOO").when(provider).getParameterValue(any()); @@ -197,12 +191,11 @@ public void instantiateObjCtor1ParamString() { }); } - @Test // DATACMNS-578 - @SuppressWarnings("unchecked") + @Test // DATACMNS-578, DATACMNS-1126 public void instantiateObjCtor2ParamStringString() { doReturn(ObjCtor2ParamStringString.class).when(entity).getType(); - doReturn(new PreferredConstructorDiscoverer<>(ObjCtor2ParamStringString.class).getConstructor())// + doReturn(PreferredConstructorDiscoverer.discover(ObjCtor2ParamStringString.class))// .when(entity).getPersistenceConstructor(); IntStream.range(0, 2).forEach(i -> { @@ -218,12 +211,11 @@ public void instantiateObjCtor2ParamStringString() { }); } - @Test // DATACMNS-578 - @SuppressWarnings("unchecked") + @Test // DATACMNS-578, DATACMNS-1126 public void instantiateObjectCtor1ParamInt() { doReturn(ObjectCtor1ParamInt.class).when(entity).getType(); - doReturn(new PreferredConstructorDiscoverer<>(ObjectCtor1ParamInt.class).getConstructor())// + doReturn(PreferredConstructorDiscoverer.discover(ObjectCtor1ParamInt.class))// .when(entity).getPersistenceConstructor(); IntStream.range(0, 2).forEach(i -> { @@ -237,12 +229,12 @@ public void instantiateObjectCtor1ParamInt() { }); } - @Test // DATACMNS-578 + @Test // DATACMNS-578, DATACMNS-1126 @SuppressWarnings("unchecked") public void instantiateObjectCtor7ParamsString5IntsString() { doReturn(ObjectCtor7ParamsString5IntsString.class).when(entity).getType(); - doReturn(new PreferredConstructorDiscoverer<>(ObjectCtor7ParamsString5IntsString.class).getConstructor())// + doReturn(PreferredConstructorDiscoverer.discover(ObjectCtor7ParamsString5IntsString.class))// .when(entity).getPersistenceConstructor(); IntStream.range(0, 2).forEach(i -> { diff --git a/src/test/java/org/springframework/data/convert/ReflectionEntityInstantiatorUnitTests.java b/src/test/java/org/springframework/data/convert/ReflectionEntityInstantiatorUnitTests.java index 1fade6e3bf..355e3f1780 100755 --- a/src/test/java/org/springframework/data/convert/ReflectionEntityInstantiatorUnitTests.java +++ b/src/test/java/org/springframework/data/convert/ReflectionEntityInstantiatorUnitTests.java @@ -51,8 +51,6 @@ public class ReflectionEntityInstantiatorUnitTests

entity; @Mock ParameterValueProvider

provider; - @Mock PreferredConstructor constructor; - @Mock Parameter parameter; @Test public void instantiatesSimpleObjectCorrectly() { @@ -68,10 +66,10 @@ public void instantiatesArrayCorrectly() { INSTANCE.createInstance(entity, provider); } - @Test + @Test // DATACMNS-1126 public void instantiatesTypeWithPreferredConstructorUsingParameterValueProvider() { - PreferredConstructor constructor = new PreferredConstructorDiscoverer(Foo.class).getConstructor(); + PreferredConstructor constructor = PreferredConstructorDiscoverer.discover(Foo.class); doReturn(constructor).when(entity).getPersistenceConstructor(); diff --git a/src/test/java/org/springframework/data/mapping/PreferredConstructorDiscovererUnitTests.java b/src/test/java/org/springframework/data/mapping/PreferredConstructorDiscovererUnitTests.java index 58a003437d..40f1ed3249 100755 --- a/src/test/java/org/springframework/data/mapping/PreferredConstructorDiscovererUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/PreferredConstructorDiscovererUnitTests.java @@ -36,13 +36,10 @@ */ public class PreferredConstructorDiscovererUnitTests

> { - @Test + @Test // DATACMNS-1126 public void findsNoArgConstructorForClassWithoutExplicitConstructor() { - PreferredConstructorDiscoverer discoverer = new PreferredConstructorDiscoverer<>( - EntityWithoutConstructor.class); - - assertThat(discoverer.getConstructor()).satisfies(constructor -> { + assertThat(PreferredConstructorDiscoverer.discover(EntityWithoutConstructor.class)).satisfies(constructor -> { assertThat(constructor).isNotNull(); assertThat(constructor.isNoArgConstructor()).isTrue(); @@ -50,13 +47,10 @@ public void findsNoArgConstructorForClassWithoutExplicitConstructor() { }); } - @Test + @Test // DATACMNS-1126 public void findsNoArgConstructorForClassWithMultipleConstructorsAndNoArgOne() { - PreferredConstructorDiscoverer discoverer = new PreferredConstructorDiscoverer<>( - ClassWithEmptyConstructor.class); - - assertThat(discoverer.getConstructor()).satisfies(constructor -> { + assertThat(PreferredConstructorDiscoverer.discover(ClassWithEmptyConstructor.class)).satisfies(constructor -> { assertThat(constructor).isNotNull(); assertThat(constructor.isNoArgConstructor()).isTrue(); @@ -64,22 +58,19 @@ public void findsNoArgConstructorForClassWithMultipleConstructorsAndNoArgOne() { }); } - @Test + @Test // DATACMNS-1126 public void doesNotThrowExceptionForMultipleConstructorsAndNoNoArgConstructorWithoutAnnotation() { - PreferredConstructorDiscoverer discoverer = new PreferredConstructorDiscoverer<>( - ClassWithMultipleConstructorsWithoutEmptyOne.class); - - assertThat(discoverer.getConstructor()).isNull(); + assertThat(PreferredConstructorDiscoverer.discover(ClassWithMultipleConstructorsWithoutEmptyOne.class)).isNull(); } - @Test + @Test // DATACMNS-1126 + @SuppressWarnings({ "unchecked", "rawtypes" }) public void usesConstructorWithAnnotationOverEveryOther() { - PreferredConstructorDiscoverer discoverer = new PreferredConstructorDiscoverer<>( - ClassWithMultipleConstructorsAndAnnotation.class); - assertThat(discoverer.getConstructor()).satisfies(constructor -> { + assertThat(PreferredConstructorDiscoverer.discover(ClassWithMultipleConstructorsAndAnnotation.class)) + .satisfies(constructor -> { assertThat(constructor).isNotNull(); assertThat(constructor.isNoArgConstructor()).isFalse(); @@ -87,7 +78,7 @@ public void usesConstructorWithAnnotationOverEveryOther() { assertThat(constructor.hasParameters()).isTrue(); - Iterator> parameters = constructor.getParameters().iterator(); + Iterator> parameters = (Iterator) constructor.getParameters().iterator(); Parameter parameter = parameters.next(); assertThat(parameter.getType().getType()).isEqualTo(Long.class); @@ -95,26 +86,25 @@ public void usesConstructorWithAnnotationOverEveryOther() { }); } - @Test // DATACMNS-134 + @Test // DATACMNS-134, DATACMNS-1126 public void discoversInnerClassConstructorCorrectly() { PersistentEntity entity = new BasicPersistentEntity<>(ClassTypeInformation.from(Inner.class)); - PreferredConstructorDiscoverer discoverer = new PreferredConstructorDiscoverer<>(entity); - assertThat(discoverer.getConstructor()).satisfies(constructor -> { + assertThat(PreferredConstructorDiscoverer.discover(entity)).satisfies(constructor -> { Parameter parameter = constructor.getParameters().iterator().next(); assertThat(constructor.isEnclosingClassParameter(parameter)).isTrue(); }); } - @Test // DATACMNS-1082 + @Test // DATACMNS-1082, DATACMNS-1126 public void skipsSyntheticConstructor() { - PersistentEntity entity = new BasicPersistentEntity<>(ClassTypeInformation.from(SyntheticConstructor.class)); - PreferredConstructorDiscoverer discoverer = new PreferredConstructorDiscoverer<>(entity); + PersistentEntity entity = new BasicPersistentEntity<>( + ClassTypeInformation.from(SyntheticConstructor.class)); - assertThat(discoverer.getConstructor()).satisfies(constructor -> { + assertThat(PreferredConstructorDiscoverer.discover(entity)).satisfies(constructor -> { PersistenceConstructor annotation = constructor.getConstructor().getAnnotation(PersistenceConstructor.class); assertThat(annotation).isNotNull(); @@ -124,8 +114,7 @@ public void skipsSyntheticConstructor() { static class SyntheticConstructor { @PersistenceConstructor - private SyntheticConstructor(String x) { - } + private SyntheticConstructor(String x) {} class InnerSynthetic { // Compiler will generate a synthetic constructor since diff --git a/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt index f7d5f5a752..0d36a165e9 100644 --- a/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt @@ -44,7 +44,7 @@ class ClassGeneratingEntityInstantiatorDataClassUnitTests { fun `should create instance`() { val entity = this.entity as PersistentEntity - val constructor = PreferredConstructorDiscoverer(Contact::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(Contact::class.java) doReturn("Walter", "White").`when`(provider).getParameterValue(any()) doReturn(constructor).whenever(entity).persistenceConstructor @@ -60,11 +60,11 @@ class ClassGeneratingEntityInstantiatorDataClassUnitTests { fun `should create instance and fill in defaults`() { val entity = this.entity as PersistentEntity - val constructor = PreferredConstructorDiscoverer(ContactWithDefaulting::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(ContactWithDefaulting::class.java) doReturn("Walter", null, "Skyler", null, null, null, null, null, null, null, /* 0-9 */ null, null, null, null, null, null, null, null, null, null, /* 10-19 */ - null, null, null, null, null, null, null, null, null, null, /* 20 - 29 */ + null, null, null, null, null, null, null, null, null, null, /* 20-29 */ null, "Walter", null, "Junior", null).`when`(provider).getParameterValue(any()) doReturn(constructor).whenever(entity).persistenceConstructor doReturn(constructor.constructor.declaringClass).whenever(entity).type diff --git a/src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt index 610660b32c..ce6d06b574 100644 --- a/src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/convert/ReflectionEntityInstantiatorDataClassUnitTests.kt @@ -30,7 +30,7 @@ import org.springframework.data.mapping.model.PreferredConstructorDiscoverer /** * Unit tests for [ReflectionEntityInstantiator] creating instances using Kotlin data classes. - + * * @author Mark Paluch */ @RunWith(MockitoJUnitRunner::class) @@ -44,7 +44,7 @@ class ReflectionEntityInstantiatorDataClassUnitTests { fun `should create instance`() { val entity = this.entity as PersistentEntity - val constructor = PreferredConstructorDiscoverer(Contact::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(Contact::class.java) doReturn("Walter", "White").`when`(provider).getParameterValue(any()) doReturn(constructor).whenever(entity).persistenceConstructor @@ -59,7 +59,7 @@ class ReflectionEntityInstantiatorDataClassUnitTests { fun `should create instance and fill in defaults`() { val entity = this.entity as PersistentEntity - val constructor = PreferredConstructorDiscoverer(ContactWithDefaulting::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(ContactWithDefaulting::class.java) doReturn("Walter", null).`when`(provider).getParameterValue(any()) doReturn(constructor).whenever(entity).persistenceConstructor diff --git a/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt b/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt index b50d0054bb..a21c83ddd8 100644 --- a/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/mapping/model/PreferredConstructorDiscovererUnitTests.kt @@ -30,7 +30,7 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should discover simple constructor`() { - val constructor = PreferredConstructorDiscoverer(Simple::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(Simple::class.java) Assertions.assertThat(constructor.parameters.size).isEqualTo(1) } @@ -38,7 +38,7 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should reject two constructors`() { - val constructor = PreferredConstructorDiscoverer(TwoConstructors::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(TwoConstructors::class.java) Assertions.assertThat(constructor.parameters.size).isEqualTo(1) } @@ -46,7 +46,7 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should discover annotated constructor`() { - val constructor = PreferredConstructorDiscoverer(AnnotatedConstructors::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(AnnotatedConstructors::class.java) Assertions.assertThat(constructor.parameters.size).isEqualTo(2) } @@ -54,7 +54,7 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should discover default constructor`() { - val constructor = PreferredConstructorDiscoverer(DefaultConstructor::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(DefaultConstructor::class.java) Assertions.assertThat(constructor.parameters.size).isEqualTo(1) } @@ -62,7 +62,7 @@ class PreferredConstructorDiscovererUnitTests { @Test // DATACMNS-1126 fun `should discover default annotated constructor`() { - val constructor = PreferredConstructorDiscoverer(TwoDefaultConstructorsAnnotated::class.java).constructor + val constructor = PreferredConstructorDiscoverer.discover(TwoDefaultConstructorsAnnotated::class.java) Assertions.assertThat(constructor.parameters.size).isEqualTo(3) } From e46ec1b658963adad8d59657acaf2e739583cef2 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 27 Jul 2017 14:34:16 +0200 Subject: [PATCH 4/4] DATACMNS-1126 - Address review feedback for EntityInstantiator. Refactor ClassGeneratingEntityInstantiator to ClassGeneratingEntityInstantiator without Kotlin-specifics and ClassGeneratingKotlinEntityInstantiator extending ClassGeneratingEntityInstantiator. Apply intantiation argument caching to ClassGeneratingEntityInstantiator. --- .../ClassGeneratingEntityInstantiator.java | 253 ++++-------------- ...assGeneratingKotlinEntityInstantiator.java | 238 ++++++++++++++++ .../data/convert/EntityInstantiators.java | 13 +- ...atingKotlinEntityInstantiatorUnitTests.kt} | 8 +- 4 files changed, 294 insertions(+), 218 deletions(-) create mode 100644 src/main/java/org/springframework/data/convert/ClassGeneratingKotlinEntityInstantiator.java rename src/test/kotlin/org/springframework/data/convert/{ClassGeneratingEntityInstantiatorDataClassUnitTests.kt => ClassGeneratingKotlinEntityInstantiatorUnitTests.kt} (92%) diff --git a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java index ef738e7c7f..59073f2a62 100644 --- a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java @@ -17,20 +17,13 @@ import static org.springframework.asm.Opcodes.*; -import kotlin.reflect.KFunction; -import kotlin.reflect.KParameter; -import kotlin.reflect.jvm.ReflectJvmMapping; - import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; import java.security.AccessController; import java.security.PrivilegedAction; -import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.stream.IntStream; import org.springframework.asm.ClassWriter; import org.springframework.asm.MethodVisitor; @@ -42,7 +35,6 @@ 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.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -53,8 +45,6 @@ * {@link PersistentEntity}'s {@link PreferredConstructor} to instantiate an instance of the entity by dynamically * generating factory methods with appropriate constructor invocations via ASM. If we cannot generate byte code for a * type, we gracefully fall-back to the {@link ReflectionEntityInstantiator}. - *

- * Adopts to Kotlin constructors using parameter defaulting. * * @author Thomas Darimont * @author Oliver Gierke @@ -141,24 +131,20 @@ private EntityInstantiator createEntityInstantiator(PersistentEntity entit } try { - - if (ReflectionUtils.isKotlinClass(entity.getType())) { - - PreferredConstructor defaultConstructor = new DefaultingKotlinConstructorResolver(entity) - .getDefaultConstructor(); - - if (defaultConstructor != null) { - return new DefaultingKotlinClassEntityInstantiator(createObjectInstantiator(entity, defaultConstructor), - entity.getPersistenceConstructor()); - } - } - - return new EntityInstantiatorAdapter(createObjectInstantiator(entity, entity.getPersistenceConstructor())); + return doCreateEntityInstantiator(entity); } catch (Throwable ex) { return ReflectionEntityInstantiator.INSTANCE; } } + /** + * @param entity + * @return + */ + protected EntityInstantiator doCreateEntityInstantiator(PersistentEntity entity) { + return new EntityInstantiatorAdapter(createObjectInstantiator(entity, entity.getPersistenceConstructor())); + } + /** * @param entity * @return @@ -183,6 +169,33 @@ private boolean shouldUseReflectionEntityInstantiator(PersistentEntity ent return false; } + /** + * Allocates an object array for instance creation. This method uses the argument array cache if possible. + * + * @param argumentCount + * @return + * @since 2.0 + * @see #ARG_CACHE_SIZE + */ + static Object[] allocateArguments(int argumentCount) { + return argumentCount < ARG_CACHE_SIZE ? OBJECT_POOL.get()[argumentCount] : new Object[argumentCount]; + } + + /** + * Deallocates an object array used for instance creation. Parameters are cleared if the array was cached. + * + * @param argumentCount + * @return + * @since 2.0 + * @see #ARG_CACHE_SIZE + */ + static void deallocateArguments(Object[] params) { + + if (params.length != 0 && params.length < ARG_CACHE_SIZE) { + Arrays.fill(params, null); + } + } + /** * Creates a dynamically generated {@link ObjectInstantiator} for the given {@link PersistentEntity} and * {@link PreferredConstructor}. There will always be exactly one {@link ObjectInstantiator} instance per @@ -192,7 +205,7 @@ private boolean shouldUseReflectionEntityInstantiator(PersistentEntity ent * @param constructor * @return */ - private ObjectInstantiator createObjectInstantiator(PersistentEntity entity, + ObjectInstantiator createObjectInstantiator(PersistentEntity entity, @Nullable PreferredConstructor constructor) { try { @@ -202,20 +215,15 @@ private ObjectInstantiator createObjectInstantiator(PersistentEntity entit } } - private static Object[] allocateArguments(int argumentCount) { - return argumentCount < ARG_CACHE_SIZE ? OBJECT_POOL.get()[argumentCount] : new Object[argumentCount]; - } - /** * Adapter to forward an invocation of the {@link EntityInstantiator} API to an {@link ObjectInstantiator}. * * @author Thomas Darimont * @author Oliver Gierke + * @author Mark Paluch */ private static class EntityInstantiatorAdapter implements EntityInstantiator { - private static final Object[] EMPTY_ARRAY = new Object[0]; - private final ObjectInstantiator instantiator; /** @@ -242,6 +250,8 @@ public , P extends PersistentPrope return (T) instantiator.newInstance(params); } catch (Exception e) { throw new MappingInstantiationException(entity, Arrays.asList(params), e); + } finally { + deallocateArguments(params); } } @@ -256,190 +266,17 @@ private

, T> Object[] extractInvocationArguments @Nullable PreferredConstructor constructor, ParameterValueProvider

provider) { if (constructor == null || !constructor.hasParameters()) { - return EMPTY_ARRAY; + return allocateArguments(0); } - List params = new ArrayList<>(constructor.getConstructor().getParameterCount()); + Object[] params = allocateArguments(constructor.getConstructor().getParameterCount()); + int index = 0; for (Parameter parameter : constructor.getParameters()) { - params.add(provider.getParameterValue(parameter)); - } - - return params.toArray(); - } - } - - /** - * Resolves a {@link PreferredConstructor} to a synthetic Kotlin constructor accepting the same user-space parameters - * suffixed by Kotlin-specifics required for defaulting and the {@code kotlin.jvm.internal.DefaultConstructorMarker}. - * - * @since 2.0 - * @author Mark Paluch - */ - static class DefaultingKotlinConstructorResolver { - - @Nullable private final PreferredConstructor defaultConstructor; - - @SuppressWarnings("unchecked") - DefaultingKotlinConstructorResolver(PersistentEntity entity) { - - Constructor hit = resolveDefaultConstructor(entity); - PreferredConstructor persistenceConstructor = entity.getPersistenceConstructor(); - - if (hit != null && persistenceConstructor != null) { - this.defaultConstructor = new PreferredConstructor<>(hit, - persistenceConstructor.getParameters().toArray(new Parameter[0])); - } else { - this.defaultConstructor = null; - } - } - - @Nullable - private static Constructor resolveDefaultConstructor(PersistentEntity entity) { - - if (entity.getPersistenceConstructor() == null) { - return null; - } - - Constructor hit = null; - Constructor constructor = entity.getPersistenceConstructor().getConstructor(); - - for (Constructor candidate : entity.getType().getDeclaredConstructors()) { - - // use only synthetic constructors - if (!candidate.isSynthetic()) { - continue; - } - - // with a parameter count greater zero - if (constructor.getParameterCount() == 0) { - continue; - } - - // candidates must contain at least two additional parameters (int, DefaultConstructorMarker) - if (constructor.getParameterCount() + 2 > candidate.getParameterCount()) { - continue; - } - - java.lang.reflect.Parameter[] constructorParameters = constructor.getParameters(); - java.lang.reflect.Parameter[] candidateParameters = candidate.getParameters(); - - if (!candidateParameters[candidateParameters.length - 1].getType().getName() - .equals("kotlin.jvm.internal.DefaultConstructorMarker")) { - continue; - } - - if (parametersMatch(constructorParameters, candidateParameters)) { - hit = candidate; - break; - } + params[index++] = provider.getParameterValue(parameter); } - return hit; - } - - private static boolean parametersMatch(java.lang.reflect.Parameter[] constructorParameters, - java.lang.reflect.Parameter[] candidateParameters) { - - return IntStream.range(0, constructorParameters.length) - .allMatch(i -> constructorParameters[i].getType().equals(candidateParameters[i].getType())); - } - - @Nullable - PreferredConstructor getDefaultConstructor() { - return defaultConstructor; - } - } - - /** - * Entity instantiator for Kotlin constructors that apply parameter defaulting. Kotlin constructors that apply - * argument defaulting are marked with {@link kotlin.jvm.internal.DefaultConstructorMarker} and accept additional - * parameters besides the regular (user-space) parameters. Additional parameters are: - *
    - *
  • defaulting bitmask ({@code int}), a bit mask slot for each 32 parameters
    • - *
  • {@code DefaultConstructorMarker} (usually null)
    • - *
- * Defaulting bitmask - *

- * The defaulting bitmask is a 32 bit integer representing which positional argument should be defaulted. Defaulted - * arguments are passed as {@literal null} and require the appropriate positional bit set ( {@code 1 << 2} for the 2. - * argument)). Since the bitmask represents only 32 bit states, it requires additional masks (slots) if more than 32 - * arguments are represented. - * - * @author Mark Paluch - * @since 2.0 - */ - static class DefaultingKotlinClassEntityInstantiator implements EntityInstantiator { - - private final ObjectInstantiator instantiator; - private final List kParameters; - private final Constructor synthetic; - - DefaultingKotlinClassEntityInstantiator(ObjectInstantiator instantiator, PreferredConstructor constructor) { - - KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(constructor.getConstructor()); - - if (kotlinConstructor == null) { - throw new IllegalArgumentException( - "No corresponding Kotlin constructor found for " + constructor.getConstructor()); - } - - this.instantiator = instantiator; - this.kParameters = kotlinConstructor.getParameters(); - this.synthetic = constructor.getConstructor(); - } - - /* - * (non-Javadoc) - * @see org.springframework.data.convert.EntityInstantiator#createInstance(org.springframework.data.mapping.PersistentEntity, org.springframework.data.mapping.model.ParameterValueProvider) - */ - @Override - @SuppressWarnings("unchecked") - public , P extends PersistentProperty

> T createInstance(E entity, - ParameterValueProvider

provider) { - - PreferredConstructor preferredConstructor = entity.getPersistenceConstructor(); - Assert.notNull(preferredConstructor, "PreferredConstructor must not be null!"); - - int[] defaulting = new int[(synthetic.getParameterCount() / 32) + 1]; - - Object[] params = allocateArguments( - synthetic.getParameterCount() + defaulting.length + /* DefaultConstructorMarker */1); - int userParameterCount = kParameters.size(); - - List> parameters = preferredConstructor.getParameters(); - - // Prepare user-space arguments - for (int i = 0; i < userParameterCount; i++) { - - int slot = i / 32; - int offset = slot * 32; - - Object param = provider.getParameterValue(parameters.get(i)); - - KParameter kParameter = kParameters.get(i); - - // what about null and parameter is mandatory? What if parameter is non-null? - if (kParameter.isOptional()) { - - if (param == null) { - defaulting[slot] = defaulting[slot] | (1 << (i - offset)); - } - } - - params[i] = param; - } - - // append nullability masks to creation arguments - for (int i = 0; i < defaulting.length; i++) { - params[userParameterCount + i] = defaulting[i]; - } - - try { - return (T) instantiator.newInstance(params); - } finally { - Arrays.fill(params, null); - } + return params; } } diff --git a/src/main/java/org/springframework/data/convert/ClassGeneratingKotlinEntityInstantiator.java b/src/main/java/org/springframework/data/convert/ClassGeneratingKotlinEntityInstantiator.java new file mode 100644 index 0000000000..03b94e61ed --- /dev/null +++ b/src/main/java/org/springframework/data/convert/ClassGeneratingKotlinEntityInstantiator.java @@ -0,0 +1,238 @@ +/* + * Copyright 2017 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 kotlin.reflect.KFunction; +import kotlin.reflect.KParameter; +import kotlin.reflect.jvm.ReflectJvmMapping; + +import java.lang.reflect.Constructor; +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.ParameterValueProvider; +import org.springframework.data.util.ReflectionUtils; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +/** + * Kotlin-specific extension to {@link ClassGeneratingEntityInstantiator} that adapts Kotlin constructors with + * defaulting. + * + * @author Mark Paluch + * @since 2.0 + */ +public class ClassGeneratingKotlinEntityInstantiator extends ClassGeneratingEntityInstantiator { + + /* + * (non-Javadoc) + * @see org.springframework.data.convert.ClassGeneratingEntityInstantiator#doCreateEntityInstantiator(org.springframework.data.mapping.PersistentEntity) + */ + @Override + protected EntityInstantiator doCreateEntityInstantiator(PersistentEntity entity) { + + if (ReflectionUtils.isKotlinClass(entity.getType()) && entity.getPersistenceConstructor() != null) { + + PreferredConstructor defaultConstructor = new DefaultingKotlinConstructorResolver(entity) + .getDefaultConstructor(); + + if (defaultConstructor != null) { + return new DefaultingKotlinClassInstantiatorAdapter(createObjectInstantiator(entity, defaultConstructor), + entity.getPersistenceConstructor()); + } + } + + return super.doCreateEntityInstantiator(entity); + } + + /** + * Resolves a {@link PreferredConstructor} to a synthetic Kotlin constructor accepting the same user-space parameters + * suffixed by Kotlin-specifics required for defaulting and the {@code kotlin.jvm.internal.DefaultConstructorMarker}. + * + * @since 2.0 + * @author Mark Paluch + */ + static class DefaultingKotlinConstructorResolver { + + @Nullable private final PreferredConstructor defaultConstructor; + + @SuppressWarnings("unchecked") + DefaultingKotlinConstructorResolver(PersistentEntity entity) { + + Constructor hit = resolveDefaultConstructor(entity); + PreferredConstructor persistenceConstructor = entity.getPersistenceConstructor(); + + if (hit != null && persistenceConstructor != null) { + this.defaultConstructor = new PreferredConstructor<>(hit, + persistenceConstructor.getParameters().toArray(new Parameter[0])); + } else { + this.defaultConstructor = null; + } + } + + @Nullable + private static Constructor resolveDefaultConstructor(PersistentEntity entity) { + + if (entity.getPersistenceConstructor() == null) { + return null; + } + + Constructor hit = null; + Constructor constructor = entity.getPersistenceConstructor().getConstructor(); + + for (Constructor candidate : entity.getType().getDeclaredConstructors()) { + + // use only synthetic constructors + if (!candidate.isSynthetic()) { + continue; + } + + // with a parameter count greater zero + if (constructor.getParameterCount() == 0) { + continue; + } + + // candidates must contain at least two additional parameters (int, DefaultConstructorMarker) + if (constructor.getParameterCount() + 2 > candidate.getParameterCount()) { + continue; + } + + java.lang.reflect.Parameter[] constructorParameters = constructor.getParameters(); + java.lang.reflect.Parameter[] candidateParameters = candidate.getParameters(); + + if (!candidateParameters[candidateParameters.length - 1].getType().getName() + .equals("kotlin.jvm.internal.DefaultConstructorMarker")) { + continue; + } + + if (parametersMatch(constructorParameters, candidateParameters)) { + hit = candidate; + break; + } + } + + return hit; + } + + private static boolean parametersMatch(java.lang.reflect.Parameter[] constructorParameters, + java.lang.reflect.Parameter[] candidateParameters) { + + return IntStream.range(0, constructorParameters.length) + .allMatch(i -> constructorParameters[i].getType().equals(candidateParameters[i].getType())); + } + + @Nullable + PreferredConstructor getDefaultConstructor() { + return defaultConstructor; + } + } + + /** + * Entity instantiator for Kotlin constructors that apply parameter defaulting. Kotlin constructors that apply + * argument defaulting are marked with {@link kotlin.jvm.internal.DefaultConstructorMarker} and accept additional + * parameters besides the regular (user-space) parameters. Additional parameters are: + *

    + *
  • defaulting bitmask ({@code int}), a bit mask slot for each 32 parameters
    • + *
  • {@code DefaultConstructorMarker} (usually null)
    • + *
+ * Defaulting bitmask + *

+ * The defaulting bitmask is a 32 bit integer representing which positional argument should be defaulted. Defaulted + * arguments are passed as {@literal null} and require the appropriate positional bit set ( {@code 1 << 2} for the 2. + * argument)). Since the bitmask represents only 32 bit states, it requires additional masks (slots) if more than 32 + * arguments are represented. + * + * @author Mark Paluch + * @since 2.0 + */ + static class DefaultingKotlinClassInstantiatorAdapter implements EntityInstantiator { + + private final ObjectInstantiator instantiator; + private final List kParameters; + private final Constructor synthetic; + + DefaultingKotlinClassInstantiatorAdapter(ObjectInstantiator instantiator, PreferredConstructor constructor) { + + KFunction kotlinConstructor = ReflectJvmMapping.getKotlinFunction(constructor.getConstructor()); + + if (kotlinConstructor == null) { + throw new IllegalArgumentException( + "No corresponding Kotlin constructor found for " + constructor.getConstructor()); + } + + this.instantiator = instantiator; + this.kParameters = kotlinConstructor.getParameters(); + this.synthetic = constructor.getConstructor(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.convert.EntityInstantiator#createInstance(org.springframework.data.mapping.PersistentEntity, org.springframework.data.mapping.model.ParameterValueProvider) + */ + @Override + @SuppressWarnings("unchecked") + public , P extends PersistentProperty

> T createInstance(E entity, + ParameterValueProvider

provider) { + + PreferredConstructor preferredConstructor = entity.getPersistenceConstructor(); + Assert.notNull(preferredConstructor, "PreferredConstructor must not be null!"); + + int[] defaulting = new int[(synthetic.getParameterCount() / 32) + 1]; + + Object[] params = allocateArguments( + synthetic.getParameterCount() + defaulting.length + /* DefaultConstructorMarker */1); + int userParameterCount = kParameters.size(); + + List> parameters = preferredConstructor.getParameters(); + + // Prepare user-space arguments + for (int i = 0; i < userParameterCount; i++) { + + int slot = i / 32; + int offset = slot * 32; + + Object param = provider.getParameterValue(parameters.get(i)); + + KParameter kParameter = kParameters.get(i); + + // what about null and parameter is mandatory? What if parameter is non-null? + if (kParameter.isOptional()) { + + if (param == null) { + defaulting[slot] = defaulting[slot] | (1 << (i - offset)); + } + } + + params[i] = param; + } + + // append nullability masks to creation arguments + for (int i = 0; i < defaulting.length; i++) { + params[userParameterCount + i] = defaulting[i]; + } + + try { + return (T) instantiator.newInstance(params); + } finally { + deallocateArguments(params); + } + } + } +} diff --git a/src/main/java/org/springframework/data/convert/EntityInstantiators.java b/src/main/java/org/springframework/data/convert/EntityInstantiators.java index 737146effd..cf05f47a8a 100644 --- a/src/main/java/org/springframework/data/convert/EntityInstantiators.java +++ b/src/main/java/org/springframework/data/convert/EntityInstantiators.java @@ -24,10 +24,11 @@ /** * Simple value object allowing access to {@link EntityInstantiator} instances for a given type falling back to a * default one. - * + * * @author Oliver Gierke * @author Thomas Darimont * @author Christoph Strobl + * @author Mark Paluch */ public class EntityInstantiators { @@ -43,7 +44,7 @@ public EntityInstantiators() { /** * Creates a new {@link EntityInstantiators} using the given {@link EntityInstantiator} as fallback. - * + * * @param fallback must not be {@literal null}. */ public EntityInstantiators(EntityInstantiator fallback) { @@ -52,17 +53,17 @@ public EntityInstantiators(EntityInstantiator fallback) { /** * Creates a new {@link EntityInstantiators} using the default fallback instantiator and the given custom ones. - * + * * @param customInstantiators must not be {@literal null}. */ public EntityInstantiators(Map, EntityInstantiator> customInstantiators) { - this(new ClassGeneratingEntityInstantiator(), customInstantiators); + this(new ClassGeneratingKotlinEntityInstantiator(), customInstantiators); } /** * Creates a new {@link EntityInstantiator} using the given fallback {@link EntityInstantiator} and the given custom * ones. - * + * * @param defaultInstantiator must not be {@literal null}. * @param customInstantiators must not be {@literal null}. */ @@ -78,7 +79,7 @@ public EntityInstantiators(EntityInstantiator defaultInstantiator, /** * Returns the {@link EntityInstantiator} to be used to create the given {@link PersistentEntity}. - * + * * @param entity must not be {@literal null}. * @return will never be {@literal null}. */ diff --git a/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt b/src/test/kotlin/org/springframework/data/convert/ClassGeneratingKotlinEntityInstantiatorUnitTests.kt similarity index 92% rename from src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt rename to src/test/kotlin/org/springframework/data/convert/ClassGeneratingKotlinEntityInstantiatorUnitTests.kt index 0d36a165e9..9ce59d24c8 100644 --- a/src/test/kotlin/org/springframework/data/convert/ClassGeneratingEntityInstantiatorDataClassUnitTests.kt +++ b/src/test/kotlin/org/springframework/data/convert/ClassGeneratingKotlinEntityInstantiatorUnitTests.kt @@ -29,13 +29,13 @@ import org.springframework.data.mapping.model.ParameterValueProvider import org.springframework.data.mapping.model.PreferredConstructorDiscoverer /** - * Unit tests for [ClassGeneratingEntityInstantiator] creating instances using Kotlin data classes. + * Unit tests for [ClassGeneratingKotlinEntityInstantiator] creating instances using Kotlin data classes. * * @author Mark Paluch */ @RunWith(MockitoJUnitRunner::class) @Suppress("UNCHECKED_CAST") -class ClassGeneratingEntityInstantiatorDataClassUnitTests { +class ClassGeneratingKotlinEntityInstantiatorUnitTests { @Mock lateinit var entity: PersistentEntity<*, *> @Mock lateinit var provider: ParameterValueProvider @@ -50,7 +50,7 @@ class ClassGeneratingEntityInstantiatorDataClassUnitTests { doReturn(constructor).whenever(entity).persistenceConstructor doReturn(constructor.constructor.declaringClass).whenever(entity).type - val instance: Contact = ClassGeneratingEntityInstantiator().createInstance(entity, provider) + val instance: Contact = ClassGeneratingKotlinEntityInstantiator().createInstance(entity, provider) Assertions.assertThat(instance.firstname).isEqualTo("Walter") Assertions.assertThat(instance.lastname).isEqualTo("White") @@ -69,7 +69,7 @@ class ClassGeneratingEntityInstantiatorDataClassUnitTests { doReturn(constructor).whenever(entity).persistenceConstructor doReturn(constructor.constructor.declaringClass).whenever(entity).type - val instance: ContactWithDefaulting = ClassGeneratingEntityInstantiator().createInstance(entity, provider) + val instance: ContactWithDefaulting = ClassGeneratingKotlinEntityInstantiator().createInstance(entity, provider) Assertions.assertThat(instance.prop0).isEqualTo("Walter") Assertions.assertThat(instance.prop2).isEqualTo("Skyler")