commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 31006] New: - MethodUtils getMatchingAccessibleMethod has non-deterministic behavior
Date Thu, 02 Sep 2004 00:02:54 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=31006>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=31006

MethodUtils getMatchingAccessibleMethod has non-deterministic behavior

           Summary: MethodUtils getMatchingAccessibleMethod has non-
                    deterministic behavior
           Product: Commons
           Version: 1.0 Alpha
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: Normal
          Priority: Other
         Component: Bean Utilities
        AssignedTo: commons-dev@jakarta.apache.org
        ReportedBy: scohen@scohen.org


MethodUtils.getMatchingAccessibleMethod() returns the first method whose
parameters are assignment compatible with the list of classes passed in. Thus,
if you have a source file with the following signatures:

public String foo(Object param);
public String foo(String param);

the first method will always be called, even if you provide a string in the
argument list. 
I've created a patch:

Index: MethodUtils.java
===================================================================
RCS file:
/home/cvspublic/jakarta-commons/beanutils/src/java/org/apache/commons/beanutils/MethodUtils.java,v
retrieving revision 1.27
diff -u -r1.27 MethodUtils.java
--- MethodUtils.java	18 Jul 2004 14:58:22 -0000	1.27
+++ MethodUtils.java	2 Sep 2004 00:00:30 -0000
@@ -26,6 +26,8 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import sun.rmi.runtime.GetThreadPoolAction;
+
 
 
 /**
@@ -46,8 +48,8 @@
  * @author Craig R. McClanahan
  * @author Ralph Schaer
  * @author Chris Audley
- * @author Rey Fran�ois
- * @author Gregor Ra�
+ * @author Rey Fran�ois
+ * @author Gregor Ra�man
  * @author Jan Sorensen
  * @author Robert Burrell Donkin
  */
@@ -560,22 +562,22 @@
             } catch (SecurityException se) {
                 // log but continue just in case the method.invoke works anyway
                 if (!loggedAccessibleWarning) {
-                    boolean vunerableJVM = false;
+                    boolean vulnerableJVM = false;
                     try {
                         String specVersion =
System.getProperty("java.specification.version");
                         if (specVersion.charAt(0) == '1' && 
-                                (specVersion.charAt(0) == '0' ||
-                                 specVersion.charAt(0) == '1' ||
-                                 specVersion.charAt(0) == '2' ||
-                                 specVersion.charAt(0) == '3')) {
+                                (specVersion.charAt(2) == '0' ||
+                                 specVersion.charAt(2) == '1' ||
+                                 specVersion.charAt(2) == '2' ||
+                                 specVersion.charAt(2) == '3')) {
                                  
-                            vunerableJVM = true;
+                            vulnerableJVM = true;
                         }
                     } catch (SecurityException e) {
                         // don't know - so display warning
-                        vunerableJVM = true;
+                        vulnerableJVM = true;
                     }
-                    if (vunerableJVM) {
+                    if (vulnerableJVM) {
                         log.warn(
                             "Current Security Manager restricts use of
workarounds for reflection bugs "
                             + " in pre-1.4 JVMs.");
@@ -593,7 +595,10 @@
         
         // search through all methods 
         int paramSize = parameterTypes.length;
+        Method bestMatch = null;
         Method[] methods = clazz.getMethods();
+        float bestMatchCost = Float.MAX_VALUE;
+        float myCost = Float.MAX_VALUE;
         for (int i = 0, size = methods.length; i < size ; i++) {
             if (methods[i].getName().equals(methodName)) {	
                 // log some trace information
@@ -648,8 +653,11 @@
             "Cannot setAccessible on method. Therefore cannot use jvm access
bug workaround.", 
                                         se);
                             }
-                            cache.put(md, method);
-                            return method;
+                            myCost =
getTotalTransformationCost(parameterTypes,method.getParameterTypes());
+                            if ( myCost < bestMatchCost ) {
+                            	bestMatch = method;
+                            	bestMatchCost = myCost;
+                            }
                         }
                         
                         log.trace("Couldn't find accessible method.");
@@ -657,12 +665,71 @@
                 }
             }
         }
-        
+        if ( bestMatch != null ){
+        	  cache.put(md, bestMatch);  
+        } else {
         // didn't find a match
-        log.trace("No match found.");
-        return null;                                        
+        	log.trace("No match found.");
+        }
+        
+        return bestMatch;                                        
     }
 
+    /**
+     * Returns the sum of the object transformation cost for each class in the
source
+     * argument list.
+     * @param srcArgs The list of arguments to transform
+     * @param destArgs 
+     * @return
+     */
+    private static float getTotalTransformationCost(Class[] srcArgs, Class[]
destArgs) {
+
+        float totalCost = 0.0f;
+        for (int i = 0; i < srcArgs.length; i++) {
+            Class srcClass, destClass;
+            srcClass = srcArgs[i];
+            destClass = destArgs[i];
+            totalCost += getObjectTransformationCost(srcClass, destClass);
+        }
+
+        return totalCost;
+    }
+    
+    /**
+     * Gets the number of steps required needed to turn the source class into the 
+     * destination class. This represents the number of steps in the object
hierarchy 
+     * graph.
+     * @param srcClass
+     * @param destClass
+     * @return
+     */
+    private static float getObjectTransformationCost(Class srcClass, Class
destClass) {
+        float cost = 0.0f;
+        while (destClass != null && !destClass.equals(srcClass)) {
+            if (destClass.isInterface() &&
isAssignmentCompatible(destClass,srcClass)) {
+                // slight penalty for interface match. 
+                // we still want an exact match to override an interface match,
but  
+                // an interface match should override anything where we have to
get a 
+                // superclass.
+                cost += 0.25f;
+                break;
+            }
+            cost++;
+            destClass = destClass.getSuperclass();
+        }
+
+        /*
+         * If the destination class is null, we've travelled all the way up to 
+         * an Object match. We'll penalize this by adding 1.5 to the cost.
+         */
+        if (destClass == null) {
+            cost += 1.5f;
+        }
+
+        return cost;
+    }
+    
+    
     /**
      * <p>Determine whether a type can be used as a parameter in a method
invocation.
      * This method handles primitive conversions correctly.</p>

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message