This one has also been open a while - I'll get this merged in unless any one has any objections. If you have any feedback, please shout!
Jon On Mon, Jul 31, 2017 at 11:04 AM, Jonathan Gallimore < [email protected]> wrote: > Hi folks, > > Do let me know if there's any objection to these PRs, if not, I'll get > them merged in. > > Thanks > > Jon > > On Tue, Jul 25, 2017 at 5:05 PM, Jonathan Gallimore < > [email protected]> wrote: > >> Sorry for the delay.... >> >> I've had a go at updating these 2 PRs to capture the >> PostConstruct/PreDestroy methods on ResourceInfo. Any feedback is most >> welcome. >> >> https://github.com/apache/tomee/pull/91 >> https://github.com/apache/tomee/pull/92 >> >> Thanks for all the feedback so far. >> >> Jon >> >> On Thu, Jul 13, 2017 at 4:34 PM, Jonathan Gallimore < >> [email protected]> wrote: >> >>> My initial thought was it sounds like a big change too. Having thought >>> about it, I do agree with Romain that its probably do-able without too much >>> hassle on 7.x and 1.7.x. I'll give it a shot. >>> >>> Jon >>> >>> On Thu, Jul 13, 2017 at 2:34 PM, Romain Manni-Bucau < >>> [email protected]> wrote: >>> >>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog >>>> <https://blog-rmannibucau.rhcloud.com> | Old Blog >>>> <http://rmannibucau.wordpress.com> | Github < >>>> https://github.com/rmannibucau> | >>>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory >>>> <https://javaeefactory-rmannibucau.rhcloud.com> >>>> >>>> 2017-07-13 15:28 GMT+02:00 Mark Struberg <[email protected]>: >>>> >>>> > Sounds way more straight forward. But is probably a bigger change, >>>> isn't? >>>> > >>>> >>>> No, would be just a matter of creating a class with a list in it (guess >>>> we >>>> can), add an instance to AppContext (guess we can) and register at >>>> deploy >>>> time resource instances in this list (same time we register the id >>>> already) >>>> and use that to destroy the instances instead of lookups. >>>> >>>> Really sounds doable even on 1.x ;) >>>> >>>> What can wait tomee 8 is to do it for all instances including resource >>>> adapters, connectors, etc... >>>> >>>> >>>> > So probably do a clean rewrite in TomEE8? >>>> > >>>> > LieGrue, >>>> > strub >>>> > >>>> > >>>> > > Am 13.07.2017 um 13:14 schrieb Romain Manni-Bucau < >>>> [email protected] >>>> > >: >>>> > > >>>> > > Hi Jon >>>> > > >>>> > > Side note before digging: you can rewrite the test this way (i find >>>> it >>>> > > easier to enter in but surely personal): >>>> > > https://gist.github.com/rmannibucau/86152958d9c139a45d4d80ad >>>> bcc2c653 >>>> > > >>>> > > Now about the issue: if i get it right issue is we lookup the >>>> instance >>>> > and >>>> > > therefore unwrap the holder with predestroy info. So fix is just >>>> about >>>> > > reading the raw wrapper (we don't have references of references of >>>> > > references with resource so it is safe) and just destroy the holder. >>>> > > >>>> > > Personally I'm very tempted to go away from the JNDI tree for the >>>> storage >>>> > > of these metadata and just put them in the AppContext (resources >>>> list?) >>>> > to >>>> > > avoid to mess up wrappings/unwrapping like we do today and have a >>>> > straight >>>> > > implmentation. Then the JNDI interaction is a plain unbind but the >>>> app >>>> > > resource lifecycle management is not relying on jndi by itself (like >>>> > EJBs). >>>> > > >>>> > > >>>> > > Romain Manni-Bucau >>>> > > @rmannibucau <https://twitter.com/rmannibucau> | Blog >>>> > > <https://blog-rmannibucau.rhcloud.com> | Old Blog >>>> > > <http://rmannibucau.wordpress.com> | Github <https://github.com/ >>>> > rmannibucau> | >>>> > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory >>>> > > <https://javaeefactory-rmannibucau.rhcloud.com> >>>> > > >>>> > > 2017-07-13 12:58 GMT+02:00 Jonathan Gallimore < >>>> > [email protected]> >>>> > > : >>>> > > >>>> > >> Ooo, interesting. Thanks Svetlin! I'll add a test case for that as >>>> well >>>> > and >>>> > >> adjust. >>>> > >> >>>> > >> Cheers! >>>> > >> >>>> > >> Jon >>>> > >> >>>> > >> On Thu, Jul 13, 2017 at 11:54 AM, Svetlin Zarev < >>>> > >> [email protected]> wrote: >>>> > >> >>>> > >>> Hi, >>>> > >>> >>>> > >>> I'm not sure what will happen if the IvmContext is in read-only >>>> mode >>>> > >> (i.e. >>>> > >>> openejb.forceReadOnlyAppNamingContext=true). You may-need to >>>> make the >>>> > >>> context writable before unbinding. >>>> > >>> >>>> > >>> Svetlin >>>> > >>> >>>> > >>> >>>> > >>> 2017-07-13 13:46 GMT+03:00 Jonathan Gallimore < >>>> > >>> [email protected]> >>>> > >>> : >>>> > >>> >>>> > >>>> Hey folks >>>> > >>>> >>>> > >>>> I noticed an issue when creating a resource inside an >>>> application: >>>> > >>>> >>>> > >>>> package org.superbiz; >>>> > >>>> >>>> > >>>> >>>> > >>>> import javax.annotation.PostConstruct; >>>> > >>>> import javax.annotation.PreDestroy; >>>> > >>>> import java.util.logging.Logger; >>>> > >>>> >>>> > >>>> public class Startup { >>>> > >>>> >>>> > >>>> private final Logger log = Logger.getLogger(Startup. >>>> > >>> class.getName()); >>>> > >>>> >>>> > >>>> @PostConstruct >>>> > >>>> public void start() { >>>> > >>>> log.info("*** " + getClass().getName() + " started ***"); >>>> > >>>> } >>>> > >>>> >>>> > >>>> @PreDestroy >>>> > >>>> public void stop() { >>>> > >>>> log.info("*** " + getClass().getName() + " stopped ***"); >>>> > >>>> } >>>> > >>>> >>>> > >>>> } >>>> > >>>> >>>> > >>>> and using WEB-INF/resources.xml to create the resource: >>>> > >>>> >>>> > >>>> <resources> >>>> > >>>> <Resource id="Startup" class-name="org.superbiz.Startup"> >>>> > >>>> # any properties you need can go here >>>> > >>>> </Resource> >>>> > >>>> </resources> >>>> > >>>> >>>> > >>>> It looks like my @PostConstruct is called ok, but my @PreDestroy >>>> is >>>> > >> not. >>>> > >>> I >>>> > >>>> believe this works ok with resources defined in tomee.xml, but >>>> I'm >>>> > >> happy >>>> > >>> to >>>> > >>>> check. >>>> > >>>> >>>> > >>>> I dug a bit deeper, and wrote a test. This appears to be an >>>> issue on >>>> > >> both >>>> > >>>> master and 1.7.x. I have created two PRs, and would appreciate >>>> any >>>> > >>>> thoughts. >>>> > >>>> >>>> > >>>> https://github.com/apache/tomee/pull/91 >>>> > >>>> https://github.com/apache/tomee/pull/92 >>>> > >>>> >>>> > >>>> Many thanks >>>> > >>>> >>>> > >>>> Jon >>>> > >>>> >>>> > >>> >>>> > >> >>>> > >>>> > >>>> >>> >>> >> >
