From c4d9f32c79a8b4fdd4e545fd05e693af7b6abb20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 18 Dec 2017 18:27:12 +0100 Subject: [PATCH] HHH-12133 Comply with API docs with respect to lifecycle management depending on the 'shouldRegistryManageLifecycle' parameter The registry should not manage the bean lifecycle when 'shouldRegistryManageLifecycle' is false. The easiest way to do so is to use BeanManager.createInstance to retrieve beans in the Standard CDI lifecycle strategy: it correctly retrieves singletons from the CDI context instead of instantiating them again. Also, fix javax.enterprise.inject.spi.Bean-based instance destructions: we used to only request destruction to the creational context, which is wrong because it may skip the execution of @PostDestroy methods in particular. --- .../resource/beans/internal/Helper.java | 27 ++++-------- .../JpaCdiLifecycleManagementStrategy.java | 21 ++++++++-- .../ManagedBeanRegistryCdiDelayedImpl.java | 10 +++-- .../ManagedBeanRegistryCdiExtendedImpl.java | 10 +++-- .../ManagedBeanRegistryCdiStandardImpl.java | 10 +++-- .../ManagedBeanRegistryDirectImpl.java | 7 +++- ...tandardCdiLifecycleManagementStrategy.java | 41 ++++++++++--------- .../spi/AbstractManagedBeanRegistry.java | 12 +++--- 8 files changed, 76 insertions(+), 62 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/Helper.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/Helper.java index 4a361f14ed97..9f0768e64253 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/Helper.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/Helper.java @@ -22,24 +22,6 @@ public class Helper { private Helper() { } - @SuppressWarnings("unchecked") - public Bean getBean(Class beanContract, BeanManager beanManager) { - Set> beans = beanManager.getBeans( beanContract ); - - if ( beans.isEmpty() ) { - throw new IllegalArgumentException( - "BeanManager returned no matching beans: contract = " + beanContract.getName() - ); - } - if ( beans.size() > 1 ) { - throw new IllegalArgumentException( - "BeanManager returned multiple matching beans: contract = " + beanContract.getName() - ); - } - - return (Bean) beans.iterator().next(); - } - @SuppressWarnings("unchecked") public Bean getNamedBean(String beanName, Class beanContract, BeanManager beanManager) { Set> beans = beanManager.getBeans( beanContract, new NamedBeanQualifier( beanName ) ); @@ -57,4 +39,13 @@ public Bean getNamedBean(String beanName, Class beanContract, BeanMana return (Bean) beans.iterator().next(); } + + public CdiLifecycleManagementStrategy getLifecycleManagementStrategy(boolean shouldRegistryManageLifecycle) { + if ( shouldRegistryManageLifecycle ) { + return JpaCdiLifecycleManagementStrategy.INSTANCE; + } + else { + return StandardCdiLifecycleManagementStrategy.INSTANCE; + } + } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/JpaCdiLifecycleManagementStrategy.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/JpaCdiLifecycleManagementStrategy.java index 0ef1567953ec..48268abe9072 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/JpaCdiLifecycleManagementStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/JpaCdiLifecycleManagementStrategy.java @@ -14,6 +14,18 @@ import org.hibernate.resource.beans.spi.ManagedBean; +/** + * A {@link CdiLifecycleManagementStrategy} to use when JPA compliance is required + * (i.e. when the bean lifecycle is to be managed by the JPA runtime, not the CDI runtime). + * + * The main characteristic of this strategy is that each requested bean is instantiated directly + * and guaranteed to not be shared in the CDI context. + * + * In particular, @Singleton-scoped or @ApplicationScoped beans are instantiated directly by this strategy, + * even if there is already an instance in the CDI context. + * This means singletons are not really singletons, but this seems to be the behavior required by + * the JPA 2.2 spec. + */ class JpaCdiLifecycleManagementStrategy implements CdiLifecycleManagementStrategy { static final JpaCdiLifecycleManagementStrategy INSTANCE = new JpaCdiLifecycleManagementStrategy(); @@ -44,7 +56,7 @@ public ManagedBean createBean(BeanManager beanManager, String beanName, C T beanInstance = bean.create( creationalContext ); - return new NamedJpaManagedBeanImpl<>( beanClass, creationalContext, beanInstance ); + return new NamedJpaManagedBeanImpl<>( beanClass, bean, creationalContext, beanInstance ); } private static class JpaManagedBeanImpl implements ManagedBean { @@ -82,13 +94,14 @@ public void release() { private static class NamedJpaManagedBeanImpl implements ManagedBean { private final Class beanClass; + private final Bean bean; private final CreationalContext creationContext; private final T beanInstance; private NamedJpaManagedBeanImpl( - Class beanClass, - CreationalContext creationContext, T beanInstance) { + Class beanClass, Bean bean, CreationalContext creationContext, T beanInstance) { this.beanClass = beanClass; + this.bean = bean; this.creationContext = creationContext; this.beanInstance = beanInstance; } @@ -105,7 +118,7 @@ public T getBeanInstance() { @Override public void release() { - creationContext.release(); + bean.destroy( beanInstance, creationContext ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiDelayedImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiDelayedImpl.java index 27d7c9d09dbc..cc67dc8f109e 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiDelayedImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiDelayedImpl.java @@ -33,13 +33,15 @@ private ManagedBeanRegistryCdiDelayedImpl(BeanManager beanManager) { } @Override - protected ManagedBean createBean(Class beanClass) { - return new LazilyInitializedManagedBeanImpl<>( beanClass, JpaCdiLifecycleManagementStrategy.INSTANCE ); + protected ManagedBean createBean(Class beanClass, boolean shouldRegistryManageLifecycle) { + return new LazilyInitializedManagedBeanImpl<>( beanClass, + Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) ); } @Override - protected ManagedBean createBean(String beanName, Class beanContract) { - return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract, StandardCdiLifecycleManagementStrategy.INSTANCE ); + protected ManagedBean createBean(String beanName, Class beanContract, boolean shouldRegistryManageLifecycle) { + return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract, + Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) ); } /** diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiExtendedImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiExtendedImpl.java index 757df3e36d46..534f2a9aaa5b 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiExtendedImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiExtendedImpl.java @@ -39,13 +39,15 @@ private ManagedBeanRegistryCdiExtendedImpl(ExtendedBeanManager beanManager) { } @Override - protected ManagedBean createBean(Class beanClass) { - return new LazilyInitializedManagedBeanImpl<>( beanClass, JpaCdiLifecycleManagementStrategy.INSTANCE ); + protected ManagedBean createBean(Class beanClass, boolean shouldRegistryManageLifecycle) { + return new LazilyInitializedManagedBeanImpl<>( beanClass, + Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) ); } @Override - protected ManagedBean createBean(String beanName, Class beanContract) { - return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract, StandardCdiLifecycleManagementStrategy.INSTANCE ); + protected ManagedBean createBean(String beanName, Class beanContract, boolean shouldRegistryManageLifecycle) { + return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract, + Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiStandardImpl.java index 4a6263f37c44..9a32904ce35b 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiStandardImpl.java @@ -34,12 +34,14 @@ private ManagedBeanRegistryCdiStandardImpl(BeanManager beanManager) { } @Override - protected ManagedBean createBean(Class beanClass) { - return JpaCdiLifecycleManagementStrategy.INSTANCE.createBean( beanManager, beanClass ); + protected ManagedBean createBean(Class beanClass, boolean shouldRegistryManageLifecycle) { + return Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) + .createBean( beanManager, beanClass ); } @Override - protected ManagedBean createBean(String beanName, Class beanContract) { - return StandardCdiLifecycleManagementStrategy.INSTANCE.createBean( beanManager, beanName, beanContract ); + protected ManagedBean createBean(String beanName, Class beanContract, boolean shouldRegistryManageLifecycle) { + return Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) + .createBean( beanManager, beanName, beanContract ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryDirectImpl.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryDirectImpl.java index 1a54714b7675..c9dd54c2b0eb 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryDirectImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryDirectImpl.java @@ -23,12 +23,15 @@ public ManagedBeanRegistryDirectImpl() { } @Override - protected ManagedBean createBean(Class beanClass) { + protected ManagedBean createBean(Class beanClass, boolean shouldRegistryManageLifecycle) { return new DirectInstantiationManagedBeanImpl<>( beanClass ); } @Override - protected ManagedBean createBean(String beanName, Class beanContract) { + protected ManagedBean createBean( + String beanName, + Class beanContract, + boolean shouldRegistryManageLifecycle) { return new DirectInstantiationManagedBeanImpl<>( beanContract ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/StandardCdiLifecycleManagementStrategy.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/StandardCdiLifecycleManagementStrategy.java index 33dda2d715e2..5a5350f7828b 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/StandardCdiLifecycleManagementStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/internal/StandardCdiLifecycleManagementStrategy.java @@ -6,12 +6,21 @@ */ package org.hibernate.resource.beans.internal; -import javax.enterprise.context.spi.CreationalContext; -import javax.enterprise.inject.spi.Bean; +import javax.enterprise.inject.Instance; import javax.enterprise.inject.spi.BeanManager; import org.hibernate.resource.beans.spi.ManagedBean; +/** + * A {@link CdiLifecycleManagementStrategy} to use when CDI compliance is required + * (i.e. when the bean lifecycle is to be managed by the CDI runtime, not the JPA runtime). + * + * The main characteristic of this strategy is that every create/destroy operation is delegated + * to the CDI runtime. + * + * In particular, @Singleton-scoped or @ApplicationScoped beans are retrieved from the CDI context, + * and are not duplicated, in contrast to {@link JpaCdiLifecycleManagementStrategy}. + */ class StandardCdiLifecycleManagementStrategy implements CdiLifecycleManagementStrategy { static final StandardCdiLifecycleManagementStrategy INSTANCE = new StandardCdiLifecycleManagementStrategy(); @@ -22,38 +31,30 @@ private StandardCdiLifecycleManagementStrategy() { @Override public ManagedBean createBean(BeanManager beanManager, Class beanClass) { - Bean bean = Helper.INSTANCE.getBean( beanClass, beanManager ); - - // Pass the bean to createCreationalContext here so that an existing instance can be returned - CreationalContext creationalContext = beanManager.createCreationalContext( bean ); + Instance instance = beanManager.createInstance().select( beanClass ); + T beanInstance = instance.get(); - T beanInstance = bean.create( creationalContext ); - - return new BeanManagerManagedBeanImpl<>( beanClass, creationalContext, beanInstance ); + return new BeanManagerManagedBeanImpl<>( beanClass, instance, beanInstance ); } @Override public ManagedBean createBean(BeanManager beanManager, String beanName, Class beanClass) { - Bean bean = Helper.INSTANCE.getNamedBean( beanName, beanClass, beanManager ); - - // Pass the bean to createCreationalContext here so that an existing instance can be returned - CreationalContext creationalContext = beanManager.createCreationalContext( bean ); - - T beanInstance = bean.create( creationalContext ); + Instance instance = beanManager.createInstance().select( beanClass, new NamedBeanQualifier( beanName ) ); + T beanInstance = instance.get(); - return new BeanManagerManagedBeanImpl<>( beanClass, creationalContext, beanInstance ); + return new BeanManagerManagedBeanImpl<>( beanClass, instance, beanInstance ); } private static class BeanManagerManagedBeanImpl implements ManagedBean { private final Class beanClass; - private final CreationalContext creationContext; + private final Instance instance; private final T beanInstance; private BeanManagerManagedBeanImpl( Class beanClass, - CreationalContext creationContext, T beanInstance) { + Instance instance, T beanInstance) { this.beanClass = beanClass; - this.creationContext = creationContext; + this.instance = instance; this.beanInstance = beanInstance; } @@ -69,7 +70,7 @@ public T getBeanInstance() { @Override public void release() { - creationContext.release(); + instance.destroy( beanInstance ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/resource/beans/spi/AbstractManagedBeanRegistry.java b/hibernate-core/src/main/java/org/hibernate/resource/beans/spi/AbstractManagedBeanRegistry.java index 5505fc345915..4b3f730ee4ce 100644 --- a/hibernate-core/src/main/java/org/hibernate/resource/beans/spi/AbstractManagedBeanRegistry.java +++ b/hibernate-core/src/main/java/org/hibernate/resource/beans/spi/AbstractManagedBeanRegistry.java @@ -27,7 +27,7 @@ public ManagedBean getBean(Class beanClass, boolean shouldRegistryMana return getCacheableBean( beanClass ); } else { - return createBean( beanClass ); + return createBean( beanClass, shouldRegistryManageLifecycle ); } } @@ -39,13 +39,13 @@ protected ManagedBean getCacheableBean(Class beanClass) { return existing; } - final ManagedBean bean = createBean( beanClass ); + final ManagedBean bean = createBean( beanClass, true ); registrations.put( beanName, bean ); return bean; } @SuppressWarnings("WeakerAccess") - protected abstract ManagedBean createBean(Class beanClass); + protected abstract ManagedBean createBean(Class beanClass, boolean shouldRegistryManageLifecycle); @Override public ManagedBean getBean(String beanName, Class beanContract, boolean shouldRegistryManageLifecycle) { @@ -53,7 +53,7 @@ public ManagedBean getBean(String beanName, Class beanContract, boolea return getCacheableBean( beanName, beanContract ); } else { - return createBean( beanName, beanContract ); + return createBean( beanName, beanContract, shouldRegistryManageLifecycle ); } } @@ -64,13 +64,13 @@ protected ManagedBean getCacheableBean(String beanName, Class beanCon return existing; } - final ManagedBean bean = createBean( beanName, beanContract ); + final ManagedBean bean = createBean( beanName, beanContract, false ); registrations.put( beanName, bean ); return bean; } @SuppressWarnings("WeakerAccess") - protected abstract ManagedBean createBean(String beanName, Class beanContract); + protected abstract ManagedBean createBean(String beanName, Class beanContract, boolean shouldRegistryManageLifecycle); @SuppressWarnings("WeakerAccess") protected void forEachBean(Consumer> consumer) {