IMHO the correct way to deal with this is to simply make bridge methods delegate to super(), which will then result in the actual intercepted method being called. To be honest I thought we already did this, as we have had multiple related bugs in the past. Do you have a test case that I can look at? Stuart > Marko Lukša <mailto:marko.luksa@gmail.com> > 7 September 2012 7:19 PM > Hey all. > > I've been working on https://issues.jboss.org/browse/WELD-1162 and > need your opinion. > > Say we have: > > public interface Foo<T> { > void doSomething(T t); > } > public interface StringFoo extends Foo<String> {} > public class StringFooImpl implements StringFoo {} > > and > > @Inject StringFoo stringFoo; > > The proxy created by Weld is a subclass of StringFooImpl and > therefore has two declared methods: > > void doSomething(Object o) { doSomething((String) o); } > void doSomething(String) {...} > > However, when StringFooImpl is a session bean, with StringFoo as its > local interface, the proxy is a subclass of Object and therefore the > proxy only has the following declared method: > > void doSomething(Object o); > > In both cases, when a client invokes stringFoo.doSomething("foo"), > the method doSomething(Object) is invoked. But there's a difference > in what happens next: > > * In the non-ejb version, the bridge method then immediately > invokes doSomething(String) and only then is the proxy's method > handler invoked. The handler is therefore dealing with the method > doSomething(*String*) > * in the EJB version, doSomething(Object) is not a bridge method, > and so the method handler is invoked directly and it (the > handler) is operating on doSomething(*Object*). > > In the second case, this ultimately means that Weld will check > whether doSomething(Object) is intercepted. It isn't, since > Beans.getInterceptableMethods() is ignoring bridge methods. The > interceptor will not be invoked. On the other hand, in the first > case, the interceptor _will_ be invoked, since Weld will be checking > whether doSomething(String) is intercepted. > > My initial solution was to make Beans.getInterceptableMethods() also > return bridge methods, but now I'm thinking the actual problem is in > the proxy itself. IMO, when creating a proxy based on an interface, > we should also generate bridge methods on the proxy (this should be > either done by Weld or by Javassist directly). These bridge methods > should be perfectly normal bridge methods and should not invoke the > method handler directly. They should simply invoke the non-bridge > method and the non-bridge method should then invoke the method handler. > > The java compiler can't add bridge methods directly to interfaces > which require them, so it adds them to all the classes implementing > the interface (StringFooImpl in our case). Since we are creating > StringFoo$Proxy, which is also a class implementing an interface > which requires bridge methods, we should add the bridge methods > to it - exactly as the java compiler would. > > This would solve the interceptor problem and possibly other similar > problems as well. > > What do you think? > > Marko > > > _______________________________________________ > weld-dev mailing list > weld-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/weld-dev _______________________________________________ weld-dev mailing list weld-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-dev

