Re: @PostConstruct and @PreDestrory

[Jonathan Gallimore <jo...@gmail.com>]

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 <
jonathan.gallimore@gmail.com> 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 <
> jonathan.gallimore@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.gallimore@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 <
>>> rmannibucau@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 <st...@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 <
>>>> rmannibucau@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/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 <
>>>> > jonathan.gallimore@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.zarev@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.gallimore@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
>>>> > >>>>
>>>> > >>>
>>>> > >>
>>>> >
>>>> >
>>>>
>>>
>>>
>>
>