Community
Participate
Working Groups
Build Identifier: searchForMethod method in SharedObjectMsg returns the method if the number of arguments of the tested method is equal to the number of arguments given without testing if the types of the arguments are equal. For me, the following solution worked: public static Method searchForMethod(Method meths[], String meth, Class args[]) { // Find it from among the given set of Method objects for (int i = 0; i < meths.length; i++) { Method test = meths[i]; if (test.getName().equals(meth)) { if (test.getParameterTypes().length == args.length) { /////////////////// new /////////////////////////////// boolean isAssignable = true; Class[] testParams = test.getParameterTypes(); for (int j = 0; j < args.length; j++) { if (args[j] != null) { isAssignable &= testParams[j].isAssignableFrom(args[j]); } } if (isAssignable) { /////////////////// new /////////////////////////////// return test; } } } } return null; } Reproducible: Always Steps to Reproduce: 1. Create a remote service with two methods having same name and number of arguments but different types. 2. Call both methods - one of it isn't called because of an invalid argument exception
Who ever is going to work on this might want to also have a look at org.eclipse.ecf.internal.provider.r_osgi.RemoteServiceImpl.getMethod(Class, String, Class[]). It is the code that is used in the r-OSGi provider. It might even make sense to push it out into a common bundle.
Wow...this is bug deja vu. You might not believe this, but I've been thinking about this bug for a couple of days now, and not had a chance to actually file it (and repair it). So this bug filing is very apropos. Assigning to me...I will try to get to it in a couple of days. And I will consult with the r osgi code, as it is boilerplate.
Fix released to HEAD. As per comment 1 the getMethod method was moved up to AbstractRemoteService and made public static. Also added to SharedObjectMsg in slightly different form (to accomodate using non-public methods). So org.eclipse.ecf.remoteservice, org.eclipse.ecf.sharedobject, and org.eclipse.ecf.provider.r-osgi have to be updated to get this fix. Fix released to HEAD. Resolving as fixed.
Created attachment 180097 [details] org.eclipse.ecf.core.util.ReflectionUtil.getMethod(...) IMHO getMethod(...) should be pushed into a ReflectionUtil class (package org.eclipse.ecf.core.util) in bundle org.eclipse.ecf or org.eclipse.ecf.identity (although reflection has nothing to do with .identity) instead, so that no code is duplicate. One drawback that comes up though, is that the bundle minor version for org.eclipse.ecf has to be incremented. Maybe to solve this problem we could create a dedicated org.eclipse.ecf.util bundle that becomes home for all util classes (StringUtil, ReflectionUtil, WhatnotUtil, ...)
Created attachment 180098 [details] mylyn/context/zip
(In reply to comment #4) > Created an attachment (id=180097) [details] > org.eclipse.ecf.core.util.ReflectionUtil.getMethod(...) > > IMHO getMethod(...) should be pushed into a ReflectionUtil class (package > org.eclipse.ecf.core.util) in bundle org.eclipse.ecf or > org.eclipse.ecf.identity (although reflection has nothing to do with .identity) > instead, so that no code is duplicate. > One drawback that comes up though, is that the bundle minor version for > org.eclipse.ecf has to be incremented. Maybe to solve this problem we could > create a dedicated org.eclipse.ecf.util bundle that becomes home for all util > classes (StringUtil, ReflectionUtil, WhatnotUtil, ...) For the moment, I'm not in favor of creating a new ReflectionUtil class in ECF core. Why? A couple of reasons: 1) the two implementations of getMethod that are now in place...one in AbstractRemoteService, and the other in org.eclipse.ecf.core.sharedobject.SharedObjectMsg differ in their implementation (they are not exactly the same...in AbstractRemoteService the reflective lookup of the Method is based upon Class.getMethod(), and in SharedObjectMsg it's based upon Class.getDeclaredMethod(). This is necessary for the different use cases. It's true that ReflectionUtil.getMethod could have a signature like public Method getMethod(Method[] candidates, final Class[] parameterTypes) throws NoSuchMethodException and be shared across both implementations...and perhaps it's a good idea to eventually do this/put this code in ReflectionUtil. 2) Since the ECF core bundles are mostly deployed via Eclipse (i.e. to support file transfer), it's going to complicate usage of ECF SDK/remote services to introduce hard dependencies to core. It will also complicate our own releng for the Eclipse contribution...and I've got enough struggles with performing those releng duties...since the Platform provides exactly zero support to use to perform these releng tasks (i.e. build/deploy in accord with the Platform's schedule, their testing procedures, etc., etc.). So this is just a practical issue for me...I can't spend any more effort over the next few months on ECF's platform contribution releng (and interacting with the Platform/p2 team...etc)...and making a new util class in core will mean that any ECF SDK consumers will have to get new ECF core, meaning that either they get Eclipse 3.6 stream (once we've made a new contribution to that stream), or they get the ECF core feature patch (for 3.5.x)..in either case it seems like a lot of additional work that I can't currently afford and potentially a time-comsuming support issue (for example, we'll have to make sure that everyone that wants to use ECF remote services on 3.5.X gets the ECF core feature patch...and if they don't things won't work properly, etc). So on balance, this doesn't seem like an extremely important thing to me (to create/add ReflectionUtil to ECF core)...especially compared to other things that are competing for resources/dev time (like Remote Service Admin impl).
(In reply to comment #6) > 2) Since the ECF core bundles are mostly deployed via Eclipse (i.e. to support > file transfer), it's going to complicate usage of ECF SDK/remote services to > introduce hard dependencies to core. It will also complicate our own releng > for the Eclipse contribution...and I've got enough struggles with performing > those releng duties...since the Platform provides exactly zero support to use > to perform these releng tasks (i.e. build/deploy in accord with the Platform's > schedule, their testing procedures, etc., etc.). So this is just a practical > issue for me...I can't spend any more effort over the next few months on ECF's > platform contribution releng (and interacting with the Platform/p2 > team...etc)...and making a new util class in core will mean that any ECF SDK > consumers will have to get new ECF core, meaning that either they get Eclipse > 3.6 stream (once we've made a new contribution to that stream), or they get the > ECF core feature patch (for 3.5.x)..in either case it seems like a lot of > additional work that I can't currently afford and potentially a time-comsuming > support issue (for example, we'll have to make sure that everyone that wants to > use ECF remote services on 3.5.X gets the ECF core feature patch...and if they > don't things won't work properly, etc). True, platform really slows down our chance for innovation by blocking updates to org.eclipse.ecf. On second thoughts, bundle org.eclipse.ecf is probably not a good home for ReflectionUtil/ClassUtil anyway. Reflection is only used in the sharedobject/remoteservice area of ECF and thus org.eclipse.ecf is far too generic to host it. Hence I have rewritten the patch so that ClassUtil lives in org.eclipse.ecf.sharedobject now. For r-OSGi this adds a new (explicit) dependency towards package "org.eclipse.ecf.core.util.reflection" exported by o.e.e.sharedobject. o.e.e.provider.r_osgi depended on o.e.e.sharedobject previously anyway (implicitly via o.e.e.provider). It makes the org.eclipse.ecf.remoteservice version increment to 4.2 unnecessary.
Created attachment 180113 [details] patch
Created attachment 180114 [details] mylyn/context/zip
(In reply to comment #8) > Created an attachment (id=180113) [details] > patch Patch released to HEAD. Thanks.
