Re: remove recently merged cdi 1.1 from 6.x

2013-11-13 Thread Emond Papegaaij
NonContextual.of(instance.getClass()).inject(instance); iow, exactly what's in the methods in NonContextualManager On Wed, Nov 13, 2013 at 9:47 PM, John Sarman johnsar...@gmail.com wrote: Since the NonContextualManager is responsible for injection post construction and predestroying, what

Re: remove recently merged cdi 1.1 from 6.x

2013-11-13 Thread John Sarman
So basically just move the NonContextual code directly into the AbstractInjector. I'm fine with that. On Wed, Nov 13, 2013 at 3:52 PM, Emond Papegaaij emond.papega...@gmail.comwrote: NonContextual.of(instance.getClass()).inject(instance); iow, exactly what's in the methods in

Re: remove recently merged cdi 1.1 from 6.x

2013-11-12 Thread Igor Vaynberg
the point on INonContextualManager was to internalize NonContextual - in case cdi implementation provides a better way to perform non-contextual injection. now that NonContextualManager is @ApplicationScoped and CdiConfiguration does not have a setter for it this functionality is lost. and even

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread Emond Papegaaij
Hi John, I've just merged the pull request in the wicket-6.x branch (still under experimental). The version still is 0.2-SNAPSHOT, as the versions are automatically increased on release. The reason I've merged the pull request is to give us all a common baseline to discuss. Could you please

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
It is not a forced requirement, just an option for full cdi injection. On Mon, Nov 11, 2013 at 3:50 AM, Emond Papegaaij emond.papega...@topicus.nl wrote: Hi John, I've just merged the pull request in the wicket-6.x branch (still under experimental). The version still is 0.2-SNAPSHOT, as

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
As far as the Test failing, I think the workaround code to use jndi first may have caused the test to fail. So far it seems that all the request pull 50 is not in the 6 branch. What forced the need for the workaround? John On Mon, Nov 11, 2013 at 8:00 AM, John Sarman johnsar...@gmail.com

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
Sorry, IS in the 6.x branch. Thanks for merging it On Mon, Nov 11, 2013 at 8:18 AM, John Sarman johnsar...@gmail.com wrote: As far as the Test failing, I think the workaround code to use jndi first may have caused the test to fail. So far it seems that all the request pull 50 is not in the

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread Emond Papegaaij
I reread the code, and I see what it is used for. CdiWebApplicationFactory can use a bit of a cleanup. For example, Instance has methods to check for satisfaction and ambiguity. Also, I think this should be rewritten as an all-or-nothing solution. You either use CdiWicketFilter with

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread Emond Papegaaij
You are right, InitialContext.lookup was returning null. I've fixed it by falling back to CDI.current().getBeanManager() in that case. The workaround is needed because of a very nasty bug in de Wildfly-Weld integration: https://issues.jboss.org/browse/WFLY-2481 Best regards, Emond On Monday

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
I just leveraged the WicketFilter and the Factory pattern in wicket. I did not want to just rewrite all that when the WicketFilter was already designed to be extensible as the CdiWicketFilter does. I guess just rewriting the createApplication in the CDIWebApplicationFactory to not fallback to the

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
I was looking at your bug, but in the case you specified where the cached BeanManager is passed, seems to be the correct behavior because the CdiConfiguration is ApplicationScoped. The CDI class states this: /** * Get the CDI BeanManager for the current context * * @return */

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
Ok, I read through your test code, so if you are saying that servlet a gets same beanmanager as servlet b then yeah thats bad. On Mon, Nov 11, 2013 at 8:44 AM, John Sarman johnsar...@gmail.com wrote: I was looking at your bug, but in the case you specified where the cached BeanManager is

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
Nevermind, I should not write emails this early, without an unsend. Servlet A should see same BeanManager as Servlet B, if both servlets are deployed from same war file. That is ApplicationScoped. On Mon, Nov 11, 2013 at 8:47 AM, John Sarman johnsar...@gmail.com wrote: Ok, I read through your

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
Edmond, I updated the cdi code to not ever use the CDI.current(). I reworked your test earlier to just inject the BeanManager and that properly allows the multiple wars to see the InjectableTargets from a shared lib. I could only conclude that CDI.current should not be called from a static

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread Emond Papegaaij
Hi John, There is nothing wrong with looking up the BeanManager in a static context. It's just that the current implementation of CDI.current().getBeanManager() is broken in Weld. I expect this to be fixed soon. The workaround moves the lookup to a single place and tries a portable JNDI lookup

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
Actually I just changed them back to the way Igor originally implemented it. On Mon, Nov 11, 2013 at 2:10 PM, Emond Papegaaij emond.papega...@gmail.comwrote: Hi John, There is nothing wrong with looking up the BeanManager in a static context. It's just that the current implementation of

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
Also Noncontextual.of is never used anywhere except in the Wicket CDI api, so I disagree. It solely used by the NonContextualManager which is ApplicationScoped and the BeanManager is injected. The CdiCleanup also uses it and is passed the BeanManager during configuration, both cases do not make

Re: remove recently merged cdi 1.1 from 6.x

2013-11-11 Thread John Sarman
Edmond, I do agree that the call should work in static methods. In fact the weld reference http://docs.jboss.org/weld/reference/latest/en-US/html/extend.html, states the following: Alternatively, a BeanManager reference may be obtained from CDI via a static method call.

Re: remove recently merged cdi 1.1 from 6.x

2013-11-09 Thread Emond Papegaaij
In wicket 6, this code also still is in experimental. The reason I ported it to Wicket 6, was to actually use it. wicket-cdi (the old module), is usable with 1.1, but not very optimal. One of the main problems with the old implementation is the amount of InjectionTargets created. The annoying

remove recently merged cdi 1.1 from 6.x

2013-11-08 Thread Igor Vaynberg
not sure why this was merged into 6.x but there are a lot of problems with this contribution as can be seen here [1]. i think this should be removed from at least the release branch. -igor [1] https://github.com/apache/wicket/pull/50#issuecomment-28063112

Re: remove recently merged cdi 1.1 from 6.x

2013-11-08 Thread John Sarman
+1 removal Never should have been merged into the 6 branch and not the 7 until there is a consensus. On Fri, Nov 8, 2013 at 4:07 PM, Igor Vaynberg igor.vaynb...@gmail.comwrote: not sure why this was merged into 6.x but there are a lot of problems with this contribution as can be seen here