Yes, when the proxy extends a class, everything is taken care of by the bridge method in the superclass. But, when dealing with local EJB interfaces, the proxy's superclass is Object, so there is no bridge method at all. For EJBs, the stacktrace would look something like this: StringFooImpl.doSth(String) StringFooImpl.doSth(Object) methodHandler.invoke("doSth(Object)") StringFoo$Proxy.doSth(Object) <-- proxy extends Object, implements StringFoo interface (no bridge method) For regular managed beans, the stacktrace would look like this: StringFooImpl.doSth(String) methodHandler.invoke("doSth(String)") StringFooImpl$Proxy.doSth(String) StringFooImpl$Proxy.doSth(Object) <-- proxy extends StringFooImpl (and has bridge method) Note the different methods methodHandler is handling. Marko On 7.9.2012 17:27, Stuart Douglas wrote: > IMHO the correct way to deal with this is to simply make bridge > methods delegate to super(), which will then result in the actual > intercepted method being called. > > To be honest I thought we already did this, as we have had multiple > related bugs in the past. Do you have a test case that I can look at? > > Stuart > >> Marko Lukša <mailto:marko.luksa@gmail.com> >> 7 September 2012 7:19 PM >> Hey all. >> >> I've been working on https://issues.jboss.org/browse/WELD-1162 and >> need your opinion. >> >> Say we have: >> >> public interface Foo<T> { >> void doSomething(T t); >> } >> public interface StringFoo extends Foo<String> {} >> public class StringFooImpl implements StringFoo {} >> >> and >> >> @Inject StringFoo stringFoo; >> >> The proxy created by Weld is a subclass of StringFooImpl and >> therefore has two declared methods: >> >> void doSomething(Object o) { doSomething((String) o); } >> void doSomething(String) {...} >> >> However, when StringFooImpl is a session bean, with StringFoo as its >> local interface, the proxy is a subclass of Object and therefore the >> proxy only has the following declared method: >> >> void doSomething(Object o); >> >> In both cases, when a client invokes stringFoo.doSomething("foo"), >> the method doSomething(Object) is invoked. But there's a difference >> in what happens next: >> >> * In the non-ejb version, the bridge method then immediately >> invokes doSomething(String) and only then is the proxy's method >> handler invoked. The handler is therefore dealing with the >> method doSomething(*String*) >> * in the EJB version, doSomething(Object) is not a bridge method, >> and so the method handler is invoked directly and it (the >> handler) is operating on doSomething(*Object*). >> >> In the second case, this ultimately means that Weld will check >> whether doSomething(Object) is intercepted. It isn't, since >> Beans.getInterceptableMethods() is ignoring bridge methods. The >> interceptor will not be invoked. On the other hand, in the first >> case, the interceptor _will_ be invoked, since Weld will be checking >> whether doSomething(String) is intercepted. >> >> My initial solution was to make Beans.getInterceptableMethods() also >> return bridge methods, but now I'm thinking the actual problem is in >> the proxy itself. IMO, when creating a proxy based on an interface, >> we should also generate bridge methods on the proxy (this should be >> either done by Weld or by Javassist directly). These bridge methods >> should be perfectly normal bridge methods and should not invoke the >> method handler directly. They should simply invoke the non-bridge >> method and the non-bridge method should then invoke the method handler. >> >> The java compiler can't add bridge methods directly to interfaces >> which require them, so it adds them to all the classes implementing >> the interface (StringFooImpl in our case). Since we are creating >> StringFoo$Proxy, which is also a class implementing an interface >> which requires bridge methods, we should add the bridge methods >> to it - exactly as the java compiler would. >> >> This would solve the interceptor problem and possibly other similar >> problems as well. >> >> What do you think? >> >> Marko >> >> >> _______________________________________________ >> weld-dev mailing list >> weld-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/weld-dev

