Actually I just changed them back to the way Igor originally implemented it.
On Mon, Nov 11, 2013 at 2:10 PM, Emond Papegaaij <[email protected]>wrote: > 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 first. This will > work in all EE containers. More importantly, the user doesn't notice the > workaround at all. > > Your commit changes the signature of NonContextual.of. You are now required > to pass in the BeanManager, which is not very nice, because NonContextual > is to do CDI injection in places where CDI can't do it for you. In these > places, you often don't have a BeanManager available. I think the > workaround should stay until the bug is fixed in Weld. > > Best regards, > Emond > > > On Mon, Nov 11, 2013 at 7:45 PM, John Sarman <[email protected]> wrote: > > > 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 method, > > so instead of joining the weld team or submitting an issue, I just > removed > > that possibility from the code. That latest code is in > > https://github.com/jsarman/wicket master branch. This also removed the > > need > > for the BeanManagerLookup workaround. > > > > > > > > > > On Mon, Nov 11, 2013 at 8:49 AM, John Sarman <[email protected]> > wrote: > > > > > 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 <[email protected]> > > wrote: > > > > > >> 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 <[email protected] > > >wrote: > > >> > > >>> 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 > > >>> */ > > >>> public abstract BeanManager getBeanManager(); > > >>> > > >>> So the cached BeanManager passed back is because it is accessed in an > > >>> ApplicationScoped class. ApplicationScoped != Wickets Application > > scope. > > >>> > > >>> > > >>> > > >>> > > >>> > > >>> On Mon, Nov 11, 2013 at 8:26 AM, Emond Papegaaij < > > >>> [email protected]> wrote: > > >>> > > >>>> 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 11 November 2013 08:18:20 John Sarman 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 6 branch. > > >>>> > What forced the need for the workaround? > > >>>> > > > >>>> > John > > >>>> > > > >>>> > On Mon, Nov 11, 2013 at 8:00 AM, John Sarman > > >>>> <[email protected]> wrote: > > >>>> > > It is not a forced requirement, just an option for full cdi > > >>>> injection. > > >>>> > > > > >>>> > > > > >>>> > > On Mon, Nov 11, 2013 at 3:50 AM, Emond Papegaaij < > > >>>> > > > > >>>> > > [email protected]> 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 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 > > >>>> > >> verify I did not break anything merging it? All testcases but > one > > >>>> pass. > > >>>> > >> The > > >>>> > >> failing testcase (CdiConfigurationTest.testMultiAppLoad) tries > to > > >>>> access > > >>>> > >> the > > >>>> > >> BeanManager from an unmanaged thread, resulting in an NPE. > > >>>> > >> > > >>>> > >> I've already noticed one aspect I do not like: the requirement > to > > >>>> > >> annotate > > >>>> > >> your app with @WicketApp. With a Producer method, it should be > > >>>> possible > > >>>> > >> to use the actual application names, without the requirement to > > >>>> duplicate > > >>>> > >> them on your application class. > > >>>> > >> > > >>>> > >> Best regards, > > >>>> > >> Emond > > >>>> > >> > > >>>> > >> On Sunday 10 November 2013 16:44:28 John Sarman wrote: > > >>>> > >> > Edmond, > > >>>> > >> > On July, I worked vigorously to get to the 0.3 snapshot, > which > > >>>> was > > >>>> what > > >>>> > >> > > >>>> > >> I > > >>>> > >> > > >>>> > >> > consider the first beta ready version of the move to cdi1.1. > > The > > >>>> 0.1 > > >>>> > >> > > >>>> > >> and > > >>>> > >> > > >>>> > >> > 0.2 snapshot was 0.1, getting it to work and learning how to > > >>>> request > > >>>> > >> > > >>>> > >> pull > > >>>> > >> > > >>>> > >> > requests. 0.2 was adding some slight fixes and testing. > After > > >>>> that > > >>>> I > > >>>> > >> > realized that I was treating the @ApplicationScoped as same > > >>>> scope that > > >>>> > >> > ThreadContext gives to a Wicket App. That is entirely wrong. > > So > > >>>> the > > >>>> > >> > previous version only properly supports at most 1 Wicket app, > > the > > >>>> > >> > > >>>> > >> second > > >>>> > >> > > >>>> > >> > could override the Configuration of the first (not > acceptable). > > >>>> In > > >>>> my > > >>>> > >> > > >>>> > >> 0.3 > > >>>> > >> > > >>>> > >> > version, I added the code to prevent that, by using the > Wicket > > >>>> app > > >>>> key > > >>>> > >> > generated as the key to the configuration properties for an > > app. > > >>>> This > > >>>> > >> > allows for multiple Wicket apps to be deployed in a Servlet. > > >>>> However, > > >>>> > >> > > >>>> > >> for > > >>>> > >> > > >>>> > >> > whatever reason, that checkin could not properly merge into > > the 7 > > >>>> > >> > > >>>> > >> branch. > > >>>> > >> > > >>>> > >> > I have to remedy this even if I just have to copy paste the > > >>>> code, to > > >>>> > >> > > >>>> > >> make > > >>>> > >> > > >>>> > >> > git happy ( I blame myself, not Git). In the meantime, I > > >>>> recommend > > >>>> > >> > > >>>> > >> looking > > >>>> > >> > > >>>> > >> > at my latest (albeit broken) pull request > > >>>> > >> > https://github.com/apache/wicket/pull/50 and port that > > version. > > >>>> It > > >>>> adds > > >>>> > >> > thorough testing, fixes the multiple deploy issue, > reintroduces > > >>>> the > > >>>> > >> > auto > > >>>> > >> > Conversation, and extends the ConversationalComponent by > > >>>> introducing > > >>>> > >> > > >>>> > >> the > > >>>> > >> > > >>>> > >> > @Conversational, which by default works the same as the > Cdi-1.0 > > >>>> > >> > ConverationalComponent, but also allows the propagation and > > >>>> auto > > >>>> > >> > > >>>> > >> feature to > > >>>> > >> > > >>>> > >> > be modified for an Object that uses the annotation, without > > >>>> affecting > > >>>> > >> > > >>>> > >> the > > >>>> > >> > > >>>> > >> > global defaults set during Configuration. The 0.3 also > > >>>> introduces > > >>>> the > > >>>> > >> > CdiWicketFilter. The CdiWicketFilter allows the > configuration > > >>>> settings > > >>>> > >> > > >>>> > >> to > > >>>> > >> > > >>>> > >> > be managed in web.xml. It also instantiates the > > >>>> WicketApplication > > >>>> > >> > using > > >>>> > >> > Cdi so that the Application is injected before the init() > > method. > > >>>> The > > >>>> > > >>> > > >>> > > >> > > > > > >
