On Wed, Nov 13, 2013 at 5:21 PM, Igor Vaynberg <[email protected]>wrote:
> On Wed, Nov 13, 2013 at 1:43 PM, John Sarman <[email protected]> wrote: > > So let me filter through injection of App. > > CdiWicketFilter gets injected factory. > > @Inject > > CdiWebApplicationFactory applicationFactory; > > > > the Factory get injected the following > > @Inject > > @Any > > Instance<WebApplication> applications; > > @Inject > > CdiConfiguration cdiConfiguration; > > > > If there is only one application in a war then the web.xml only needs > > <filter> > > <filter-name>CdiApplication</filter-name> > > <filter-class>org.apache.wicket.cdi.CdiWicketFilter</filter-class> > > </filter> > > if multiple then additional > > <init-param> > > <param-name>applicationName</param-name> > > <param-value>CdiExample</param-value> > > </init-param> > > and @WicketApp("CdiExample") added. > > i dont see this as an advantage. specifying a class name is trivial. > Yeah I didn't do it as simplification it just organically happened that way. Initially I used injection to find CDI Impl without needing to write code to find it. > > further you are assuming i am running inside a container that has its > filter's managed by cdi, this is not always the case so using your cdi > filter would fail. one would have to fall back on wicket-filter and > I am 100% assuming that since you just included wicket-weld in the build. Therefore you do have a managed container at that point. > specify the cdi application factory that you provide, but that class > itself assumes it is managed by cdi, so it wont work either. so you > did not leave an escape hatch for people using simple containers. > > my original code may not be "cdi-pretty" but it works for all > containers out there - cdi managed or not. I couldn't have got anywhere without your code. I was pretty to me m_BLAH m_BLAT is ugly. I'm an ole school constructor versus setter myself, Object is ready at Construction. But with CDI I get that guarantee with the no arg, easier to Mock. > > > > No need to start up CDI in init() > > we do not start up cdi in init, we configure how it interacts with the > wicket application. settings such things as conversation propagation > mode, etc. > Yeah but that starts it otherwise the Injectors are not set up and it wouldn't work(inject). > > > Start using CDI. > > > > For apps that may want to add CDI they just > > <filter> > > <filter-name>CdiApplication</filter-name> > > <filter-class>org.apache.wicket.cdi.CdiWicketFilter</filter-class> > > </filter> > > > > Start using CDI. > > > > So I still defend injection of the Cdi Wicket integration code. > > > > Also if it is removed then code will have to be added to differentiate > > which cdi implementation is being used. > > > > Currently CDI does this so long as two different CDI implementation jars > > aren't add, which would be ambiguous. > > this will still work. you can still inject the configuration using > noncontextual and pull the right instance. or use jdk's ServiceLoader > to find all implementors. > > my problem with this is that there is a specific lifecycle to the > application that is not managed by cdi. it is not safe to use the > application instance after it has been created, you have to wait until > wicket calls certain methods on it. > Yeah, I do wait. That's why I used the Factory. Only thing that is done is some member variables are populated. I did not override Wicket management, just injected some dependencies. > by making it managed you are giving the impression that it is safe to > inject the instance and use it in various places. it is not, not until > it has been properly configured. i do not want to debug cases where my > configuration changes to the application disappear because my code got > that injected the application and configured it got called before > internalInit(). either create a subcpass with @PostConstruct that > configures the application - which wont work - people dont like using > subclasses - or create a provider. > Like I said Cdi injects some members, the Factory returns the application to WicketFilter and the same lifecycle commences. > > -igor > > > On Wed, Nov 13, 2013 at 4:31 PM, Igor Vaynberg <[email protected] > >wrote: > > > >> i agree we should restart with the original implementation and > >> incrementally introduce the new features - thats what i proposed in > >> the original pull request. > >> > >> i am not a big fan of having the application instance managed. what is > >> the value of this? it can be injected using noncontextual just like > >> everything else... > >> > >> -igor > >> > >> > >> On Wed, Nov 13, 2013 at 1:19 PM, Emond Papegaaij > >> <[email protected]> wrote: > >> > Hi all, > >> > > >> > You probably noticed the the flood of emails regarding wicket-cdi > these > >> > last few days, which IMHO is good, because it means wicket-cdi is > alive. > >> > However, the current status is that the current (old) implementation > of > >> > wicket-cdi works badly with CDI 1.1 and the experimental (new) > version is > >> > broken in various ways. This is not good, as there is no good > >> > implementation to use for CDI 1.1 users. > >> > > >> > I've reviewed parts of the code in wicket-cdi-1.1 and noticed the > >> following > >> > problems: > >> > - Featuritis: it supports all kinds of usecases nobody is every going > to > >> > use, such as: partly managed applications, per-conversation > >> > auto-conversions, per-conversation propagation, package ignores, > switched > >> > to enable/disable injection on almost everyting. > >> > - Buggy: auto-conversations are broken for everything but pages, > >> injection > >> > in anonymous classes was broken, probably more. > >> > - Too much state: configuration options are copied all over the place, > >> > objects with different lifecycles share state. > >> > - Too much injection: everything is injected, which makes it almost > >> > impossible to see what's linked to what. > >> > > >> > I've also noticed some very nices features: > >> > - CDI 1.1 support with conversations via the Weld API > >> > - Management of the application and the Wicket-cdi configuration by > cdi. > >> > - Better implementation of NonContextual injection, using caches. > >> > - Better testcases > >> > > >> > The experimental code is in such a state that is it almost impossible > to > >> > cleanup. On the other hand, I do not want to loose some of the new > >> > features. Therefore, I propose the following: restart the > wicket-cdi-1.1 > >> > implementation, starting from the current wicket-cdi implementation > and > >> > reintroducing the features one-by-one. Also, I would like to remove > some > >> of > >> > the existing features, like most of the toggle-switches, and I would > like > >> > to make the new CdiWebApplicationFactory mandatory to always have the > >> > application be managed. All this will still be experimental in wicket > 6, > >> > but perhaps it can be made the default in 7. As we at Topicus are > >> currently > >> > starting the migration from JBoss 7.1 to WildFly 8, I can work on > this 1 > >> or > >> > 2 days a week. > >> > > >> > Let me know what you think, > >> > Emond > >> >
