HHH-12133 Comply with API docs with respect to lifecycle management d…

…epending 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.
hibernate · Jan 10, 2018 · c4d9f32 · c4d9f32
1 parent 84b82ed
commit c4d9f32
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 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
@@ -22,24 +22,6 @@ public class Helper {
 	private Helper() {
 	}
 
-	@SuppressWarnings("unchecked")
-	public <T> Bean<T> getBean(Class<T> beanContract, BeanManager beanManager) {
-		Set<Bean<?>> 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<T>) beans.iterator().next();
-	}
-
 	@SuppressWarnings("unchecked")
 	public <T> Bean<T> getNamedBean(String beanName, Class<T> beanContract, BeanManager beanManager) {
 		Set<Bean<?>> beans = beanManager.getBeans( beanContract, new NamedBeanQualifier( beanName ) );
@@ -57,4 +39,13 @@ public <T> Bean<T> getNamedBean(String beanName, Class<T> beanContract, BeanMana
 
 		return (Bean<T>) beans.iterator().next();
 	}
+
+	public CdiLifecycleManagementStrategy getLifecycleManagementStrategy(boolean shouldRegistryManageLifecycle) {
+		if ( shouldRegistryManageLifecycle ) {
+			return JpaCdiLifecycleManagementStrategy.INSTANCE;
+		}
+		else {
+			return StandardCdiLifecycleManagementStrategy.INSTANCE;
+		}
+	}
 }
