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

2013-10-10 Thread Romain Manni-Bucau
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 *
*Blog: **http://rmannibucau.wordpress.com/*
*LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
*Github: https://github.com/rmannibucau*



2013/10/10 Mark Struberg 

>
> >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 
> >To: "dev@deltaspike.apache.org" ; Mark
> Struberg 
> >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 
> >
> >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 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" 
> >>> 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 
> >>> Authored: Wed Oct 9 16:46:06 2013 +0200
> >>> Committer: Romain Manni-Bucau 
> >>> 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

[jira] [Created] (DELTASPIKE-425) DependentProvider#destroy() must not destroy NormalScoped instances

2013-10-10 Thread Mark Struberg (JIRA)
Mark Struberg created DELTASPIKE-425:


 Summary: DependentProvider#destroy() must not destroy NormalScoped 
instances
 Key: DELTASPIKE-425
 URL: https://issues.apache.org/jira/browse/DELTASPIKE-425
 Project: DeltaSpike
  Issue Type: Improvement
  Components: Core
Affects Versions: 0.5
Reporter: Mark Struberg
Assignee: Mark Struberg
 Fix For: 0.6


Our DependentProvider is intended to hold information (the Bean, the contextual 
instance and the CreationalContext) of @Dependent scoped instances. This allows 
us to later invoke #destroy() which will properly cleanup this instance (mainly 
calling @PreDestroy if available).

For NormalScoped instances we must not perform any action in destroy as 
NormalScoped beans always have their own lifecycle which we must not disrupt. 
Which means the DependentProvider#destroy() method must detect whether the 
provided instance is NormalScoped and skip any action in this case.



--
This message was sent by Atlassian JIRA
(v6.1#6144)