You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@deltaspike.apache.org by rm...@apache.org on 2013/10/09 16:46:06 UTC

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 <rm...@apache.org>
Authored: Wed Oct 9 16:46:06 2013 +0200
Committer: Romain Manni-Bucau <rm...@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()


Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Gerhard Petracek <ge...@gmail.com>.
@mark: +1

regards,
gerhard



2013/10/11 Mark Struberg <st...@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 <rm...@gmail.com>
> >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
> Struberg <st...@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 <st...@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 <rm...@gmail.com>
> >> >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
> >> Struberg <st...@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 <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
> >> >>> To: commits@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 <rm...@apache.org>
> >> >>> Authored: Wed Oct 9 16:46:06 2013 +0200
> >> >>> Committer: Romain Manni-Bucau <rm...@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()
> >> >>>
> >> >>
> >> >
> >> >
> >> >
> >>
> >
> >
> >
>

Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Romain Manni-Bucau <rm...@gmail.com>.
@Mark: if we add the constraint to not use other scoped injections it would
work...but I agree it is maybe a big constraint since the em will often be
@ReqScoped

*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/11 Karl Kildén <ka...@gmail.com>

> Hello!
>
> I have some trouble understanding why it's bad to destroy the instance with
> that example. What about this example, does it make sense?
>
>             DependentProvider<ContextControl> ctxControl =
> BeanProvider.getDependent(ContextControl.class);
>
>             ctxControl.get().startContext(ApplicationScoped.class);
>             // Do something useful in for example a Quartz Job
>
>             ctxControl.destroy();
>
> cheers
>
>
>
>
> On 11 October 2013 10:15, Mark Struberg <st...@yahoo.de> wrote:
>
> >
> > >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 <rm...@gmail.com>
> > >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
> > Struberg <st...@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 <st...@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 <rm...@gmail.com>
> > >> >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
> > >> Struberg <st...@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 <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
> > >> >>> To: commits@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 <rm...@apache.org>
> > >> >>> Authored: Wed Oct 9 16:46:06 2013 +0200
> > >> >>> Committer: Romain Manni-Bucau <rm...@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()
> > >> >>>
> > >> >>
> > >> >
> > >> >
> > >> >
> > >>
> > >
> > >
> > >
> >
>

Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Karl Kildén <ka...@gmail.com>.
Hello!

I have some trouble understanding why it's bad to destroy the instance with
that example. What about this example, does it make sense?

            DependentProvider<ContextControl> ctxControl =
BeanProvider.getDependent(ContextControl.class);

            ctxControl.get().startContext(ApplicationScoped.class);
            // Do something useful in for example a Quartz Job

            ctxControl.destroy();

cheers




On 11 October 2013 10:15, Mark Struberg <st...@yahoo.de> wrote:

>
> >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 <rm...@gmail.com>
> >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
> Struberg <st...@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 <st...@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 <rm...@gmail.com>
> >> >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
> >> Struberg <st...@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 <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
> >> >>> To: commits@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 <rm...@apache.org>
> >> >>> Authored: Wed Oct 9 16:46:06 2013 +0200
> >> >>> Committer: Romain Manni-Bucau <rm...@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()
> >> >>>
> >> >>
> >> >
> >> >
> >> >
> >>
> >
> >
> >
>

Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Mark Struberg <st...@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 <rm...@gmail.com>
>To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark Struberg <st...@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 <st...@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 <rm...@gmail.com>
>> >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
>> Struberg <st...@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 <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
>> >>> To: commits@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 <rm...@apache.org>
>> >>> Authored: Wed Oct 9 16:46:06 2013 +0200
>> >>> Committer: Romain Manni-Bucau <rm...@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()
>> >>>
>> >>
>> >
>> >
>> >
>>
>
>
>

Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <st...@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 <rm...@gmail.com>
> >To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark
> Struberg <st...@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 <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
> >>> To: commits@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 <rm...@apache.org>
> >>> Authored: Wed Oct 9 16:46:06 2013 +0200
> >>> Committer: Romain Manni-Bucau <rm...@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()
> >>>
> >>
> >
> >
> >
>

Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Mark Struberg <st...@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 <rm...@gmail.com>
>To: "dev@deltaspike.apache.org" <de...@deltaspike.apache.org>; Mark Struberg <st...@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 <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
>>> To: commits@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 <rm...@apache.org>
>>> Authored: Wed Oct 9 16:46:06 2013 +0200
>>> Committer: Romain Manni-Bucau <rm...@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()
>>>
>>
>
>
>

Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <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 <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
> > To: commits@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 <rm...@apache.org>
> > Authored: Wed Oct 9 16:46:06 2013 +0200
> > Committer: Romain Manni-Bucau <rm...@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()
> >
>

Re: git commit: DELTASPIKE-424 taking into account EntityManagerResolver with a normal scope

Posted by Mark Struberg <st...@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: "rmannibucau@apache.org" <rm...@apache.org>
> To: commits@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 <rm...@apache.org>
> Authored: Wed Oct 9 16:46:06 2013 +0200
> Committer: Romain Manni-Bucau <rm...@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()
>