From ad081c1c879227d5331a7897b0d4514e95df9498 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 07d7b550a..db2a3f99a 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 bb3f263cd..b1734977e 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