From 9478441dbdd907cb4e76455b37be16cf44645235 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 17 Aug 2018 11:19:31 +0200 Subject: [PATCH] =?UTF-8?q?DATACMNS-1373=20-=20Use=20ReflectUtils.defineCl?= =?UTF-8?q?ass(=E2=80=A6)=20to=20load=20generated=20EntityInstantiators.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now use Spring's ReflectUtils.defineClass(…) to load generated EntityInstantiators which uses internally either MethodHandles.Lookup.defineClass(…) (on Java 9 and higher) or reflective defineClass invocation on the originating ClassLoader. Class injection into the originating ClassLoader assigns the ClassLoader of the entity to the generated class which allows optimized instantiation of package-protected classes and constructors. Original pull request: #308. --- .../ClassGeneratingEntityInstantiator.java | 60 +++------------ ...GeneratingEntityInstantiatorUnitTests.java | 77 +++++++++++++++++++ 2 files changed, 89 insertions(+), 48 deletions(-) diff --git a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java index d083b0bba..025ea6f91 100644 --- a/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java +++ b/src/main/java/org/springframework/data/convert/ClassGeneratingEntityInstantiator.java @@ -19,8 +19,6 @@ import static org.springframework.asm.Opcodes.*; import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -30,6 +28,7 @@ import org.springframework.asm.ClassWriter; import org.springframework.asm.MethodVisitor; import org.springframework.asm.Opcodes; import org.springframework.asm.Type; +import org.springframework.cglib.core.ReflectUtils; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PreferredConstructor; @@ -38,7 +37,6 @@ import org.springframework.data.mapping.model.MappingInstantiationException; import org.springframework.data.mapping.model.ParameterValueProvider; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; -import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** @@ -139,20 +137,20 @@ public class ClassGeneratingEntityInstantiator implements EntityInstantiator { * @param entity * @return */ - private boolean shouldUseReflectionEntityInstantiator(PersistentEntity entity) { + boolean shouldUseReflectionEntityInstantiator(PersistentEntity entity) { Class type = entity.getType(); if (type.isInterface() // || type.isArray() // - || !Modifier.isPublic(type.getModifiers()) // + || Modifier.isPrivate(type.getModifiers()) // || (type.isMemberClass() && !Modifier.isStatic(type.getModifiers())) // || ClassUtils.isCglibProxyClass(type)) { // return true; } PreferredConstructor persistenceConstructor = entity.getPersistenceConstructor(); - if (persistenceConstructor == null || !Modifier.isPublic(persistenceConstructor.getConstructor().getModifiers())) { + if (persistenceConstructor == null || Modifier.isPrivate(persistenceConstructor.getConstructor().getModifiers())) { return true; } @@ -299,6 +297,7 @@ public class ClassGeneratingEntityInstantiator implements EntityInstantiator { * * * @author Thomas Darimont + * @author Mark Paluch */ static class ObjectInstantiatorClassGenerator { @@ -310,13 +309,6 @@ public class ClassGeneratingEntityInstantiator implements EntityInstantiator { private static final String[] IMPLEMENTED_INTERFACES = new String[] { Type.getInternalName(ObjectInstantiator.class) }; - private final ByteArrayClassLoader classLoader; - - ObjectInstantiatorClassGenerator() { - - this.classLoader = AccessController.doPrivileged( - (PrivilegedAction) () -> new ByteArrayClassLoader(ClassUtils.getDefaultClassLoader())); - } /** * Generate a new class for the given {@link PersistentEntity}. @@ -331,7 +323,13 @@ public class ClassGeneratingEntityInstantiator implements EntityInstantiator { String className = generateClassName(entity); byte[] bytecode = generateBytecode(className, entity, constructor); - return classLoader.loadClass(className, bytecode); + Class type = entity.getType(); + + try { + return ReflectUtils.defineClass(className, bytecode, type.getClassLoader(), type.getProtectionDomain()); + } catch (Exception e) { + throw new IllegalStateException(e); + } } /** @@ -523,39 +521,5 @@ public class ClassGeneratingEntityInstantiator implements EntityInstantiator { throw new IllegalArgumentException("Unboxing should not be attempted for descriptor '" + ch + "'"); } } - - /** - * A {@link ClassLoader} that can load classes from {@code byte[]} representations. - * - * @author Thomas Darimont - */ - private class ByteArrayClassLoader extends ClassLoader { - - public ByteArrayClassLoader(@Nullable ClassLoader parent) { - super(parent); - } - - /** - * Tries to load a class given {@code byte[]}. - * - * @param name must not be {@literal null} - * @param bytes must not be {@literal null} - * @return - */ - public Class loadClass(String name, byte[] bytes) { - - Assert.notNull(name, "name must not be null"); - Assert.notNull(bytes, "bytes must not be null"); - - try { - Class clazz = findClass(name); - if (clazz != null) { - return clazz; - } - } catch (ClassNotFoundException ignore) {} - - return defineClass(name, bytes, 0, bytes.length); - } - } } } diff --git a/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java b/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java index 4de3b357e..1e05d5047 100755 --- a/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java +++ b/src/test/java/org/springframework/data/convert/ClassGeneratingEntityInstantiatorUnitTests.java @@ -303,6 +303,66 @@ public class ClassGeneratingEntityInstantiatorUnitTests

typeInDefaultPackage = Class.forName("TypeInDefaultPackage"); + prepareMocks(typeInDefaultPackage); + + assertThat(this.instance.shouldUseReflectionEntityInstantiator(entity)).isFalse(); + assertThat(this.instance.createInstance(entity, provider)).isInstanceOf(typeInDefaultPackage); + } + + @Test // DATACMNS-1373 + public void shouldNotInstantiateClassWithPrivateConstructor() { + + prepareMocks(ClassWithPrivateConstructor.class); + + assertThat(this.instance.shouldUseReflectionEntityInstantiator(entity)).isTrue(); + } + + private void prepareMocks(Class type) { + + doReturn(type).when(entity).getType(); + doReturn(PreferredConstructorDiscoverer.discover(type))// + .when(entity).getPersistenceConstructor(); + } + static class Foo { Foo(String foo) { @@ -424,4 +484,21 @@ public class ClassGeneratingEntityInstantiatorUnitTests