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 < jonathan.gallim...@gmail.com> 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 < > jonathan.gallim...@gmail.com> 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 < >> rmannibu...@gmail.com> 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 <strub...@yahoo.de.invalid>: >>> >>> > 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 < >>> rmannibu...@gmail.com >>> > >: >>> > > >>> > > 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/86152958d9c139a45d4d80adbcc2c653 >>> > > >>> > > 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 < >>> > jonathan.gallim...@gmail.com> >>> > > : >>> > > >>> > >> 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 < >>> > >> svetlin.angelov.za...@gmail.com> 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 < >>> > >>> jonathan.gallim...@gmail.com> >>> > >>> : >>> > >>> >>> > >>>> 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 >>> > >>>> >>> > >>> >>> > >> >>> > >>> > >>> >> >> >
