diff --git a/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java b/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java index f9837df503a4e..fb33b8f5689f2 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java @@ -914,7 +914,7 @@ public void execute(final BuildContext bc) { return new RuntimeValue<>(object); } } - throw new RuntimeException("Cannot inject type " + s); + return null; }) : null; for (int i = 0; i < methodArgs.length; i++) { diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/BytecodeRecorderConstantDefinitionBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/BytecodeRecorderConstantDefinitionBuildItem.java new file mode 100644 index 0000000000000..c214df8324729 --- /dev/null +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/BytecodeRecorderConstantDefinitionBuildItem.java @@ -0,0 +1,42 @@ +package io.quarkus.deployment.builditem; + +import io.quarkus.builder.item.MultiBuildItem; +import io.quarkus.deployment.recording.BytecodeRecorderImpl; + +/** + * The definition of a constant + * that can be injected into recorders via their {@code @Inject}-annotated constructor. + * + * Compared to simply passing the value to a recorder proxy, + * this build item allows for injecting values into recorders + * without introducing new dependencies from build steps + * that use the recorder to build steps that create the constant value. + * This can be useful in complex dependency graphs. + */ +public final class BytecodeRecorderConstantDefinitionBuildItem extends MultiBuildItem { + + private final Holder holder; + + public BytecodeRecorderConstantDefinitionBuildItem(Class type, T value) { + this.holder = new Holder<>(type, value); + } + + public void register(BytecodeRecorderImpl recorder) { + holder.register(recorder); + } + + // Necessary because generics are not allowed on BuildItems. + private static class Holder { + private final Class type; + private final T value; + + public Holder(Class type, T value) { + this.type = type; + this.value = value; + } + + public void register(BytecodeRecorderImpl recorder) { + recorder.registerConstant(type, value); + } + } +} diff --git a/core/deployment/src/main/java/io/quarkus/deployment/proxy/ProxyFactory.java b/core/deployment/src/main/java/io/quarkus/deployment/proxy/ProxyFactory.java index db67bdaed9e22..f34f64c6f9c16 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/proxy/ProxyFactory.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/proxy/ProxyFactory.java @@ -19,8 +19,6 @@ import io.quarkus.gizmo.MethodCreator; import io.quarkus.gizmo.MethodDescriptor; import io.quarkus.gizmo.ResultHandle; -import io.quarkus.runtime.RuntimeValue; -import io.quarkus.runtime.annotations.ConfigRoot; /** * A factory that can generate proxies of a class. @@ -55,7 +53,7 @@ public ProxyFactory(ProxyConfiguration configuration) { if (!findConstructor(superClass, configuration.isAllowPackagePrivate(), true)) { throw new IllegalArgumentException( "A proxy cannot be created for class " + this.superClassName - + " because it does contain a no-arg constructor"); + + " because it does not declare a no-arg constructor"); } if (Modifier.isFinal(superClass.getModifiers())) { throw new IllegalArgumentException( @@ -88,20 +86,9 @@ private boolean findConstructor(Class clazz, boolean allowPackagePrivate, boo //ctor needs to be @Inject or the only constructor if (constructor.isAnnotationPresent(Inject.class) || (ctors.length == 1 && constructor.getParameterCount() > 0)) { - if (!isModifiedCorrect(allowPackagePrivate, constructor)) { + if (!isModifierCorrect(allowPackagePrivate, constructor)) { return false; } - //if we have a constructor with only simple arguments (i.e. that also have a no-arg constructor) - //then we will use that, and just create the types - //this allows us to create proxys for recorders that use constructor injection for config objects - for (var i : constructor.getParameterTypes()) { - if (!(i.isAnnotationPresent(ConfigRoot.class) || i == RuntimeValue.class)) { - return false; - } - if (!findConstructor(i, allowPackagePrivate, false)) { - return false; - } - } injectConstructor = constructor; return true; } @@ -110,13 +97,13 @@ private boolean findConstructor(Class clazz, boolean allowPackagePrivate, boo for (Constructor constructor : ctors) { if (constructor.getParameterCount() == 0) { injectConstructor = constructor; - return isModifiedCorrect(allowPackagePrivate, constructor); + return isModifierCorrect(allowPackagePrivate, constructor); } } return false; } - private boolean isModifiedCorrect(boolean allowPackagePrivate, Constructor constructor) { + private boolean isModifierCorrect(boolean allowPackagePrivate, Constructor constructor) { if (allowPackagePrivate) { return !Modifier.isPrivate(constructor.getModifiers()); } @@ -271,9 +258,18 @@ public T newInstance(InvocationHandler handler) throws IllegalAccessException, I args[0] = handler; Class[] parameterTypes = this.constructor.getParameterTypes(); for (int i = 1; i < constructor.getParameterCount(); ++i) { - Constructor ctor = parameterTypes[i].getConstructor(); - ctor.setAccessible(true); - args[i] = ctor.newInstance(); + Constructor paramConstructor = null; + try { + paramConstructor = parameterTypes[i].getConstructor(); + } catch (NoSuchMethodException e) { + // We won't use the constructor + } + if (paramConstructor != null) { + paramConstructor.setAccessible(true); + args[i] = paramConstructor.newInstance(); + } else { + args[i] = null; + } } return (T) constructor.newInstance(args); } catch (Exception e) { diff --git a/core/deployment/src/main/java/io/quarkus/deployment/recording/BytecodeRecorderImpl.java b/core/deployment/src/main/java/io/quarkus/deployment/recording/BytecodeRecorderImpl.java index 9ce8bf24ff31d..abe191b6970f7 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/recording/BytecodeRecorderImpl.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/recording/BytecodeRecorderImpl.java @@ -12,6 +12,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.Parameter; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Proxy; import java.net.MalformedURLException; import java.net.URL; @@ -130,6 +131,7 @@ public class BytecodeRecorderImpl implements RecorderContext { private final Function configCreatorFunction; private final List loaders = new ArrayList<>(); + private final Map, ConstantHolder> constants = new HashMap<>(); private final Set classesToUseRecorableConstructor = new HashSet<>(); private final boolean useIdentityComparison; @@ -239,6 +241,12 @@ public void registerObjectLoader(ObjectLoader loader) { loaders.add(loader); } + public void registerConstant(Class type, T value) { + Assert.checkNotNullParam("type", type); + Assert.checkNotNullParam("value", value); + constants.put(type, new ConstantHolder<>(type, value)); + } + @Override public Class classProxy(String name) { // if it's a primitive there is no need to create a proxy (and doing so would result in errors when the value is used) @@ -512,7 +520,7 @@ ResultHandle createValue(MethodContext context, MethodCreator method, ResultHand } } for (var e : existingRecorderValues.entrySet()) { - e.getValue().preWrite(); + e.getValue().preWrite(parameterMap); } //when this is true it is no longer possible to allocate items in the array. this is a guard against programmer error @@ -1555,6 +1563,19 @@ ResultHandle createValue(MethodContext context, MethodCreator method, ResultHand return null; } + private ConstantHolder findConstantForParam(final java.lang.reflect.Type paramType) { + ConstantHolder holder = null; + if (paramType instanceof Class) { + holder = constants.get(paramType); + } else if (paramType instanceof ParameterizedType) { + ParameterizedType p = (ParameterizedType) paramType; + if (p.getRawType() == RuntimeValue.class) { + holder = constants.get(p.getActualTypeArguments()[0]); + } + } + return holder; + } + interface BytecodeInstruction { } @@ -1623,11 +1644,25 @@ final class NewRecorder extends DeferredArrayStoreParameter { this.injectCtor = injectCtor; } - void preWrite() { + void preWrite(Map parameterMap) { if (injectCtor != null) { try { - for (java.lang.reflect.Type param : injectCtor.getGenericParameterTypes()) { + java.lang.reflect.Type[] parameterTypes = injectCtor.getGenericParameterTypes(); + Annotation[][] parameterAnnotations = injectCtor.getParameterAnnotations(); + for (int i = 0; i < parameterTypes.length; i++) { + java.lang.reflect.Type param = parameterTypes[i]; + var constantHolder = findConstantForParam(param); + if (constantHolder != null) { + deferredParameters.add(loadObjectInstance(constantHolder.value, parameterMap, + constantHolder.type, Arrays.stream(parameterAnnotations[i]) + .anyMatch(s -> s.annotationType() == RelaxedValidation.class))); + continue; + } var obj = configCreatorFunction.apply(param); + if (obj == null) { + // No matching constant nor config. + throw new RuntimeException("Cannot inject type " + param); + } if (obj instanceof RuntimeValue) { if (!staticInit) { var result = findLoaded(((RuntimeValue) obj).getValue()); @@ -1720,6 +1755,16 @@ static final class NonDefaultConstructorHolder { } } + static final class ConstantHolder { + final Class type; + final T value; + + ConstantHolder(Class type, T value) { + this.type = type; + this.value = value; + } + } + private static final class ProxyInstance { final Object proxy; final String key; diff --git a/core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java b/core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java index 38e694f92d948..fc81c874d7b57 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java @@ -34,6 +34,7 @@ import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem; import io.quarkus.deployment.builditem.ApplicationClassNameBuildItem; import io.quarkus.deployment.builditem.ApplicationInfoBuildItem; +import io.quarkus.deployment.builditem.BytecodeRecorderConstantDefinitionBuildItem; import io.quarkus.deployment.builditem.BytecodeRecorderObjectLoaderBuildItem; import io.quarkus.deployment.builditem.CombinedIndexBuildItem; import io.quarkus.deployment.builditem.FeatureBuildItem; @@ -107,6 +108,7 @@ void build(List staticInitTasks, List features, BuildProducer appClassNameProducer, List loaders, + List constants, List recordableConstructorBuildItems, BuildProducer generatedClass, LaunchModeBuildItem launchMode, @@ -173,8 +175,7 @@ void build(List staticInitTasks, tryBlock.invokeStaticMethod(CONFIGURE_STEP_TIME_START); for (StaticBytecodeRecorderBuildItem holder : staticInitTasks) { writeRecordedBytecode(holder.getBytecodeRecorder(), null, substitutions, recordableConstructorBuildItems, loaders, - gizmoOutput, startupContext, - tryBlock); + constants, gizmoOutput, startupContext, tryBlock); } tryBlock.returnValue(null); @@ -249,7 +250,7 @@ void build(List staticInitTasks, for (MainBytecodeRecorderBuildItem holder : mainMethod) { writeRecordedBytecode(holder.getBytecodeRecorder(), holder.getGeneratedStartupContextClassName(), substitutions, recordableConstructorBuildItems, - loaders, gizmoOutput, startupContext, tryBlock); + loaders, constants, gizmoOutput, startupContext, tryBlock); } // Startup log messages @@ -418,7 +419,9 @@ private void generateMainForQuarkusApplication(String quarkusApplicationClassNam private void writeRecordedBytecode(BytecodeRecorderImpl recorder, String fallbackGeneratedStartupTaskClassName, List substitutions, List recordableConstructorBuildItems, - List loaders, GeneratedClassGizmoAdaptor gizmoOutput, + List loaders, + List constants, + GeneratedClassGizmoAdaptor gizmoOutput, ResultHandle startupContext, BytecodeCreator bytecodeCreator) { if ((recorder == null || recorder.isEmpty()) && fallbackGeneratedStartupTaskClassName == null) { @@ -436,6 +439,9 @@ private void writeRecordedBytecode(BytecodeRecorderImpl recorder, String fallbac for (var item : recordableConstructorBuildItems) { recorder.markClassAsConstructorRecordable(item.getClazz()); } + for (BytecodeRecorderConstantDefinitionBuildItem constant : constants) { + constant.register(recorder); + } recorder.writeBytecode(gizmoOutput); } diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java index 499e7e8e31066..85b3cde0ca5b3 100644 --- a/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java +++ b/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java @@ -383,6 +383,28 @@ void runTest(Consumer generator, Object... expected) throw } } + @Test + public void testConstantInjection() throws Exception { + runTest(generator -> { + generator.registerConstant(TestJavaBean.class, new TestJavaBean("Some string", 42)); + TestRecorderWithTestJavaBeanInjectedInConstructor recorder = generator + .getRecordingProxy(TestRecorderWithTestJavaBeanInjectedInConstructor.class); + recorder.retrieveConstant(); + }, new TestJavaBean("Some string", 42)); + } + + @Test + public void testConstantInjectionAndSubstitution() throws Exception { + runTest(generator -> { + generator.registerConstant(NonSerializable.class, new NonSerializable("Some string", 42)); + generator.registerSubstitution(NonSerializable.class, NonSerializable.Serialized.class, + NonSerializable.Substitution.class); + TestRecorderWithNonSerializableInjectedInConstructor recorder = generator + .getRecordingProxy(TestRecorderWithNonSerializableInjectedInConstructor.class); + recorder.retrieveConstant(); + }, new NonSerializable("Some string", 42)); + } + private static class TestClassOutput implements ClassOutput { private final TestClassLoader tcl; diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorderWithNonSerializableInjectedInConstructor.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorderWithNonSerializableInjectedInConstructor.java new file mode 100644 index 0000000000000..e1b511953e9af --- /dev/null +++ b/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorderWithNonSerializableInjectedInConstructor.java @@ -0,0 +1,18 @@ +package io.quarkus.deployment.recording; + +import javax.inject.Inject; + +public class TestRecorderWithNonSerializableInjectedInConstructor { + + private final NonSerializable constant; + + @Inject + public TestRecorderWithNonSerializableInjectedInConstructor(NonSerializable constant) { + this.constant = constant; + } + + public void retrieveConstant() { + TestRecorder.RESULT.add(constant); + } + +} diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorderWithTestJavaBeanInjectedInConstructor.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorderWithTestJavaBeanInjectedInConstructor.java new file mode 100644 index 0000000000000..def6ebb00fc6e --- /dev/null +++ b/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorderWithTestJavaBeanInjectedInConstructor.java @@ -0,0 +1,18 @@ +package io.quarkus.deployment.recording; + +import javax.inject.Inject; + +public class TestRecorderWithTestJavaBeanInjectedInConstructor { + + private final TestJavaBean constant; + + @Inject + public TestRecorderWithTestJavaBeanInjectedInConstructor(TestJavaBean constant) { + this.constant = constant; + } + + public void retrieveConstant() { + TestRecorder.RESULT.add(constant); + } + +} diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java index 33791e78f93b3..f3f8fe31e2fe8 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java @@ -92,6 +92,7 @@ import io.quarkus.deployment.builditem.AdditionalApplicationArchiveMarkerBuildItem; import io.quarkus.deployment.builditem.AdditionalIndexedClassesBuildItem; import io.quarkus.deployment.builditem.ApplicationArchivesBuildItem; +import io.quarkus.deployment.builditem.BytecodeRecorderConstantDefinitionBuildItem; import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem; import io.quarkus.deployment.builditem.CombinedIndexBuildItem; import io.quarkus.deployment.builditem.DevServicesLauncherConfigResultBuildItem; @@ -104,6 +105,7 @@ import io.quarkus.deployment.builditem.RunTimeConfigurationDefaultBuildItem; import io.quarkus.deployment.builditem.ServiceStartBuildItem; import io.quarkus.deployment.builditem.SystemPropertyBuildItem; +import io.quarkus.deployment.builditem.TransformedClassesBuildItem; import io.quarkus.deployment.builditem.nativeimage.NativeImageProxyDefinitionBuildItem; import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBuildItem; import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem; @@ -144,7 +146,9 @@ import io.quarkus.runtime.configuration.ConfigUtils; import io.quarkus.runtime.configuration.ConfigurationException; import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.ClassFileLocator; import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.pool.TypePool; /** * Simulacrum of JPA bootstrap. @@ -494,9 +498,10 @@ public void defineJpaEntities( } @BuildStep - public ProxyDefinitionsBuildItem pregenProxies( + public BytecodeRecorderConstantDefinitionBuildItem pregenProxies( JpaModelBuildItem jpaModel, JpaModelIndexBuildItem indexBuildItem, + TransformedClassesBuildItem transformedClassesBuildItem, List persistenceUnitDescriptorBuildItems, BuildProducer generatedClassBuildItemBuildProducer, LiveReloadBuildItem liveReloadBuildItem) { @@ -509,8 +514,28 @@ public ProxyDefinitionsBuildItem pregenProxies( managedClassAndPackageNames.addAll(pud.getManagedClassNames()); } PreGeneratedProxies proxyDefinitions = generatedProxies(managedClassAndPackageNames, - indexBuildItem.getIndex(), generatedClassBuildItemBuildProducer, liveReloadBuildItem); - return new ProxyDefinitionsBuildItem(proxyDefinitions); + indexBuildItem.getIndex(), transformedClassesBuildItem, + generatedClassBuildItemBuildProducer, liveReloadBuildItem); + + // Make proxies available through a constant; + // this is a hack to avoid introducing circular dependencies between build steps. + // + // If we just passed the proxy definitions to #build as a normal build item, + // we would have the following dependencies: + // + // #pregenProxies => ProxyDefinitionsBuildItem => #build => BeanContainerListenerBuildItem + // => Arc container init => BeanContainerBuildItem + // => some RestEasy Reactive Method => BytecodeTransformerBuildItem + // => build step that transforms bytecode => TransformedClassesBuildItem + // => #pregenProxies + // + // Since the dependency from #preGenProxies to #build is only a static init thing + // (#build needs to pass the proxy definitions to the recorder), + // we get rid of the circular dependency by defining a constant + // to pass the proxy definitions to the recorder. + // That way, the dependency is only between #pregenProxies + // and the build step that generates the bytecode of bytecode recorders. + return new BytecodeRecorderConstantDefinitionBuildItem(PreGeneratedProxies.class, proxyDefinitions); } @BuildStep(onlyIf = NativeOrNativeSourcesBuild.class) @@ -554,7 +579,6 @@ public void build(RecorderContext recorderContext, HibernateOrmRecorder recorder JpaModelBuildItem jpaModel, List persistenceUnitDescriptorBuildItems, List integrationBuildItems, - ProxyDefinitionsBuildItem proxyDefinitions, BuildProducer feature, BuildProducer beanContainerListener, LaunchModeBuildItem launchMode) throws Exception { @@ -620,8 +644,7 @@ public void build(RecorderContext recorderContext, HibernateOrmRecorder recorder beanContainerListener .produce(new BeanContainerListenerBuildItem( - recorder.initMetadata(finalStagePUDescriptors, scanner, integratorClasses, - proxyDefinitions.getProxies()))); + recorder.initMetadata(finalStagePUDescriptors, scanner, integratorClasses))); } private void validateHibernatePropertiesNotUsed() { @@ -709,7 +732,6 @@ public HibernateModelClassCandidatesForFieldAccessBuildItem candidatesForFieldAc @Record(STATIC_INIT) public void build(HibernateOrmRecorder recorder, HibernateOrmConfig hibernateOrmConfig, BuildProducer jpaModelPersistenceUnitMapping, - BuildProducer buildProducer, BuildProducer syntheticBeans, List descriptors, JpaModelBuildItem jpaModel) throws Exception { @@ -1483,6 +1505,7 @@ private static MultiTenancyStrategy getMultiTenancyStrategy(Optional mul } private PreGeneratedProxies generatedProxies(Set managedClassAndPackageNames, IndexView combinedIndex, + TransformedClassesBuildItem transformedClassesBuildItem, BuildProducer generatedClassBuildItemBuildProducer, LiveReloadBuildItem liveReloadBuildItem) { ProxyCache proxyCache = liveReloadBuildItem.getContextObject(ProxyCache.class); @@ -1507,7 +1530,9 @@ private PreGeneratedProxies generatedProxies(Set managedClassAndPackageN } proxyAnnotations.put(i.target().asClass().name().toString(), proxyClass.asClass().name().toString()); } - try (ProxyBuildingHelper proxyHelper = new ProxyBuildingHelper(Thread.currentThread().getContextClassLoader())) { + TypePool transformedClassesTypePool = createTransformedClassesTypePool(transformedClassesBuildItem, + managedClassAndPackageNames); + try (ProxyBuildingHelper proxyHelper = new ProxyBuildingHelper(transformedClassesTypePool)) { for (String managedClassOrPackageName : managedClassAndPackageNames) { CachedProxy result; if (proxyCache.cache.containsKey(managedClassOrPackageName) @@ -1554,6 +1579,27 @@ private PreGeneratedProxies generatedProxies(Set managedClassAndPackageN return preGeneratedProxies; } + // Creates a TypePool that is aware of class transformations applied to entity classes, + // so that ByteBuddy can take these transformations into account. + // This is especially important when getters/setters are added to entity classes, + // because we want those methods to be overridden in proxies to trigger proxy initialization. + private TypePool createTransformedClassesTypePool(TransformedClassesBuildItem transformedClassesBuildItem, + Set entityClasses) { + Map transformedClasses = new HashMap<>(); + for (Set transformedClassSet : transformedClassesBuildItem + .getTransformedClassesByJar().values()) { + for (TransformedClassesBuildItem.TransformedClass transformedClass : transformedClassSet) { + String className = transformedClass.getClassName(); + if (entityClasses.contains(className)) { + transformedClasses.put(className, transformedClass.getData()); + } + } + } + return TypePool.Default.of(new ClassFileLocator.Compound( + new ClassFileLocator.Simple(transformedClasses), + ClassFileLocator.ForClassLoader.of(Thread.currentThread().getContextClassLoader()))); + } + private boolean isModified(String entity, Set changedClasses, IndexView index) { if (changedClasses.contains(entity)) { return true; diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyBuildingHelper.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyBuildingHelper.java index 3419488fa8a2c..2927e7fa4254f 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyBuildingHelper.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyBuildingHelper.java @@ -1,13 +1,20 @@ package io.quarkus.hibernate.orm.deployment; -import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import org.hibernate.bytecode.internal.bytebuddy.BytecodeProviderImpl; import org.hibernate.proxy.pojo.bytebuddy.ByteBuddyProxyHelper; import net.bytebuddy.ClassFileVersion; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDefinition; +import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.matcher.ElementMatcher; +import net.bytebuddy.matcher.ElementMatchers; +import net.bytebuddy.pool.TypePool; /** * Makes it slightly more readable to interact with the Hibernate @@ -15,22 +22,24 @@ */ final class ProxyBuildingHelper implements AutoCloseable { - private final ClassLoader contextClassLoader; + private static final ElementMatcher NO_ARG_CONSTRUCTOR = ElementMatchers + .isConstructor().and(ElementMatchers.takesNoArguments()); + + private final TypePool typePool; private ByteBuddyProxyHelper byteBuddyProxyHelper; private BytecodeProviderImpl bytecodeProvider; - public ProxyBuildingHelper(ClassLoader contextClassLoader) { - this.contextClassLoader = contextClassLoader; + public ProxyBuildingHelper(TypePool typePool) { + this.typePool = typePool; } public DynamicType.Unloaded buildUnloadedProxy(String mappedClassName, Set interfaceNames) { - final Class[] interfaces = new Class[interfaceNames.size()]; + List interfaces = new ArrayList<>(); int i = 0; for (String name : interfaceNames) { - interfaces[i++] = uninitializedClass(name); + interfaces.add(typePool.describe(name).resolve()); } - final Class mappedClass = uninitializedClass(mappedClassName); - return getByteBuddyProxyHelper().buildUnloadedProxy(mappedClass, interfaces); + return getByteBuddyProxyHelper().buildUnloadedProxy(typePool, typePool.describe(mappedClassName).resolve(), interfaces); } private ByteBuddyProxyHelper getByteBuddyProxyHelper() { @@ -43,32 +52,17 @@ private ByteBuddyProxyHelper getByteBuddyProxyHelper() { return this.byteBuddyProxyHelper; } - private Class uninitializedClass(String entity) { - try { - return Class.forName(entity, false, contextClassLoader); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); - } - } - public boolean isProxiable(String managedClassOrPackageName) { - Class mappedClass; - try { - mappedClass = Class.forName(managedClassOrPackageName, false, contextClassLoader); - } catch (ClassNotFoundException e) { + TypePool.Resolution mappedClassResolution = typePool.describe(managedClassOrPackageName); + if (!mappedClassResolution.isResolved()) { // Probably a package name - consider it's not proxiable. return false; } - if (Modifier.isFinal(mappedClass.getModifiers())) { - return false; - } - try { - mappedClass.getDeclaredConstructor(); - } catch (NoSuchMethodException e) { - return false; - } - return true; + TypeDescription mappedClass = mappedClassResolution.resolve(); + + return !mappedClass.isFinal() + && !mappedClass.getDeclaredMethods().filter(NO_ARG_CONSTRUCTOR).isEmpty(); } @Override diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyDefinitionsBuildItem.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyDefinitionsBuildItem.java deleted file mode 100644 index 44774754cd2f9..0000000000000 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyDefinitionsBuildItem.java +++ /dev/null @@ -1,28 +0,0 @@ -package io.quarkus.hibernate.orm.deployment; - -import java.util.Objects; - -import io.quarkus.builder.item.SimpleBuildItem; -import io.quarkus.hibernate.orm.runtime.proxies.PreGeneratedProxies; - -/** - * Contains the reference to the class definitions of the proxies - * that Hibernate ORM might require at runtime. - * In Quarkus such proxies are built upfront, during the build. - * This needs to be a separate build item from other components so - * to avoid cycles in the rather complex build graph required by - * this extension. - */ -public final class ProxyDefinitionsBuildItem extends SimpleBuildItem { - - private final PreGeneratedProxies proxies; - - public ProxyDefinitionsBuildItem(PreGeneratedProxies proxies) { - Objects.requireNonNull(proxies); - this.proxies = proxies; - } - - public PreGeneratedProxies getProxies() { - return proxies; - } -} diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/TransactionTestUtils.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/TransactionTestUtils.java new file mode 100644 index 0000000000000..7f57554a2dbfc --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/TransactionTestUtils.java @@ -0,0 +1,32 @@ +package io.quarkus.hibernate.orm; + +import javax.transaction.HeuristicMixedException; +import javax.transaction.HeuristicRollbackException; +import javax.transaction.NotSupportedException; +import javax.transaction.RollbackException; +import javax.transaction.SystemException; +import javax.transaction.UserTransaction; + +public class TransactionTestUtils { + + public static void inTransaction(UserTransaction transaction, Runnable runnable) { + try { + transaction.begin(); + try { + runnable.run(); + transaction.commit(); + } catch (Throwable t) { + try { + transaction.rollback(); + } catch (Throwable t2) { + t.addSuppressed(t2); + } + throw t; + } + } catch (SystemException | NotSupportedException | RollbackException | HeuristicMixedException + | HeuristicRollbackException e) { + throw new IllegalStateException("Transaction exception", e); + } + } + +} diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerFinalFieldTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerFinalFieldTest.java index 9d2df3696772c..13a2287eec7c6 100644 --- a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerFinalFieldTest.java +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/enhancer/HibernateEntityEnhancerFinalFieldTest.java @@ -20,14 +20,13 @@ import javax.persistence.EntityManager; import javax.persistence.GeneratedValue; import javax.persistence.Id; -import javax.transaction.NotSupportedException; -import javax.transaction.SystemException; import javax.transaction.UserTransaction; import org.hibernate.annotations.Immutable; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.hibernate.orm.TransactionTestUtils; import io.quarkus.test.QuarkusUnitTest; /** @@ -41,6 +40,7 @@ public class HibernateEntityEnhancerFinalFieldTest { @RegisterExtension static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar + .addClass(TransactionTestUtils.class) .addClasses( EntityWithFinalField.class, EntityWithEmbeddedIdWithFinalField.class, EntityWithEmbeddedIdWithFinalField.EmbeddableId.class, @@ -143,17 +143,7 @@ public void embeddableNonIdWithFinalField_smokeTest() { } private void inTransaction(Runnable runnable) { - try { - transaction.begin(); - try { - runnable.run(); - transaction.commit(); - } catch (Exception e) { - transaction.rollback(); - } - } catch (SystemException | NotSupportedException e) { - throw new IllegalStateException("Transaction exception", e); - } + TransactionTestUtils.inTransaction(transaction, runnable); } @Entity(name = "withfinal") diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/packages/PackageLevelAnnotationTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/packages/PackageLevelAnnotationTest.java index 20f2da92decbb..b299c86075d08 100644 --- a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/packages/PackageLevelAnnotationTest.java +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/packages/PackageLevelAnnotationTest.java @@ -4,13 +4,12 @@ import javax.inject.Inject; import javax.persistence.EntityManager; -import javax.transaction.NotSupportedException; -import javax.transaction.SystemException; import javax.transaction.UserTransaction; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.hibernate.orm.TransactionTestUtils; import io.quarkus.test.QuarkusUnitTest; public class PackageLevelAnnotationTest { @@ -18,6 +17,7 @@ public class PackageLevelAnnotationTest { @RegisterExtension static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar + .addClass(TransactionTestUtils.class) .addPackage(PackageLevelAnnotationTest.class.getPackage()) .addAsResource("application.properties")); @@ -56,16 +56,6 @@ public void test() { } private void inTransaction(Runnable runnable) { - try { - transaction.begin(); - try { - runnable.run(); - transaction.commit(); - } catch (Exception e) { - transaction.rollback(); - } - } catch (SystemException | NotSupportedException e) { - throw new IllegalStateException("Transaction exception", e); - } + TransactionTestUtils.inTransaction(transaction, runnable); } } diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/publicfields/PublicFieldAccessFinalFieldTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/publicfields/PublicFieldAccessFinalFieldTest.java index 1f7a6ac0a8053..aaed67aa68cad 100644 --- a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/publicfields/PublicFieldAccessFinalFieldTest.java +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/publicfields/PublicFieldAccessFinalFieldTest.java @@ -20,14 +20,13 @@ import javax.persistence.EntityManager; import javax.persistence.GeneratedValue; import javax.persistence.Id; -import javax.transaction.NotSupportedException; -import javax.transaction.SystemException; import javax.transaction.UserTransaction; import org.hibernate.annotations.Immutable; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.hibernate.orm.TransactionTestUtils; import io.quarkus.test.QuarkusUnitTest; /** @@ -41,6 +40,7 @@ public class PublicFieldAccessFinalFieldTest { @RegisterExtension static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar + .addClass(TransactionTestUtils.class) .addClasses( EntityWithFinalField.class, EntityWithEmbeddedIdWithFinalField.class, EntityWithEmbeddedIdWithFinalField.EmbeddableId.class, @@ -143,17 +143,7 @@ public void embeddableNonIdWithFinalField_smokeTest() { } private void inTransaction(Runnable runnable) { - try { - transaction.begin(); - try { - runnable.run(); - transaction.commit(); - } catch (Exception e) { - transaction.rollback(); - } - } catch (SystemException | NotSupportedException e) { - throw new IllegalStateException("Transaction exception", e); - } + TransactionTestUtils.inTransaction(transaction, runnable); } @Entity(name = "withfinal") diff --git a/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/publicfields/PublicFieldWithProxyAndLazyLoadingAndInheritanceTest.java b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/publicfields/PublicFieldWithProxyAndLazyLoadingAndInheritanceTest.java new file mode 100644 index 0000000000000..b0a8ae6d00c0b --- /dev/null +++ b/extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/publicfields/PublicFieldWithProxyAndLazyLoadingAndInheritanceTest.java @@ -0,0 +1,122 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package io.quarkus.hibernate.orm.publicfields; + +import static io.quarkus.hibernate.orm.TransactionTestUtils.inTransaction; +import static org.assertj.core.api.Assertions.assertThat; + +import javax.inject.Inject; +import javax.persistence.Entity; +import javax.persistence.EntityManager; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.transaction.UserTransaction; + +import org.hibernate.Hibernate; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.hibernate.orm.TransactionTestUtils; +import io.quarkus.test.QuarkusUnitTest; + +public class PublicFieldWithProxyAndLazyLoadingAndInheritanceTest { + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClass(TransactionTestUtils.class) + .addClasses(Containing.class, Contained.class, ContainedExtended.class) + .addClass(FieldAccessEnhancedDelegate.class)) + .withConfigurationResource("application.properties"); + + @Inject + EntityManager em; + + @Inject + UserTransaction transaction; + + private Long containingID; + + @Test + public void test() { + FieldAccessEnhancedDelegate delegate = new FieldAccessEnhancedDelegate(); + inTransaction(transaction, () -> { + containingID = delegate.createEntities(em, "George"); + }); + inTransaction(transaction, () -> { + delegate.testLazyLoading(em, containingID); + }); + } + + @Entity(name = "Containing") + public static class Containing { + + @Id + @GeneratedValue(strategy = GenerationType.AUTO) + public Long id; + + @ManyToOne(fetch = FetchType.LAZY, optional = false) + public Contained contained; + } + + @Entity(name = "Contained") + public static class Contained { + + @Id + @GeneratedValue(strategy = GenerationType.AUTO) + public Long id; + + public String name; + + Contained() { + } + + Contained(String name) { + this.name = name; + } + } + + @Entity(name = "ContainedExtended") + public static class ContainedExtended extends Contained { + + ContainedExtended() { + } + + ContainedExtended(String name) { + this.name = name; + } + + } + + /** + * A class whose bytecode is transformed by Quarkus to replace public field access with getter/setter access. + *

+ * (Test bytecode was not transformed by Quarkus when using QuarkusUnitTest last time I checked). + */ + private static class FieldAccessEnhancedDelegate { + + public long createEntities(EntityManager entityManager, String name) { + Containing containing = new Containing(); + ContainedExtended contained = new ContainedExtended(name); + containing.contained = contained; + entityManager.persist(contained); + entityManager.persist(containing); + return containing.id; + } + + public void testLazyLoading(EntityManager entityManager, Long containingID) { + Containing containing = entityManager.find(Containing.class, containingID); + Contained contained = containing.contained; + assertThat(contained).isNotNull(); + assertThat(Hibernate.isPropertyInitialized(contained, "name")).isFalse(); + assertThat(contained.name).isNotNull(); + } + } +} diff --git a/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRecorder.java b/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRecorder.java index da8ea35090e7d..c7180f513cc54 100644 --- a/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRecorder.java +++ b/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRecorder.java @@ -8,6 +8,8 @@ import java.util.Set; import java.util.function.Supplier; +import javax.inject.Inject; + import org.eclipse.microprofile.config.ConfigProvider; import org.hibernate.MultiTenancyStrategy; import org.hibernate.Session; @@ -33,7 +35,13 @@ @Recorder public class HibernateOrmRecorder { - private List entities = new ArrayList<>(); + private final PreGeneratedProxies proxyDefinitions; + private final List entities = new ArrayList<>(); + + @Inject + public HibernateOrmRecorder(PreGeneratedProxies proxyDefinitions) { + this.proxyDefinitions = proxyDefinitions; + } public void enlistPersistenceUnit(Set entityClassNames) { entities.addAll(entityClassNames); @@ -55,8 +63,7 @@ public void setupPersistenceProvider(HibernateOrmRuntimeConfig hibernateOrmRunti } public BeanContainerListener initMetadata(List parsedPersistenceXmlDescriptors, - Scanner scanner, Collection> additionalIntegrators, - PreGeneratedProxies proxyDefinitions) { + Scanner scanner, Collection> additionalIntegrators) { SchemaManagementIntegrator.clearDsMap(); for (QuarkusPersistenceUnitDefinition i : parsedPersistenceXmlDescriptors) { if (i.getDataSource().isPresent()) {