Hey I need someone to take a look at https://github.com/weld/core/pull/225 My initial change was to add bridge methods to the proxy (when proxying an interface), but later realized this could never work, because the interceptors expect a Method object that would have to point to a non-existing method (the method exists only on the proxy, but not on the interface). That's why the only solution was to make Beans.getInterceptableMethods() return the bridge methods also. The reason why you guys had problems with this in the past was the fact that MethodReference was missing the return type and thus treated two methods with the same name and parameters but different return types as equal, when they clearly aren't. This is why Weld was throwing the duplicate interceptor definition errors. Marko On 7.9.2012 17:43, Marko Lukša wrote: > Yes, when the proxy extends a class, everything is taken care of by > the bridge method in the superclass. But, when dealing with local EJB > interfaces, the proxy's superclass is Object, so there is no bridge > method at all. > > For EJBs, the stacktrace would look something like this: > StringFooImpl.doSth(String) > StringFooImpl.doSth(Object) > methodHandler.invoke("doSth(Object)") > StringFoo$Proxy.doSth(Object) <-- proxy extends Object, implements > StringFoo interface (no bridge method) > > For regular managed beans, the stacktrace would look like this: > StringFooImpl.doSth(String) > methodHandler.invoke("doSth(String)") > StringFooImpl$Proxy.doSth(String) > StringFooImpl$Proxy.doSth(Object) <-- proxy extends StringFooImpl > (and has bridge method) > > Note the different methods methodHandler is handling. > > Marko > > > On 7.9.2012 17:27, Stuart Douglas wrote: >> IMHO the correct way to deal with this is to simply make bridge >> methods delegate to super(), which will then result in the actual >> intercepted method being called. >> >> To be honest I thought we already did this, as we have had multiple >> related bugs in the past. Do you have a test case that I can look at? >> >> Stuart >> >>> Marko Lukša <mailto:marko.luksa@gmail.com> >>> 7 September 2012 7:19 PM >>> Hey all. >>> >>> I've been working on https://issues.jboss.org/browse/WELD-1162 and >>> need your opinion. >>> >>> Say we have: >>> >>> public interface Foo<T> { >>> void doSomething(T t); >>> } >>> public interface StringFoo extends Foo<String> {} >>> public class StringFooImpl implements StringFoo {} >>> >>> and >>> >>> @Inject StringFoo stringFoo; >>> >>> The proxy created by Weld is a subclass of StringFooImpl and >>> therefore has two declared methods: >>> >>> void doSomething(Object o) { doSomething((String) o); } >>> void doSomething(String) {...} >>> >>> However, when StringFooImpl is a session bean, with StringFoo as its >>> local interface, the proxy is a subclass of Object and therefore the >>> proxy only has the following declared method: >>> >>> void doSomething(Object o); >>> >>> In both cases, when a client invokes stringFoo.doSomething("foo"), >>> the method doSomething(Object) is invoked. But there's a difference >>> in what happens next: >>> >>> * In the non-ejb version, the bridge method then immediately >>> invokes doSomething(String) and only then is the proxy's method >>> handler invoked. The handler is therefore dealing with the >>> method doSomething(*String*) >>> * in the EJB version, doSomething(Object) is not a bridge method, >>> and so the method handler is invoked directly and it (the >>> handler) is operating on doSomething(*Object*). >>> >>> In the second case, this ultimately means that Weld will check >>> whether doSomething(Object) is intercepted. It isn't, since >>> Beans.getInterceptableMethods() is ignoring bridge methods. The >>> interceptor will not be invoked. On the other hand, in the first >>> case, the interceptor _will_ be invoked, since Weld will be checking >>> whether doSomething(String) is intercepted. >>> >>> My initial solution was to make Beans.getInterceptableMethods() also >>> return bridge methods, but now I'm thinking the actual problem is in >>> the proxy itself. IMO, when creating a proxy based on an interface, >>> we should also generate bridge methods on the proxy (this should be >>> either done by Weld or by Javassist directly). These bridge methods >>> should be perfectly normal bridge methods and should not invoke the >>> method handler directly. They should simply invoke the non-bridge >>> method and the non-bridge method should then invoke the method handler. >>> >>> The java compiler can't add bridge methods directly to interfaces >>> which require them, so it adds them to all the classes implementing >>> the interface (StringFooImpl in our case). Since we are creating >>> StringFoo$Proxy, which is also a class implementing an interface >>> which requires bridge methods, we should add the bridge methods >>> to it - exactly as the java compiler would. >>> >>> This would solve the interceptor problem and possibly other similar >>> problems as well. >>> >>> What do you think? >>> >>> Marko >>> >>> >>> _______________________________________________ >>> weld-dev mailing list >>> weld-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/weld-dev > _______________________________________________ weld-dev mailing list weld-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/weld-dev

