>If you don't destroy it you'll likely leak. Yes, fully agree. But the way we do it is still wrong. IF it is a @Dependent scoped instance, then we must store the DependentProvider<EntityManager> somewhere and only invoke it's destroy() method once we really need.
For NormalScoped beans you are relieved of this burden, but for @Dependent ones you need to handle it manually. The DependentProvider just helps to easily store the CreationalContext, the Bean<T> and the instance for convenience reasons. LieGrue, strub >________________________________ > From: Romain Manni-Bucau <rmannibu...@gmail.com> >To: "dev@deltaspike.apache.org" <dev@deltaspike.apache.org>; Mark Struberg ><strub...@yahoo.de> >Sent: Thursday, 10 October 2013, 9:07 >Subject: Re: git commit: DELTASPIKE-424 taking into account >EntityManagerResolver with a normal scope > > >If you don't destroy it you'll likely leak. > >And once again you are in your case where you handle the emf yourself. In >other cases @Dependent should work fine. > >That said nothing again preventing using @Dependent so just do it If it >solves the issue. Normal scopes were the important part for me. > >*Romain Manni-Bucau* >*Twitter: @rmannibucau <https://twitter.com/rmannibucau>* >*Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/> >*LinkedIn: **http://fr.linkedin.com/in/rmannibucau* >*Github: https://github.com/rmannibucau* > > > > >2013/10/10 Mark Struberg <strub...@yahoo.de> > >> >> >Both works >> >> That's exactly where I disagree. While both 'work' in the sample, >> immediately destroying the instances again after creating them - and only >> later returning the java reference to the now 'dead' EntityManagerResolver >> - is broken if you will use it in productive scenarios. >> >> >> Either we fix this, or we don't need any special handling. In this case I >> suggest to just use DependentProvider.get() for all cases. It has no harm >> on NormalScoped instances anyway. >> >> I will guard DependentProvider#destroy to not have any impact on >> @Dependent scoped instances. >> >> >> LieGrue, >> strub >> >> >> >________________________________ >> > From: Romain Manni-Bucau <rmannibu...@gmail.com> >> >To: "dev@deltaspike.apache.org" <dev@deltaspike.apache.org>; Mark >> Struberg <strub...@yahoo.de> >> >Sent: Thursday, 10 October 2013, 8:33 >> >Subject: Re: git commit: DELTASPIKE-424 taking into account >> EntityManagerResolver with a normal scope >> > >> > >> > >> >Both works Mark depending as you said from where you take your em. We >> just need to be explicit on this behavior. BTW I prefer the normal scope >> usage too. >> > >> > >> >Romain Manni-Bucau >> >Twitter: @rmannibucau >> >Blog: http://rmannibucau.wordpress.com/ >> >LinkedIn: http://fr.linkedin.com/in/rmannibucau >> >Github: https://github.com/rmannibucau >> > >> > >> > >> > >> >2013/10/10 Mark Struberg <strub...@yahoo.de> >> > >> >Not sure if we really need this flag. >> >>The most important question to me is _when_ do we trigger the destroy() >> method. >> >> >> >>The following code doesn't make much sense imo: >> >> >> >>> final DependentProvider<? extends EntityManagerResolver> resolver = >> lookupResolver(emrc); >> >>> result = resolver.get().resolveEntityManager(); >> >>> resolver.destroy(); >> >> >> >> >> >>The DependentProvider is intended to store it's instances somewhere to >> be able to properly destroy the created @Dependent instance once you don't >> need it anymore. Invoking destroy() immediately but returning the created >> contextual instance makes no sense imo. Imagine what happens if you would >> close the EntityManagerFactory in a @PreDestroy method. >> >> >> >>If there is no clean way to clean up the instance again, then we should >> only rely on NormalScoped instances and remove the handling (and get the >> warnings again, because they make sense). >> >> >> >>LieGrue, >> >>strub >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >>----- Original Message ----- >> >>> From: "rmannibu...@apache.org" <rmannibu...@apache.org> >> >>> To: comm...@deltaspike.apache.org >> >>> Cc: >> >>> Sent: Wednesday, 9 October 2013, 16:46 >> >>> Subject: git commit: DELTASPIKE-424 taking into account >> EntityManagerResolver with a normal scope >> >>> >> >>> Updated Branches: >> >>> refs/heads/master bdc9cdce6 -> e8148110e >> >>> >> >>> >> >>> DELTASPIKE-424 taking into account EntityManagerResolver with a normal >> scope >> >>> >> >>> >> >>> Project: http://git-wip-us.apache.org/repos/asf/deltaspike/repo >> >>> Commit: >> http://git-wip-us.apache.org/repos/asf/deltaspike/commit/e8148110 >> >>> Tree: http://git-wip-us.apache.org/repos/asf/deltaspike/tree/e8148110 >> >>> Diff: http://git-wip-us.apache.org/repos/asf/deltaspike/diff/e8148110 >> >>> >> >>> Branch: refs/heads/master >> >>> Commit: e8148110ea2458fa2244a439583da0f2adddb482 >> >>> Parents: bdc9cdc >> >>> Author: Romain Manni-Bucau <rmannibu...@apache.org> >> >>> Authored: Wed Oct 9 16:46:06 2013 +0200 >> >>> Committer: Romain Manni-Bucau <rmannibu...@apache.org> >> >>> Committed: Wed Oct 9 16:46:06 2013 +0200 >> >>> >> >>> ---------------------------------------------------------------------- >> >>> .../data/impl/RepositoryExtension.java | 2 +- >> >>> .../data/impl/handler/EntityManagerLookup.java | 20 +++++++++++------ >> >>> .../data/impl/meta/RepositoryComponent.java | 23 >> +++++++++++++++++++- >> >>> .../data/impl/meta/RepositoryComponents.java | 16 ++++++++------ >> >>> .../data/impl/builder/part/QueryRootTest.java | 5 ++--- >> >>> 5 files changed, 47 insertions(+), 19 deletions(-) >> >>> ---------------------------------------------------------------------- >> >>> >> >>> >> >>> >> http://git-wip-us.apache.org/repos/asf/deltaspike/blob/e8148110/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/RepositoryExtension.java >> >>> ---------------------------------------------------------------------- >> >>> diff --git >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/RepositoryExtension.java >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/RepositoryExtension.java >> >>> index 076393b..8ba0dca 100755 >> >>> --- >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/RepositoryExtension.java >> >>> +++ >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/RepositoryExtension.java >> >>> @@ -72,7 +72,7 @@ public class RepositoryExtension implements Extension >> >>> { >> >>> log.log(Level.FINER, "getHandlerClass: Repository >> >>> annotation detected on {0}", >> >>> event.getAnnotatedType()); >> >>> - RepositoryComponentsFactory.instance().add(repoClass); >> >>> + RepositoryComponentsFactory.instance().add(repoClass, >> >>> beanManager); >> >>> } >> >>> catch (RepositoryDefinitionException e) >> >>> { >> >>> >> >>> >> http://git-wip-us.apache.org/repos/asf/deltaspike/blob/e8148110/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/handler/EntityManagerLookup.java >> >>> ---------------------------------------------------------------------- >> >>> diff --git >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/handler/EntityManagerLookup.java >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/handler/EntityManagerLookup.java >> >>> index 5548f16..4554497 100644 >> >>> --- >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/handler/EntityManagerLookup.java >> >>> +++ >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/handler/EntityManagerLookup.java >> >>> @@ -36,15 +36,22 @@ public class EntityManagerLookup >> >>> @Any >> >>> private Instance<EntityManager> entityManager; >> >>> >> >>> - public EntityManager lookupFor(RepositoryComponent repository) >> >>> + public EntityManager lookupFor(final RepositoryComponent >> repository) >> >>> { >> >>> EntityManager result = null; >> >>> if (repository.hasEntityManagerResolver()) >> >>> { >> >>> - DependentProvider<? extends EntityManagerResolver> >> resolver = >> >>> - >> lookupResolver(repository.getEntityManagerResolverClass()); >> >>> - result = resolver.get().resolveEntityManager(); >> >>> - resolver.destroy(); >> >>> + final Class<? extends EntityManagerResolver> emrc = >> >>> repository.getEntityManagerResolverClass(); >> >>> + if (!repository.isEntityManagerResolverIsNormalScope()) >> >>> + { >> >>> + final DependentProvider<? extends >> EntityManagerResolver> >> >>> resolver = lookupResolver(emrc); >> >>> + result = resolver.get().resolveEntityManager(); >> >>> + resolver.destroy(); >> >>> + } >> >>> + else >> >>> + { >> >>> + result = >> >>> BeanProvider.getContextualReference(emrc).resolveEntityManager(); >> >>> + } >> >>> } >> >>> else >> >>> { >> >>> @@ -60,7 +67,6 @@ public class EntityManagerLookup >> >>> private DependentProvider<? extends EntityManagerResolver> >> >>> lookupResolver( >> >>> Class<? extends EntityManagerResolver> resolverClass) >> >>> { >> >>> - DependentProvider<? extends EntityManagerResolver> resolver = >> >>> BeanProvider.getDependent(resolverClass); >> >>> - return resolver; >> >>> + return BeanProvider.getDependent(resolverClass); >> >>> } >> >>> } >> >>> >> >>> >> http://git-wip-us.apache.org/repos/asf/deltaspike/blob/e8148110/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponent.java >> >>> ---------------------------------------------------------------------- >> >>> diff --git >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponent.java >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponent.java >> >>> index c2dc466..5acee4e 100644 >> >>> --- >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponent.java >> >>> +++ >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponent.java >> >>> @@ -19,6 +19,7 @@ >> >>> package org.apache.deltaspike.data.impl.meta; >> >>> >> >>> import java.io.Serializable; >> >>> +import java.lang.annotation.Annotation; >> >>> import java.lang.reflect.Method; >> >>> import java.util.Arrays; >> >>> import java.util.Collection; >> >>> @@ -29,6 +30,8 @@ import java.util.Set; >> >>> import java.util.logging.Level; >> >>> import java.util.logging.Logger; >> >>> >> >>> +import javax.enterprise.inject.spi.Bean; >> >>> +import javax.enterprise.inject.spi.BeanManager; >> >>> import javax.persistence.FlushModeType; >> >>> >> >>> import org.apache.deltaspike.data.api.EntityManagerConfig; >> >>> @@ -53,11 +56,12 @@ public class RepositoryComponent >> >>> private final Class<?> repoClass; >> >>> private final RepositoryEntity entityClass; >> >>> private final Class<? extends EntityManagerResolver> >> >>> entityManagerResolver; >> >>> + private final boolean entityManagerResolverIsNormalScope; >> >>> private final FlushModeType entityManagerFlushMode; >> >>> >> >>> private final Map<Method, RepositoryMethod> methods = new >> >>> HashMap<Method, RepositoryMethod>(); >> >>> >> >>> - public RepositoryComponent(Class<?> repoClass, RepositoryEntity >> >>> entityClass) >> >>> + public RepositoryComponent(Class<?> repoClass, RepositoryEntity >> >>> entityClass, BeanManager beanManager) >> >>> { >> >>> if (entityClass == null) >> >>> { >> >>> @@ -67,9 +71,26 @@ public class RepositoryComponent >> >>> this.entityClass = entityClass; >> >>> this.entityManagerResolver = >> extractEntityManagerResolver(repoClass); >> >>> this.entityManagerFlushMode = >> extractEntityManagerFlushMode(repoClass); >> >>> + >> >>> + if (entityManagerResolver != null && beanManager != null) >> >>> + { >> >>> + final Set<Bean<?>> beans = >> >>> beanManager.getBeans(entityManagerResolver); >> >>> + final Class<? extends Annotation> scope = >> >>> beanManager.resolve(beans).getScope(); >> >>> + entityManagerResolverIsNormalScope = >> >>> beanManager.isNormalScope(scope); >> >>> + } >> >>> + else >> >>> + { >> >>> + entityManagerResolverIsNormalScope = false; >> >>> + } >> >>> + >> >>> initialize(); >> >>> } >> >>> >> >>> + public boolean isEntityManagerResolverIsNormalScope() >> >>> + { >> >>> + return entityManagerResolverIsNormalScope; >> >>> + } >> >>> + >> >>> public String getEntityName() >> >>> { >> >>> return EntityUtils.entityName(entityClass.getEntityClass()); >> >>> >> >>> >> http://git-wip-us.apache.org/repos/asf/deltaspike/blob/e8148110/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponents.java >> >>> ---------------------------------------------------------------------- >> >>> diff --git >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponents.java >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponents.java >> >>> index e6ded42..1bc12db 100644 >> >>> --- >> >>> >> a/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponents.java >> >>> +++ >> >>> >> b/deltaspike/modules/data/impl/src/main/java/org/apache/deltaspike/data/impl/meta/RepositoryComponents.java >> >>> @@ -18,6 +18,12 @@ >> >>> */ >> >>> package org.apache.deltaspike.data.impl.meta; >> >>> >> >>> +import org.apache.deltaspike.data.impl.RepositoryDefinitionException; >> >>> +import >> >>> >> org.apache.deltaspike.data.impl.meta.extractor.AnnotationMetadataExtractor; >> >>> +import >> org.apache.deltaspike.data.impl.meta.extractor.MetadataExtractor; >> >>> +import >> org.apache.deltaspike.data.impl.meta.extractor.TypeMetadataExtractor; >> >>> + >> >>> +import javax.enterprise.inject.spi.BeanManager; >> >>> import java.io.Serializable; >> >>> import java.lang.reflect.Method; >> >>> import java.util.Arrays; >> >>> @@ -25,11 +31,6 @@ import java.util.HashMap; >> >>> import java.util.List; >> >>> import java.util.Map; >> >>> >> >>> -import org.apache.deltaspike.data.impl.RepositoryDefinitionException; >> >>> -import >> >>> >> org.apache.deltaspike.data.impl.meta.extractor.AnnotationMetadataExtractor; >> >>> -import >> org.apache.deltaspike.data.impl.meta.extractor.MetadataExtractor; >> >>> -import >> org.apache.deltaspike.data.impl.meta.extractor.TypeMetadataExtractor; >> >>> - >> >>> /** >> >>> * Convenience class to access Repository and Repository method meta >> data. >> >>> * Acts as repository for Repository related meta data. >> >>> @@ -47,13 +48,14 @@ public class RepositoryComponents implements >> Serializable >> >>> /** >> >>> * Add a Repository class to the meta data repository. >> >>> * >> >>> + * >> >>> * @param repoClass The repo class. >> >>> * @return {@code true} if Repository class has been added, >> >>> {@code false} otherwise. >> >>> */ >> >>> - public void add(Class<?> repoClass) >> >>> + public void add(Class<?> repoClass, BeanManager bm) >> >>> { >> >>> RepositoryEntity entityClass = >> extractEntityMetaData(repoClass); >> >>> - RepositoryComponent repo = new RepositoryComponent(repoClass, >> >>> entityClass); >> >>> + RepositoryComponent repo = new RepositoryComponent(repoClass, >> >>> entityClass, bm); >> >>> repos.put(repoClass, repo); >> >>> } >> >>> >> >>> >> >>> >> http://git-wip-us.apache.org/repos/asf/deltaspike/blob/e8148110/deltaspike/modules/data/impl/src/test/java/org/apache/deltaspike/data/impl/builder/part/QueryRootTest.java >> >>> ---------------------------------------------------------------------- >> >>> diff --git >> >>> >> a/deltaspike/modules/data/impl/src/test/java/org/apache/deltaspike/data/impl/builder/part/QueryRootTest.java >> >>> >> b/deltaspike/modules/data/impl/src/test/java/org/apache/deltaspike/data/impl/builder/part/QueryRootTest.java >> >>> index f904bb2..1c70466 100644 >> >>> --- >> >>> >> a/deltaspike/modules/data/impl/src/test/java/org/apache/deltaspike/data/impl/builder/part/QueryRootTest.java >> >>> +++ >> >>> >> b/deltaspike/modules/data/impl/src/test/java/org/apache/deltaspike/data/impl/builder/part/QueryRootTest.java >> >>> @@ -30,9 +30,8 @@ import org.junit.Test; >> >>> >> >>> public class QueryRootTest >> >>> { >> >>> - >> >>> - private final RepositoryComponent repo = new >> >>> RepositoryComponent(SimpleRepository.class, new >> RepositoryEntity(Simple.class, >> >>> Long.class)); >> >>> - private final RepositoryComponent repoFetchBy = new >> >>> RepositoryComponent(SimpleFetchRepository.class, new >> >>> RepositoryEntity(Simple.class, Long.class)); >> >>> + private final RepositoryComponent repo = new >> >>> RepositoryComponent(SimpleRepository.class, new >> RepositoryEntity(Simple.class, >> >>> Long.class), null); >> >>> + private final RepositoryComponent repoFetchBy = new >> >>> RepositoryComponent(SimpleFetchRepository.class, new >> >>> RepositoryEntity(Simple.class, Long.class), null); >> >>> >> >>> @Test >> >>> public void should_create_simple_query() >> >>> >> >> >> > >> > >> > >> > > >
