>Both works

That's exactly where I disagree. While both 'work' in the sample, immediately 
destroying the instances again after creating them - and only later returning 
the java reference to the now 'dead' EntityManagerResolver - is broken if you 
will use it in productive scenarios. 


Either we fix this, or we don't need any special handling. In this case I 
suggest to just use DependentProvider.get() for all cases. It has no harm on 
NormalScoped instances anyway.

I will guard DependentProvider#destroy to not have any impact on @Dependent 
scoped instances.


LieGrue,
strub


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

Reply via email to