You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Igor Vaynberg <ig...@gmail.com> on 2013/11/08 22:07:42 UTC
remove recently merged cdi 1.1 from 6.x
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
Posted by John Sarman <jo...@gmail.com>.
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 super classes createApplication if no Applications are
found using CDI would created the all or none solution, the the other cases
in the code are useful IMO. First if you did not stress out over the extra
line of code in the WebApplication and annotated with
@WicketApp("SomeName") then the factory simply gets the instance via the
name. If the extra line of code was too much then it verifies there is one
and only one Application in the Instance<WebApplication> variable. If
there is only on WebApplication the it uses it. However if there are more
then we have an ambiguity, and fails loudly. If anything the search via
applicationClassName should be added to allow for the fully qualified name
as is the normal with the default filter.
On Mon, Nov 11, 2013 at 8:20 AM, Emond Papegaaij <emond.papegaaij@topicus.nl
> wrote:
> 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
> CdiWebApplicationFactory or you don't. This should greatly reduce the
> amount of code and should allow for more robust error checking.
>
> Best regards,
> Emond
>
>
>
> On Monday 11 November 2013 08:00:10 John Sarman 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
> <emond.papegaaij@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 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
> > > >
> > > > <em...@gmail.com>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
> > >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Emond Papegaaij <em...@topicus.nl>.
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
CdiWebApplicationFactory or you don't. This should greatly reduce the
amount of code and should allow for more robust error checking.
Best regards,
Emond
On Monday 11 November 2013 08:00:10 John Sarman 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
<emond.papegaaij@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 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
> > >
> > > <em...@gmail.com>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
> >
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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
<em...@gmail.com>wrote:
> 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 <jo...@gmail.com> wrote:
>
> > Since the NonContextualManager is responsible for injection post
> > construction and predestroying, what would you replace it with?
> >
> >
> > On Wed, Nov 13, 2013 at 3:22 PM, Emond Papegaaij
> > <em...@gmail.com>wrote:
> >
> > > On Tue, Nov 12, 2013 at 11:32 PM, Igor Vaynberg <
> igor.vaynberg@gmail.com
> > > >wrote:
> > >
> > > > the point on INonContextualManager was to internalize NonContextual -
> > > > in case cdi implementation provides a better way to perform
> > > > non-contextual injection.
> > > >
> > >
> > > I don't think that going to happen. Even if it does happen, I see no
> > reason
> > > why Wicket must be able to use that implementation specific way of
> > > injecting objects. The current implementation works fine and is
> supported
> > > by all CDI providers.
> > >
> > > now that NonContextualManager is @ApplicationScoped and
> > > > CdiConfiguration does not have a setter for it this functionality is
> > > > lost. and even if cdi configuration had a setter, there is no longer
> a
> > > > guarantee that someone did not inject one and use it before a new one
> > > > was set on the configuration. that is why these things were not
> > > > managed by cdi in the initial code...
> > > >
> > >
> > > That's what alternatives are for. But again: I see no reason to replace
> > > NonContextual.
> > >
> > > I would like to remove NonContextualManager (and the interface) as I
> > don't
> > > see any added value.
> > >
> > > Best regards,
> > > Emond
> > >
> > > On Mon, Nov 11, 2013 at 11:16 AM, John Sarman <jo...@gmail.com>
> > > wrote:
> > > > > 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 it unnice since it is an internal api.
> > > > >
> > > > >
> > > > > On Mon, Nov 11, 2013 at 2:11 PM, John Sarman <johnsarman@gmail.com
> >
> > > > wrote:
> > > > >
> > > > >> 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.papegaaij@gmail.com> 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 <
> johnsarman@gmail.com
> > >
> > > > >>> 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 <
> > johnsarman@gmail.com
> > > >
> > > > >>> 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 <
> > > johnsarman@gmail.com
> > > > >
> > > > >>> > 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 <
> > > > johnsarman@gmail.com
> > > > >>> > >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 <
> > > > >>> > >>> emond.papegaaij@topicus.nl> 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
> > > > >>> > >>>> <jo...@gmail.com> 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 <
> > > > >>> > >>>> > >
> > > > >>> > >>>> > > emond.papegaaij@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
> > 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
> > > > >>> > >>>>
> > > > >>> > >>>
> > > > >>> > >>>
> > > > >>> > >>
> > > > >>> > >
> > > > >>> >
> > > > >>>
> > > > >>
> > > > >>
> > > >
> > >
> >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Emond Papegaaij <em...@gmail.com>.
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 <jo...@gmail.com> wrote:
> Since the NonContextualManager is responsible for injection post
> construction and predestroying, what would you replace it with?
>
>
> On Wed, Nov 13, 2013 at 3:22 PM, Emond Papegaaij
> <em...@gmail.com>wrote:
>
> > On Tue, Nov 12, 2013 at 11:32 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> > >wrote:
> >
> > > the point on INonContextualManager was to internalize NonContextual -
> > > in case cdi implementation provides a better way to perform
> > > non-contextual injection.
> > >
> >
> > I don't think that going to happen. Even if it does happen, I see no
> reason
> > why Wicket must be able to use that implementation specific way of
> > injecting objects. The current implementation works fine and is supported
> > by all CDI providers.
> >
> > now that NonContextualManager is @ApplicationScoped and
> > > CdiConfiguration does not have a setter for it this functionality is
> > > lost. and even if cdi configuration had a setter, there is no longer a
> > > guarantee that someone did not inject one and use it before a new one
> > > was set on the configuration. that is why these things were not
> > > managed by cdi in the initial code...
> > >
> >
> > That's what alternatives are for. But again: I see no reason to replace
> > NonContextual.
> >
> > I would like to remove NonContextualManager (and the interface) as I
> don't
> > see any added value.
> >
> > Best regards,
> > Emond
> >
> > On Mon, Nov 11, 2013 at 11:16 AM, John Sarman <jo...@gmail.com>
> > wrote:
> > > > 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 it unnice since it is an internal api.
> > > >
> > > >
> > > > On Mon, Nov 11, 2013 at 2:11 PM, John Sarman <jo...@gmail.com>
> > > wrote:
> > > >
> > > >> 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.papegaaij@gmail.com> 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 <johnsarman@gmail.com
> >
> > > >>> 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 <
> johnsarman@gmail.com
> > >
> > > >>> 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 <
> > johnsarman@gmail.com
> > > >
> > > >>> > 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 <
> > > johnsarman@gmail.com
> > > >>> > >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 <
> > > >>> > >>> emond.papegaaij@topicus.nl> 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
> > > >>> > >>>> <jo...@gmail.com> 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 <
> > > >>> > >>>> > >
> > > >>> > >>>> > > emond.papegaaij@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
> 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
> > > >>> > >>>>
> > > >>> > >>>
> > > >>> > >>>
> > > >>> > >>
> > > >>> > >
> > > >>> >
> > > >>>
> > > >>
> > > >>
> > >
> >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
Since the NonContextualManager is responsible for injection post
construction and predestroying, what would you replace it with?
On Wed, Nov 13, 2013 at 3:22 PM, Emond Papegaaij
<em...@gmail.com>wrote:
> On Tue, Nov 12, 2013 at 11:32 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
>
> > the point on INonContextualManager was to internalize NonContextual -
> > in case cdi implementation provides a better way to perform
> > non-contextual injection.
> >
>
> I don't think that going to happen. Even if it does happen, I see no reason
> why Wicket must be able to use that implementation specific way of
> injecting objects. The current implementation works fine and is supported
> by all CDI providers.
>
> now that NonContextualManager is @ApplicationScoped and
> > CdiConfiguration does not have a setter for it this functionality is
> > lost. and even if cdi configuration had a setter, there is no longer a
> > guarantee that someone did not inject one and use it before a new one
> > was set on the configuration. that is why these things were not
> > managed by cdi in the initial code...
> >
>
> That's what alternatives are for. But again: I see no reason to replace
> NonContextual.
>
> I would like to remove NonContextualManager (and the interface) as I don't
> see any added value.
>
> Best regards,
> Emond
>
> On Mon, Nov 11, 2013 at 11:16 AM, John Sarman <jo...@gmail.com>
> wrote:
> > > 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 it unnice since it is an internal api.
> > >
> > >
> > > On Mon, Nov 11, 2013 at 2:11 PM, John Sarman <jo...@gmail.com>
> > wrote:
> > >
> > >> 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.papegaaij@gmail.com> 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 <jo...@gmail.com>
> > >>> 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 <johnsarman@gmail.com
> >
> > >>> 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 <
> johnsarman@gmail.com
> > >
> > >>> > 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 <
> > johnsarman@gmail.com
> > >>> > >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 <
> > >>> > >>> emond.papegaaij@topicus.nl> 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
> > >>> > >>>> <jo...@gmail.com> 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 <
> > >>> > >>>> > >
> > >>> > >>>> > > emond.papegaaij@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 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
> > >>> > >>>>
> > >>> > >>>
> > >>> > >>>
> > >>> > >>
> > >>> > >
> > >>> >
> > >>>
> > >>
> > >>
> >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Emond Papegaaij <em...@gmail.com>.
On Tue, Nov 12, 2013 at 11:32 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> the point on INonContextualManager was to internalize NonContextual -
> in case cdi implementation provides a better way to perform
> non-contextual injection.
>
I don't think that going to happen. Even if it does happen, I see no reason
why Wicket must be able to use that implementation specific way of
injecting objects. The current implementation works fine and is supported
by all CDI providers.
now that NonContextualManager is @ApplicationScoped and
> CdiConfiguration does not have a setter for it this functionality is
> lost. and even if cdi configuration had a setter, there is no longer a
> guarantee that someone did not inject one and use it before a new one
> was set on the configuration. that is why these things were not
> managed by cdi in the initial code...
>
That's what alternatives are for. But again: I see no reason to replace
NonContextual.
I would like to remove NonContextualManager (and the interface) as I don't
see any added value.
Best regards,
Emond
On Mon, Nov 11, 2013 at 11:16 AM, John Sarman <jo...@gmail.com> wrote:
> > 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 it unnice since it is an internal api.
> >
> >
> > On Mon, Nov 11, 2013 at 2:11 PM, John Sarman <jo...@gmail.com>
> wrote:
> >
> >> 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.papegaaij@gmail.com> 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 <jo...@gmail.com>
> >>> 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 <jo...@gmail.com>
> >>> 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 <johnsarman@gmail.com
> >
> >>> > 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 <
> johnsarman@gmail.com
> >>> > >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 <
> >>> > >>> emond.papegaaij@topicus.nl> 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
> >>> > >>>> <jo...@gmail.com> 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 <
> >>> > >>>> > >
> >>> > >>>> > > emond.papegaaij@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 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
> >>> > >>>>
> >>> > >>>
> >>> > >>>
> >>> > >>
> >>> > >
> >>> >
> >>>
> >>
> >>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Igor Vaynberg <ig...@gmail.com>.
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 if cdi configuration had a setter, there is no longer a
guarantee that someone did not inject one and use it before a new one
was set on the configuration. that is why these things were not
managed by cdi in the initial code...
-igor
On Mon, Nov 11, 2013 at 11:16 AM, John Sarman <jo...@gmail.com> wrote:
> 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 it unnice since it is an internal api.
>
>
> On Mon, Nov 11, 2013 at 2:11 PM, John Sarman <jo...@gmail.com> wrote:
>
>> 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.papegaaij@gmail.com> 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 <jo...@gmail.com>
>>> 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 <jo...@gmail.com>
>>> 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 <jo...@gmail.com>
>>> > 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 <johnsarman@gmail.com
>>> > >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 <
>>> > >>> emond.papegaaij@topicus.nl> 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
>>> > >>>> <jo...@gmail.com> 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 <
>>> > >>>> > >
>>> > >>>> > > emond.papegaaij@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 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
>>> > >>>>
>>> > >>>
>>> > >>>
>>> > >>
>>> > >
>>> >
>>>
>>
>>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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.
CDI.current().getBeanManager()
So the code was intentionally placed in the cdi api to do exactly what the
wicket cdi code was doing. That being said I still lean to just not using
it to allow for the code to work even on app servers that may use the weld
impl that has the bug. Either way it is solved, (add lookup class to use
jndi, or use @Inject and pass the reference to the NonContextual) is fine
with me. My approach was to just get rid of the static method lookup, and
on that path I reverted my changes to the NonContextual class to allow the
BeanManager as a constructor arg and method args where applicable.
OT: I updated the tests to use cdi-unit 2.2.1 jar, that is in my repo as
well.
John
On Mon, Nov 11, 2013 at 2:16 PM, John Sarman <jo...@gmail.com> wrote:
> 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 it unnice since it is an internal api.
>
>
> On Mon, Nov 11, 2013 at 2:11 PM, John Sarman <jo...@gmail.com> wrote:
>
>> 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.papegaaij@gmail.com> 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 <jo...@gmail.com>
>>> 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 <jo...@gmail.com>
>>> 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 <jo...@gmail.com>
>>> > 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 <johnsarman@gmail.com
>>> > >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 <
>>> > >>> emond.papegaaij@topicus.nl> 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
>>> > >>>> <jo...@gmail.com> 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 <
>>> > >>>> > >
>>> > >>>> > > emond.papegaaij@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 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
>>> > >>>>
>>> > >>>
>>> > >>>
>>> > >>
>>> > >
>>> >
>>>
>>
>>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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 it unnice since it is an internal api.
On Mon, Nov 11, 2013 at 2:11 PM, John Sarman <jo...@gmail.com> wrote:
> 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.papegaaij@gmail.com> 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 <jo...@gmail.com>
>> 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 <jo...@gmail.com>
>> 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 <jo...@gmail.com>
>> > 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 <johnsarman@gmail.com
>> > >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 <
>> > >>> emond.papegaaij@topicus.nl> 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
>> > >>>> <jo...@gmail.com> 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 <
>> > >>>> > >
>> > >>>> > > emond.papegaaij@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 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
>> > >>>>
>> > >>>
>> > >>>
>> > >>
>> > >
>> >
>>
>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
Actually I just changed them back to the way Igor originally implemented it.
On Mon, Nov 11, 2013 at 2:10 PM, Emond Papegaaij
<em...@gmail.com>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 <jo...@gmail.com> 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 <jo...@gmail.com>
> 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 <jo...@gmail.com>
> > 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 <johnsarman@gmail.com
> > >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 <
> > >>> emond.papegaaij@topicus.nl> 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
> > >>>> <jo...@gmail.com> 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 <
> > >>>> > >
> > >>>> > > emond.papegaaij@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 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
> > >>>>
> > >>>
> > >>>
> > >>
> > >
> >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Emond Papegaaij <em...@gmail.com>.
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 <jo...@gmail.com> 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 <jo...@gmail.com> 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 <jo...@gmail.com>
> 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 <johnsarman@gmail.com
> >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 <
> >>> emond.papegaaij@topicus.nl> 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
> >>>> <jo...@gmail.com> 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 <
> >>>> > >
> >>>> > > emond.papegaaij@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 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
> >>>>
> >>>
> >>>
> >>
> >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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 <jo...@gmail.com> 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 <jo...@gmail.com> 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 <jo...@gmail.com>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 <
>>> emond.papegaaij@topicus.nl> 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
>>>> <jo...@gmail.com> 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 <
>>>> > >
>>>> > > emond.papegaaij@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 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
>>>>
>>>
>>>
>>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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 <jo...@gmail.com> 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 <jo...@gmail.com> 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 <
>> emond.papegaaij@topicus.nl> 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
>>> <jo...@gmail.com> 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 <
>>> > >
>>> > > emond.papegaaij@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 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
>>>
>>
>>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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 <jo...@gmail.com> 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 <
> emond.papegaaij@topicus.nl> 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
>> <jo...@gmail.com> 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 <
>> > >
>> > > emond.papegaaij@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 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
>>
>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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 <emond.papegaaij@topicus.nl
> 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
> <jo...@gmail.com> 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 <
> > >
> > > emond.papegaaij@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 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
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Emond Papegaaij <em...@topicus.nl>.
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
<jo...@gmail.com> 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 <
> >
> > emond.papegaaij@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 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
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
Sorry, IS in the 6.x branch. Thanks for merging it
On Mon, Nov 11, 2013 at 8:18 AM, John Sarman <jo...@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 6 branch.
> What forced the need for the workaround?
>
> John
>
>
> On Mon, Nov 11, 2013 at 8:00 AM, John Sarman <jo...@gmail.com> 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 <
>> emond.papegaaij@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 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
>>> >
>>> > <em...@gmail.com>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
>>> <jo...@gmail.com> 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
>>> <igor.vaynberg@gmail.com
>>> > > >
>>> > > > >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
>>>
>>>
>>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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 <jo...@gmail.com> 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 <
> emond.papegaaij@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 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
>> >
>> > <em...@gmail.com>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
>> <jo...@gmail.com> 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
>> <igor.vaynberg@gmail.com
>> > > >
>> > > > >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
>>
>>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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.papegaaij@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 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
> >
> > <em...@gmail.com>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
> <jo...@gmail.com> 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
> <igor.vaynberg@gmail.com
> > > >
> > > > >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
>
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Emond Papegaaij <em...@topicus.nl>.
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
>
> <em...@gmail.com>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
<jo...@gmail.com> 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
<igor.vaynberg@gmail.com
> > >
> > > >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
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
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
<em...@gmail.com>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 <jo...@gmail.com> 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 <igor.vaynberg@gmail.com
> > >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
> > >
> >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by Emond Papegaaij <em...@gmail.com>.
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 <jo...@gmail.com> 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 <igor.vaynberg@gmail.com
> >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
> >
>
Re: remove recently merged cdi 1.1 from 6.x
Posted by John Sarman <jo...@gmail.com>.
+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 <ig...@gmail.com>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
>