diff --git a/...rc/main/java/org/hibernate/resource/beans/internal/JpaCdiLifecycleManagementStrategy.java b/...rc/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 <T> ManagedBean<T> 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<T> implements ManagedBean<T> {
@@ -82,13 +94,14 @@ public void release() {
 
 	private static class NamedJpaManagedBeanImpl<T> implements ManagedBean<T> {
 		private final Class<T> beanClass;
+		private final Bean<T> bean;
 		private final CreationalContext<T> creationContext;
 		private final T beanInstance;
 
 		private NamedJpaManagedBeanImpl(
-				Class<T> beanClass,
-				CreationalContext<T> creationContext, T beanInstance) {
+				Class<T> beanClass, Bean<T> bean, CreationalContext<T> 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/...rc/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiDelayedImpl.java b/...rc/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiDelayedImpl.java
@@ -33,13 +33,15 @@ private ManagedBeanRegistryCdiDelayedImpl(BeanManager beanManager) {
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(Class<T> beanClass) {
-		return new LazilyInitializedManagedBeanImpl<>( beanClass, JpaCdiLifecycleManagementStrategy.INSTANCE );
+	protected <T> ManagedBean<T> createBean(Class<T> beanClass, boolean shouldRegistryManageLifecycle) {
+		return new LazilyInitializedManagedBeanImpl<>( beanClass,
+				Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) );
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract) {
-		return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract, StandardCdiLifecycleManagementStrategy.INSTANCE );
+	protected <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract, boolean shouldRegistryManageLifecycle) {
+		return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract,
+				Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) );
 	}
 
 	/**

diff --git a/...c/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiExtendedImpl.java b/...c/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiExtendedImpl.java
@@ -39,13 +39,15 @@ private ManagedBeanRegistryCdiExtendedImpl(ExtendedBeanManager beanManager) {
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(Class<T> beanClass) {
-		return new LazilyInitializedManagedBeanImpl<>( beanClass, JpaCdiLifecycleManagementStrategy.INSTANCE );
+	protected <T> ManagedBean<T> createBean(Class<T> beanClass, boolean shouldRegistryManageLifecycle) {
+		return new LazilyInitializedManagedBeanImpl<>( beanClass,
+				Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) );
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract) {
-		return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract, StandardCdiLifecycleManagementStrategy.INSTANCE );
+	protected <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract, boolean shouldRegistryManageLifecycle) {
+		return new LazilyInitializedNamedManagedBeanImpl<>( beanName, beanContract,
+				Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle ) );
 	}
 
 	@Override

diff --git a/...c/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiStandardImpl.java b/...c/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryCdiStandardImpl.java
@@ -34,12 +34,14 @@ private ManagedBeanRegistryCdiStandardImpl(BeanManager beanManager) {
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(Class<T> beanClass) {
-		return JpaCdiLifecycleManagementStrategy.INSTANCE.createBean( beanManager, beanClass );
+	protected <T> ManagedBean<T> createBean(Class<T> beanClass, boolean shouldRegistryManageLifecycle) {
+		return Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle )
+				.createBean( beanManager, beanClass );
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract) {
-		return StandardCdiLifecycleManagementStrategy.INSTANCE.createBean( beanManager, beanName, beanContract );
+	protected <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract, boolean shouldRegistryManageLifecycle) {
+		return Helper.INSTANCE.getLifecycleManagementStrategy( shouldRegistryManageLifecycle )
+				.createBean( beanManager, beanName, beanContract );
 	}
 }
diff --git a/...re/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryDirectImpl.java b/...re/src/main/java/org/hibernate/resource/beans/internal/ManagedBeanRegistryDirectImpl.java
@@ -23,12 +23,15 @@ public ManagedBeanRegistryDirectImpl() {
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(Class<T> beanClass) {
+	protected <T> ManagedBean<T> createBean(Class<T> beanClass, boolean shouldRegistryManageLifecycle) {
 		return new DirectInstantiationManagedBeanImpl<>( beanClass );
 	}
 
 	@Override
-	protected <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract) {
+	protected <T> ManagedBean<T> createBean(
+			String beanName,
+			Class<T> beanContract,
+			boolean shouldRegistryManageLifecycle) {
 		return new DirectInstantiationManagedBeanImpl<>( beanContract );
 	}
 }
diff --git a/...in/java/org/hibernate/resource/beans/internal/StandardCdiLifecycleManagementStrategy.java b/...in/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 <T> ManagedBean<T> createBean(BeanManager beanManager, Class<T> beanClass) {
-		Bean<T> bean = Helper.INSTANCE.getBean( beanClass, beanManager );
-
-		// Pass the bean to createCreationalContext here so that an existing instance can be returned
-		CreationalContext<T> creationalContext = beanManager.createCreationalContext( bean );
+		Instance<T> 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 <T> ManagedBean<T> createBean(BeanManager beanManager, String beanName, Class<T> beanClass) {
-		Bean<T> bean = Helper.INSTANCE.getNamedBean( beanName, beanClass, beanManager );
-
-		// Pass the bean to createCreationalContext here so that an existing instance can be returned
-		CreationalContext<T> creationalContext = beanManager.createCreationalContext( bean );
-
-		T beanInstance = bean.create( creationalContext );
+		Instance<T> 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<T> implements ManagedBean<T> {
 		private final Class<T> beanClass;
-		private final CreationalContext<T> creationContext;
+		private final Instance<T> instance;
 		private final T beanInstance;
 
 		private BeanManagerManagedBeanImpl(
 				Class<T> beanClass,
-				CreationalContext<T> creationContext, T beanInstance) {
+				Instance<T> 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/...nate-core/src/main/java/org/hibernate/resource/beans/spi/AbstractManagedBeanRegistry.java b/...nate-core/src/main/java/org/hibernate/resource/beans/spi/AbstractManagedBeanRegistry.java
@@ -27,7 +27,7 @@ public <T> ManagedBean<T> getBean(Class<T> beanClass, boolean shouldRegistryMana
 			return getCacheableBean( beanClass );
 		}
 		else {
-			return createBean( beanClass );
+			return createBean( beanClass, shouldRegistryManageLifecycle );
 		}
 	}
 
@@ -39,21 +39,21 @@ protected <T> ManagedBean<T> getCacheableBean(Class<T> beanClass) {
 			return existing;
 		}
 
-		final ManagedBean<T> bean = createBean( beanClass );
+		final ManagedBean<T> bean = createBean( beanClass, true );
 		registrations.put( beanName, bean );
 		return bean;
 	}
 
 	@SuppressWarnings("WeakerAccess")
-	protected abstract <T> ManagedBean<T> createBean(Class<T> beanClass);
+	protected abstract <T> ManagedBean<T> createBean(Class<T> beanClass, boolean shouldRegistryManageLifecycle);
 
 	@Override
 	public <T> ManagedBean<T> getBean(String beanName, Class<T> beanContract, boolean shouldRegistryManageLifecycle) {
 		if ( shouldRegistryManageLifecycle ) {
 			return getCacheableBean( beanName, beanContract );
 		}
 		else {
-			return createBean( beanName, beanContract );
+			return createBean( beanName, beanContract, shouldRegistryManageLifecycle );
 		}
 	}
 
@@ -64,13 +64,13 @@ protected  <T> ManagedBean<T> getCacheableBean(String beanName, Class<T> beanCon
 			return existing;
 		}
 
-		final ManagedBean<T> bean = createBean( beanName, beanContract );
+		final ManagedBean<T> bean = createBean( beanName, beanContract, false );
 		registrations.put( beanName, bean );
 		return bean;
 	}
 
 	@SuppressWarnings("WeakerAccess")
-	protected abstract <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract);
+	protected abstract <T> ManagedBean<T> createBean(String beanName, Class<T> beanContract, boolean shouldRegistryManageLifecycle);
 
 	@SuppressWarnings("WeakerAccess")
 	protected void forEachBean(Consumer<ManagedBean<?>> consumer) {