You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Emond Papegaaij <em...@topicus.nl> on 2013/07/03 10:09:33 UTC
WICKET-5226 breaks many wicket-cdi applications
Hi all,
The changes made for WICKET-5226 (no longer injecting anonymous inner
classes) breaks our applications badly, and I suspect we are not the only
one. The following pattern is quite common:
add(new Link<Void>("rest") {
@Inject
private AccountRestResourceClient accountClient;
@Override
public void onClick() {....}
}
In Wicket 6.9.0, this no longer works: accountClient will not get injected
because the Link is an anonymous inner class. To make things worse, even
in concrete classes, you can't rely on injection to work, because someone
could subclass your class as an anonymous inner class and injection will
again not work.
This change was made because Weld 2.0 throws an exception when trying
to create an InjectionTarget for anonymous classes. This will be reduced to
a log-warning in 2.0.2: https://issues.jboss.org/browse/WELD-1441
As there is no workaround for this problem and it is likely to break all
applications using wicket-cdi, I'm starting a vote to revert the changes
made for 5226 and release 6.9.1:
[ ] Yes, revert 5226 and release 6.9.1
[ ] No, revert 5226 but not release 6.9.1
[ ] No, keep as is
See WICKET-5264
Best regards,
Emond
Re: WICKET-5226 breaks many wicket-cdi applications
Posted by Martin Grigorov <mg...@apache.org>.
On Wed, Jul 3, 2013 at 11:09 AM, Emond Papegaaij <emond.papegaaij@topicus.nl
> wrote:
> Hi all,
>
> The changes made for WICKET-5226 (no longer injecting anonymous inner
> classes) breaks our applications badly, and I suspect we are not the only
> one. The following pattern is quite common:
>
> add(new Link<Void>("rest") {
> @Inject
> private AccountRestResourceClient accountClient;
>
Nothing wrong with this code but this is the first time I see this pattern.
>
> @Override
> public void onClick() {....}
> }
>
> In Wicket 6.9.0, this no longer works: accountClient will not get injected
> because the Link is an anonymous inner class. To make things worse, even
> in concrete classes, you can't rely on injection to work, because someone
> could subclass your class as an anonymous inner class and injection will
> again not work.
>
> This change was made because Weld 2.0 throws an exception when trying
> to create an InjectionTarget for anonymous classes. This will be reduced to
> a log-warning in 2.0.2: https://issues.jboss.org/browse/WELD-1441
https://issues.jboss.org/browse/WFLY-1378?focusedCommentId=12776674&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12776674
talks
about wrongusage of BeanManager.createInjectionTarget() in wicket-cdi.
>
>
> As there is no workaround for this problem and it is likely to break all
> applications using wicket-cdi, I'm starting a vote to revert the changes
> made for 5226 and release 6.9.1:
>
> [ x ] Yes, revert 5226 and release 6.9.1
>
wicket-cdi-1.1 also does this + some more blacklisting. Any reviews are
welcome!
> [ ] No, revert 5226 but not release 6.9.1
> [ ] No, keep as is
>
> See WICKET-5264
>
> Best regards,
> Emond
>
Re: WICKET-5226 breaks many wicket-cdi applications
Posted by Emond Papegaaij <em...@topicus.nl>.
On Wednesday 03 July 2013 12:07:07 Martin Grigorov wrote:
> Here are the commits for WELD-1441:
>
>
https://github.com/weld/core/commit/569c8efc5f0dd1dcc9c942fd3dd8c1f6
5a216b9e
>
https://github.com/weld/core/commit/9e248b5306b382f88f41cca1325178e
d6fbe4306
>
> What bothers me is
>
https://github.com/weld/core/commit/569c8efc5f0dd1dcc9c942fd3dd8c1f6
5a216b9e
> #L1R55 It throws CreationException for abstract classes and non-static-
inner
> ones. Anonymous are not even mentioned in this ticket and its commits.
And
> Weld 2.0 does fail when processing an anonymous class!
Wicket will never call produce(), as that's the method to get
(create/resolve) an instance of the bean.
> I'm just saying that after reverting this change Wicket+Weld 2.0.2 users
> will see their apps failing again, no matter whether they use your pattern
> for DI in anonymous classes or not. Weld 2.0.1 throws exception no
matter
> whether there are @Inject-ed members in the anonymous class or not.
Weld 2.0.1 is broken and broken frameworks break your application. Weld
2.0.2 users should be fine. We should however look at the wicket code and
see if we can cache the InjectionTarget instances. According to the weld
devs, get a new target over and over again for the same class is a bad
idea. But that's something that can go into 6.10
Emond
>
>
>
> On Wed, Jul 3, 2013 at 11:53 AM, Emond Papegaaij
<emond.papegaaij@topicus.nl
> > wrote:
> >
> > Here's my vote:
> > [X] Yes, revert 5226 and release 6.9.1
> >
> > Emond
> >
> > On Wednesday 03 July 2013 10:09:33 Emond Papegaaij wrote:
> > > Hi all,
> > >
> > > The changes made for WICKET-5226 (no longer injecting anonymous
> >
> > inner
> >
> > > classes) breaks our applications badly, and I suspect we are not the
> >
> > only
> >
> > > one. The following pattern is quite common:
> > >
> > > add(new Link<Void>("rest") {
> > >
> > > @Inject
> > > private AccountRestResourceClient accountClient;
> > >
> > > @Override
> > > public void onClick() {....}
> > >
> > > }
> > >
> > > In Wicket 6.9.0, this no longer works: accountClient will not get
> >
> > injected
> >
> > > because the Link is an anonymous inner class. To make things
worse,
> >
> > even
> >
> > > in concrete classes, you can't rely on injection to work, because
> >
> > someone
> >
> > > could subclass your class as an anonymous inner class and
injection will
> > > again not work.
> > >
> > > This change was made because Weld 2.0 throws an exception when
> >
> > trying
> >
> > > to create an InjectionTarget for anonymous classes. This will be
reduced
> >
> > to
> >
> > > a log-warning in 2.0.2: https://issues.jboss.org/browse/WELD-1441
> > >
> > > As there is no workaround for this problem and it is likely to break
all
> > > applications using wicket-cdi, I'm starting a vote to revert the
changes
> > > made for 5226 and release 6.9.1:
> > >
> > > [ ] Yes, revert 5226 and release 6.9.1
> > > [ ] No, revert 5226 but not release 6.9.1
> > > [ ] No, keep as is
> > >
> > > See WICKET-5264
> > >
> > > Best regards,
> > > Emond
Re: WICKET-5226 breaks many wicket-cdi applications
Posted by Martin Grigorov <mg...@apache.org>.
Here are the commits for WELD-1441:
https://github.com/weld/core/commit/569c8efc5f0dd1dcc9c942fd3dd8c1f65a216b9e
https://github.com/weld/core/commit/9e248b5306b382f88f41cca1325178ed6fbe4306
What bothers me is
https://github.com/weld/core/commit/569c8efc5f0dd1dcc9c942fd3dd8c1f65a216b9e#L1R55
It throws CreationException for abstract classes and non-static-inner ones.
Anonymous are not even mentioned in this ticket and its commits. And Weld
2.0 does fail when processing an anonymous class!
I'm just saying that after reverting this change Wicket+Weld 2.0.2 users
will see their apps failing again, no matter whether they use your pattern
for DI in anonymous classes or not. Weld 2.0.1 throws exception no matter
whether there are @Inject-ed members in the anonymous class or not.
On Wed, Jul 3, 2013 at 11:53 AM, Emond Papegaaij <emond.papegaaij@topicus.nl
> wrote:
> Here's my vote:
> [X] Yes, revert 5226 and release 6.9.1
>
> Emond
>
> On Wednesday 03 July 2013 10:09:33 Emond Papegaaij wrote:
> > Hi all,
> >
> > The changes made for WICKET-5226 (no longer injecting anonymous
> inner
> > classes) breaks our applications badly, and I suspect we are not the
> only
> > one. The following pattern is quite common:
> >
> > add(new Link<Void>("rest") {
> > @Inject
> > private AccountRestResourceClient accountClient;
> >
> > @Override
> > public void onClick() {....}
> > }
> >
> > In Wicket 6.9.0, this no longer works: accountClient will not get
> injected
> > because the Link is an anonymous inner class. To make things worse,
> even
> > in concrete classes, you can't rely on injection to work, because
> someone
> > could subclass your class as an anonymous inner class and injection will
> > again not work.
> >
> > This change was made because Weld 2.0 throws an exception when
> trying
> > to create an InjectionTarget for anonymous classes. This will be reduced
> to
> > a log-warning in 2.0.2: https://issues.jboss.org/browse/WELD-1441
> >
> > As there is no workaround for this problem and it is likely to break all
> > applications using wicket-cdi, I'm starting a vote to revert the changes
> > made for 5226 and release 6.9.1:
> >
> > [ ] Yes, revert 5226 and release 6.9.1
> > [ ] No, revert 5226 but not release 6.9.1
> > [ ] No, keep as is
> >
> > See WICKET-5264
> >
> > Best regards,
> > Emond
>
Re: WICKET-5226 breaks many wicket-cdi applications
Posted by Emond Papegaaij <em...@topicus.nl>.
Here's my vote:
[X] Yes, revert 5226 and release 6.9.1
Emond
On Wednesday 03 July 2013 10:09:33 Emond Papegaaij wrote:
> Hi all,
>
> The changes made for WICKET-5226 (no longer injecting anonymous
inner
> classes) breaks our applications badly, and I suspect we are not the
only
> one. The following pattern is quite common:
>
> add(new Link<Void>("rest") {
> @Inject
> private AccountRestResourceClient accountClient;
>
> @Override
> public void onClick() {....}
> }
>
> In Wicket 6.9.0, this no longer works: accountClient will not get injected
> because the Link is an anonymous inner class. To make things worse,
even
> in concrete classes, you can't rely on injection to work, because
someone
> could subclass your class as an anonymous inner class and injection will
> again not work.
>
> This change was made because Weld 2.0 throws an exception when
trying
> to create an InjectionTarget for anonymous classes. This will be reduced
to
> a log-warning in 2.0.2: https://issues.jboss.org/browse/WELD-1441
>
> As there is no workaround for this problem and it is likely to break all
> applications using wicket-cdi, I'm starting a vote to revert the changes
> made for 5226 and release 6.9.1:
>
> [ ] Yes, revert 5226 and release 6.9.1
> [ ] No, revert 5226 but not release 6.9.1
> [ ] No, keep as is
>
> See WICKET-5264
>
> Best regards,
> Emond