I think so. Not sure how this affects Glassfish though. Stuart
Ales Justin wrote: > So this means we can drop Marko' fix? > > On Sep 25, 2012, at 2:24, Stuart Douglas<[email protected]> 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:[email protected]> >>>>>> 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 >>>>>> [email protected] >>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>> _______________________________________________ >>> weld-dev mailing list >>> [email protected] >>> https://lists.jboss.org/mailman/listinfo/weld-dev >> _______________________________________________ >> weld-dev mailing list >> [email protected] >> https://lists.jboss.org/mailman/listinfo/weld-dev _______________________________________________ weld-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/weld-dev