So this means we can drop Marko' fix? On Sep 25, 2012, at 2:24, Stuart Douglas&lt;stuart.w.douglas(a)gmail.com&gt; wrote: > This is the relevant AS7 fix: > > https://github.com/jbossas/jboss-as/pull/3112 > > Stuart > > Marko Lukša wrote: >> Hey >> >> I need someone to take a look at https://github.com/weld/core/pull/225 >> >> My initial change was to add bridge methods to the proxy (when proxying >> an interface), but later realized this could never work, because the >> interceptors expect a Method object that would have to point to a >> non-existing method (the method exists only on the proxy, but not on the >> interface). >> >> That's why the only solution was to make Beans.getInterceptableMethods() >> return the bridge methods also. The reason why you guys had problems >> with this in the past was the fact that MethodReference was missing the >> return type and thus treated two methods with the same name and >> parameters but different return types as equal, when they clearly >> aren't. This is why Weld was throwing the duplicate interceptor >> definition errors. >> >> Marko >> >> On 7.9.2012 17:43, Marko Lukša wrote: >>> Yes, when the proxy extends a class, everything is taken care of by >>> the bridge method in the superclass. But, when dealing with local EJB >>> interfaces, the proxy's superclass is Object, so there is no bridge >>> method at all. >>> >>> For EJBs, the stacktrace would look something like this: >>> StringFooImpl.doSth(String) >>> StringFooImpl.doSth(Object) >>> methodHandler.invoke("doSth(Object)") >>> StringFoo$Proxy.doSth(Object)<-- proxy extends Object, implements >>> StringFoo interface (no bridge method) >>> >>> For regular managed beans, the stacktrace would look like this: >>> StringFooImpl.doSth(String) >>> methodHandler.invoke("doSth(String)") >>> StringFooImpl$Proxy.doSth(String) >>> StringFooImpl$Proxy.doSth(Object)<-- proxy extends StringFooImpl >>> (and has bridge method) >>> >>> Note the different methods methodHandler is handling. >>> >>> Marko >>> >>> >>> On 7.9.2012 17:27, Stuart Douglas wrote: >>>> IMHO the correct way to deal with this is to simply make bridge >>>> methods delegate to super(), which will then result in the actual >>>> intercepted method being called. >>>> >>>> To be honest I thought we already did this, as we have had multiple >>>> related bugs in the past. Do you have a test case that I can look at? >>>> >>>> Stuart >>>> >>>>> Marko Lukša<mailto:marko.luksa@gmail.com> >>>>> 7 September 2012 7:19 PM >>>>> Hey all. >>>>> >>>>> I've been working on https://issues.jboss.org/browse/WELD-1162 and >>>>> need your opinion. >>>>> >>>>> Say we have: >>>>> >>>>> public interface Foo<T> { >>>>> void doSomething(T t); >>>>> } >>>>> public interface StringFoo extends Foo<String> {} >>>>> public class StringFooImpl implements StringFoo {} >>>>> >>>>> and >>>>> >>>>> @Inject StringFoo stringFoo; >>>>> >>>>> The proxy created by Weld is a subclass of StringFooImpl and >>>>> therefore has two declared methods: >>>>> >>>>> void doSomething(Object o) { doSomething((String) o); } >>>>> void doSomething(String) {...} >>>>> >>>>> However, when StringFooImpl is a session bean, with StringFoo as its >>>>> local interface, the proxy is a subclass of Object and therefore the >>>>> proxy only has the following declared method: >>>>> >>>>> void doSomething(Object o); >>>>> >>>>> In both cases, when a client invokes stringFoo.doSomething("foo"), >>>>> the method doSomething(Object) is invoked. But there's a difference >>>>> in what happens next: >>>>> >>>>> * In the non-ejb version, the bridge method then immediately >>>>> invokes doSomething(String) and only then is the proxy's method >>>>> handler invoked. The handler is therefore dealing with the >>>>> method doSomething(*String*) >>>>> * in the EJB version, doSomething(Object) is not a bridge method, >>>>> and so the method handler is invoked directly and it (the >>>>> handler) is operating on doSomething(*Object*). >>>>> >>>>> In the second case, this ultimately means that Weld will check >>>>> whether doSomething(Object) is intercepted. It isn't, since >>>>> Beans.getInterceptableMethods() is ignoring bridge methods. The >>>>> interceptor will not be invoked. On the other hand, in the first >>>>> case, the interceptor _will_ be invoked, since Weld will be checking >>>>> whether doSomething(String) is intercepted. >>>>> >>>>> My initial solution was to make Beans.getInterceptableMethods() also >>>>> return bridge methods, but now I'm thinking the actual problem is in >>>>> the proxy itself. IMO, when creating a proxy based on an interface, >>>>> we should also generate bridge methods on the proxy (this should be >>>>> either done by Weld or by Javassist directly). These bridge methods >>>>> should be perfectly normal bridge methods and should not invoke the >>>>> method handler directly. They should simply invoke the non-bridge >>>>> method and the non-bridge method should then invoke the method handler. >>>>> >>>>> The java compiler can't add bridge methods directly to interfaces >>>>> which require them, so it adds them to all the classes implementing >>>>> the interface (StringFooImpl in our case). Since we are creating >>>>> StringFoo$Proxy, which is also a class implementing an interface >>>>> which requires bridge methods, we should add the bridge methods >>>>> to it - exactly as the java compiler would. >>>>> >>>>> This would solve the interceptor problem and possibly other similar >>>>> problems as well. >>>>> >>>>> What do you think? >>>>> >>>>> Marko >>>>> >>>>> >>>>> _______________________________________________ >>>>> weld-dev mailing list >>>>> weld-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >> _______________________________________________ >> weld-dev mailing list >> weld-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/weld-dev > _______________________________________________ > weld-dev mailing list > weld-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/weld-dev