@mark: +1 regards, gerhard
2013/10/11 Mark Struberg <strub...@yahoo.de> > > >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() > >> >>> > >> >> > >> > > >> > > >> > > >> > > > > > > >
