Hi Daniel, Please see inline.
On Mon, Mar 21, 2016 at 10:32 AM, Daniel Stoch <[email protected]> wrote: > Hi, > > Your patch allows to attach custom implementation of > getImplementations() method so we can solve issues with IInitializers > initialization under OSGi. So Wicket itself does not solve these > problems but it gives an extension point where we can plug-in a proper > solution. So I think it is ok. > One question: is it safe to call IInitializer.init() method more than > once for application? If not then maybe > CompoundClassResolver.getImplementations() should perform check for > duplicates (it is possible that more than one IClassResolver returns > the same IInitializer). > It is not safe. We never needed to deal with such use case before. > > We are not using WAR packaged application but services (as Łukasz > wrote in his mail). So in such case there is no a single ClassLoader > which can return all IInitializers. I think the whole concept of such > static initialization fails under dynamic environment (like OSGi) where you can start/stop modules during application lifecycle and you > cannot guarantee that all modules will be loaded before > Application.init() call. So in OSGi a better solution would be to > register/unregister IInitializers using services. > It is safe to init+destroy several times though. IInitializer#destroy() should revert whatever it did in #init(). In non-dynamic environment (normal Servlet container) the initializers are called before YourApp#init(), so the application can "override" whatever a 3rd party library's IInitializer did. In dynamic environment (like OSGi) the module might be registered after MyApp#init() has been executed, so the application won't be able to revert. In dynamic environment, I think, each bundle should initialize just its own IInitializer and should not deal with others bundles' IInitializers. > The simplest solution could be not to use IInitializers concept in > custom modules and for standard Wicket modules add their initializers > directly in code of getImplementations() method in our custom > IClassResolver implementation. > Now I think there is no need of IClassResolver#getImplementations() at all. In OSGi environment during the app start ServiceLoader will find just wicket-core's IInitializer and will execute it. The IInitializers by other bundles (like wicket-extensions) will have to be executed "manually" when they are loaded/started/activated. I guess you will have to check what PAX-Wicket does and use it. > > PS1. Your patch is for master (8.x), so it needs adaptation for 6.x ;). > Yes. New development goes in 8.x these days :-) > PS2. wicket-6.22.0 tag is probably missing in git (on GitHub). > Yes, you are right! But this is out of my control. > > Thanks for your patience! > > -- > Best regards, > Daniel Stoch > > On Wed, Mar 16, 2016 at 9:23 PM, Martin Grigorov <[email protected]> > wrote: > > Thanks, Lukasz! > > > > > https://git1-us-west.apache.org/repos/asf?p=wicket.git;a=commit;h=36cb5824 > > > > @Daniel: Could you please take a look at the suggestion and comment > whether > > it will work ? Or even better test it before 6.23.0 is out. > > Thanks! > > > > Martin Grigorov > > Wicket Training and Consulting > > https://twitter.com/mtgrigorov > > > > On Wed, Mar 16, 2016 at 5:54 PM, Łukasz Dywicki <[email protected]> > wrote: > > > >> Martin, > >> I think it will work, but to be extra safe I would pass application > class > >> loader to service loader call. In case of OSGi webapp lifecycle may be > >> affected if it’s not packaged as a WAR but installed as JAR registering > >> servlet. In case of bundle/jar deployment activation/bootstrap of module > >> will start with context of mechanism starting it. For example it can be > >> aries or spring startup handler which parses configuration before > >> application is reached by Jetty/Tomcat. This is quite opposite to > typical > >> webapp start where container already assembled classpath and > configuration > >> is started inside a WAR by filter or lifecycle listener. > >> > >> Cheers, > >> Łukasz > >> — > >> Apache Karaf Committer & PMC > >> Twitter: ldywicki > >> Blog: http://dywicki.pl > >> Code-House - http://code-house.org > >> > >> > >> > Wiadomość napisana przez Martin Grigorov <[email protected]> w > dniu > >> 16 mar 2016, o godz. 17:38: > >> > > >> > Hi Lukasz, > >> > > >> > Thanks for helping! > >> > > >> > The ServiceLoader impl is in DefaultClassResolver which is used *by > >> > default*, i.e. in normal servlet containers. > >> > In OSGi environments the application developer could provide > >> > OsgiClassResolver that will use OSGi APIs to find the impls of a class > >> > (IInitializer in this case). Would that work ? > >> > > >> > Martin Grigorov > >> > Wicket Training and Consulting > >> > https://twitter.com/mtgrigorov > >> > > >> > On Wed, Mar 16, 2016 at 5:24 PM, Łukasz Dywicki <[email protected]> > >> wrote: > >> > > >> >> Martin, > >> >> As far I know it will not help, cause of classloader visibility. > >> >> ServiceLoader call without given context will use TCCL which could be > >> >> everything but app classloader during initialization. Making service > >> loader > >> >> working properly under OSGi requires some extra steps (ie. via > >> >> aries-spi-fly or geronimo-osgi-locator/registry). > >> >> > >> >> Long time ago I’ve written an improved osgi integration for wicket > which > >> >> is much simpler than pax-wicket. It is something between wicket stuff > >> and > >> >> pax: > >> >> https://github.com/Code-House/wicket-osgi < > >> >> https://github.com/Code-House/wicket-osgi> > >> >> > >> >> It is based on initializers. :-) > >> >> > >> >> Best regards, > >> >> Lukasz > >> >> — > >> >> [email protected] > >> >> Twitter: ldywicki > >> >> Blog: http://dywicki.pl > >> >> Code-House - http://code-house.org > >> >> > >> >>> Wiadomość napisana przez Martin Grigorov <[email protected]> w > dniu > >> >> 14 mar 2016, o godz. 22:51: > >> >>> > >> >>> Hi, > >> >>> > >> >>> With [1] I've made a change in Wicket to use IClassResolver to find > all > >> >>> IInitializer impls. > >> >>> Could this help for friendlier OSGi usage ? > >> >>> Is it OK to find all impls by interface class in OSGi environment ? > >> >>> > >> >>> The only problem is that you have to register OsgiClassResolver in > >> >>> MyApplication#internalInit(), not in #init(). > >> >>> > >> >>> 1. > >> >>> > >> >> > >> > https://git1-us-west.apache.org/repos/asf?p=wicket.git;a=commit;h=39f897aa > >> >>> > >> >>> Martin Grigorov > >> >>> Wicket Training and Consulting > >> >>> https://twitter.com/mtgrigorov > >> >>> > >> >>> On Mon, Feb 29, 2016 at 7:35 PM, Łukasz Dywicki < > [email protected]> > >> >> wrote: > >> >>> > >> >>>> FYI there is pax-wicket project which is intended to support OSGi > >> based > >> >>>> deployments for Wicket. > >> >>>> > >> >>>> Cheers, > >> >>>> Lukasz > >> >>>> — > >> >>>> [email protected] > >> >>>> Twitter: ldywicki > >> >>>> Blog: http://dywicki.pl > >> >>>> Code-House - http://code-house.org > >> >>>> > >> >>>>> Wiadomość napisana przez Daniel Stoch <[email protected]> w > >> dniu > >> >>>> 23 lut 2016, o godz. 15:01: > >> >>>>> > >> >>>>> On Tue, Feb 23, 2016 at 2:33 PM, Martin Grigorov < > >> [email protected] > >> >>> > >> >>>> wrote: > >> >>>>> > >> >>>>>> > >> >>>>>> Please check > >> >>>>>> > >> >>>> > >> >> > >> > https://github.com/wicketstuff/core/blob/master/wicket-osgi-parent/wicket-osgi/src/main/java/org/wicketstuff/osgi/OsgiClassResolver.java > >> >>>>>> (or its wicket-6.x version). > >> >>>>> > >> >>>>> But I think this implementation is incorrect and it can work only > >> >>>>> under special conditions. It assumes that class of my application > has > >> >>>>> access to all classes: so bundle should contains: > >> >>>>> DynamicImport-Package: *. > >> >>>>> But even when I add such line to MANIFEST.MF with my application > >> >>>>> bundle, the situation is the same like using my own > implementation of > >> >>>>> IClassResolver: some IInitializers are not found at all. > >> >>>>> > >> >>>>> So even with OsgiClassResolver it does not work as it is designed. > >> >>>>> > >> >>>>> -- > >> >>>>> Daniel > >> >>>> > >> >>>> > >> >> > >> >> > >> > >> >
