On Wed, Nov 13, 2013 at 1:43 PM, John Sarman <johnsar...@gmail.com> 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.
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
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.
>
> 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.
> 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.
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.
-igor
> On Wed, Nov 13, 2013 at 4:31 PM, Igor Vaynberg <igor.vaynb...@gmail.com>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
>> <emond.papega...@gmail.com> 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
>>
