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
> changes do not break the original Cdi-1.0, initialization technique, to
> support the backwards compatibility.
> 
> John
> 
> 
> 
> On Sat, Nov 9, 2013 at 3:29 PM, Emond Papegaaij
> 
> <[email protected]>wrote:
> > 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
> > warnings will probably be fixed in Weld (
> > https://issues.jboss.org/browse/WELD-1547), but the fact remains that
> > InjectionTargets are very expensive to create and should be cached.
> > Another
> > thing I like about the new module is that it actually uses CDI, not just
> > makes it available in Wicket. Also, the integration requires a lot less
> > code in a container that uses Weld (like Wildfly).
> > 
> > I do agree that the code is far from ready. For example, I don't think
> > entire packages should be ignored. Also, I don't like how settings like
> > auto-conversations are injected. I do like that CDI is used for that, but
> > I'd rather see a configuration object with a @Producer method for all
> > settings at once. Having the code in wicket 6 allows me to work on 
these
> > issues. I do not expect our current application to be ported to Wicket 
7
> > any time soon, but we are migrating to CDI 1.1 on Wildfly.
> > 
> > Best regards,
> > Emond
> > 
> > On Fri, Nov 8, 2013 at 10:10 PM, John Sarman 
<[email protected]> wrote:
> > > +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 
<[email protected]
> > > 
> > > >wrote:
> > > > 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

Reply via email to