Author: markt
Date: Wed Aug 28 19:02:39 2013
New Revision: 1518328

URL: http://svn.apache.org/r1518328
Log:
Partial fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=55483
This fixes calling overloaded methods and aligns the method matching code in 
the API and the implementation. Unfortunately, this has required a fair amount 
of duplication. I don't see a way to avoid this, short of an el-util JAR that 
both depend on.

Added:
    tomcat/trunk/test/javax/el/TestUtil.java   (with props)
Modified:
    tomcat/trunk/java/javax/el/LocalStrings.properties
    tomcat/trunk/java/javax/el/Util.java
    tomcat/trunk/java/org/apache/el/parser/AstValue.java
    tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java

Modified: tomcat/trunk/java/javax/el/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/LocalStrings.properties?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/LocalStrings.properties (original)
+++ tomcat/trunk/java/javax/el/LocalStrings.properties Wed Aug 28 19:02:39 2013
@@ -45,4 +45,7 @@ lambdaExpression.tooFewArgs=Only [{0}] a
 
 staticFieldELResolver.methodNotFound=No matching public static method named 
[{0}] found on class [{1}]
 staticFieldELResolver.notFound=No public static field named [{0}] was found on 
class [{1}]
-staticFieldELResolver.notWriteable=Writing to static fields (in this case 
field [{0}] on class [{1}]) is not permitted
\ No newline at end of file
+staticFieldELResolver.notWriteable=Writing to static fields (in this case 
field [{0}] on class [{1}]) is not permitted
+
+util.method.notfound=Method not found: {0}.{1}({2})
+util.method.ambiguous=Unable to find unambiguous method: {0}.{1}({2})

Modified: tomcat/trunk/java/javax/el/Util.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/Util.java?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/Util.java (original)
+++ tomcat/trunk/java/javax/el/Util.java Wed Aug 28 19:02:39 2013
@@ -22,9 +22,12 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.text.MessageFormat;
+import java.util.HashMap;
 import java.util.Locale;
+import java.util.Map;
 import java.util.MissingResourceException;
 import java.util.ResourceBundle;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.locks.Lock;
@@ -185,48 +188,311 @@ class Util {
     }
 
 
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    @SuppressWarnings("null")
     static Method findMethod(Class<?> clazz, String methodName,
-            Class<?>[] paramTypes, Object[] params) {
+            Class<?>[] paramTypes, Object[] paramValues) {
 
-        Method matchingMethod = null;
+        if (clazz == null || methodName == null) {
+            throw new MethodNotFoundException(
+                    message(null, "util.method.notfound", clazz, methodName,
+                    paramString(paramTypes)));
+        }
 
-        if (paramTypes != null) {
-            try {
-                matchingMethod =
-                    getMethod(clazz, clazz.getMethod(methodName, paramTypes));
-            } catch (NoSuchMethodException e) {
-                throw new MethodNotFoundException(e);
-            }
+        int paramCount;
+        if (paramTypes == null) {
+            paramTypes = getTypesFromValues(paramValues);
+        }
+
+        if (paramTypes == null) {
+            paramCount = 0;
         } else {
-            int paramCount = 0;
-            if (params != null) {
-                paramCount = params.length;
+            paramCount = paramTypes.length;
+        }
+
+        Method[] methods = clazz.getMethods();
+        Map<Method,Integer> candidates = new HashMap<>();
+
+        for (Method m : methods) {
+            if (!m.getName().equals(methodName)) {
+                // Method name doesn't match
+                continue;
             }
-            Method[] methods = clazz.getMethods();
-            for (Method m : methods) {
-                if (methodName.equals(m.getName())) {
-                    if (m.getParameterTypes().length == paramCount) {
-                        // Same number of parameters - use the first match
-                        matchingMethod = getMethod(clazz, m);
+
+            Class<?>[] mParamTypes = m.getParameterTypes();
+            int mParamCount;
+            if (mParamTypes == null) {
+                mParamCount = 0;
+            } else {
+                mParamCount = mParamTypes.length;
+            }
+
+            // Check the number of parameters
+            if (!(paramCount == mParamCount ||
+                    (m.isVarArgs() && paramCount >= mParamCount))) {
+                // Method has wrong number of parameters
+                continue;
+            }
+
+            // Check the parameters match
+            int exactMatch = 0;
+            boolean noMatch = false;
+            for (int i = 0; i < mParamCount; i++) {
+                // Can't be null
+                if (mParamTypes[i].equals(paramTypes[i])) {
+                    exactMatch++;
+                } else if (i == (mParamCount - 1) && m.isVarArgs()) {
+                    Class<?> varType = mParamTypes[i].getComponentType();
+                    for (int j = i; j < paramCount; j++) {
+                        if (!isAssignableFrom(paramTypes[j], varType)) {
+                            if (paramValues == null) {
+                                noMatch = true;
+                                break;
+                            } else {
+                                if (!isCoercibleFrom(paramValues[j], varType)) 
{
+                                    noMatch = true;
+                                    break;
+                                }
+                            }
+                        }
+                        // Don't treat a varArgs match as an exact match, it 
can
+                        // lead to a varArgs method matching when the result
+                        // should be ambiguous
+                    }
+                } else if (!isAssignableFrom(paramTypes[i], mParamTypes[i])) {
+                    if (paramValues == null) {
+                        noMatch = true;
                         break;
+                    } else {
+                        if (!isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+                            noMatch = true;
+                            break;
+                        }
                     }
-                    if (m.isVarArgs()
-                            && paramCount > m.getParameterTypes().length - 2) {
-                        matchingMethod = getMethod(clazz, m);
+                }
+            }
+            if (noMatch) {
+                continue;
+            }
+
+            // If a method is found where every parameter matches exactly,
+            // return it
+            if (exactMatch == paramCount) {
+                return getMethod(clazz, m);
+            }
+
+            candidates.put(m, Integer.valueOf(exactMatch));
+        }
+
+        // Look for the method that has the highest number of parameters where
+        // the type matches exactly
+        int bestMatch = 0;
+        Method match = null;
+        boolean multiple = false;
+        for (Map.Entry<Method, Integer> entry : candidates.entrySet()) {
+            if (entry.getValue().intValue() > bestMatch ||
+                    match == null) {
+                bestMatch = entry.getValue().intValue();
+                match = entry.getKey();
+                multiple = false;
+            } else if (entry.getValue().intValue() == bestMatch) {
+                multiple = true;
+            }
+        }
+        if (multiple) {
+            if (bestMatch == paramCount - 1) {
+                // Only one parameter is not an exact match - try using the
+                // super class
+                match = resolveAmbiguousMethod(candidates.keySet(), 
paramTypes);
+            } else {
+                match = null;
+            }
+
+            if (match == null) {
+                // If multiple methods have the same matching number of 
parameters
+                // the match is ambiguous so throw an exception
+                throw new MethodNotFoundException(message(
+                        null, "util.method.ambiguous", clazz, methodName,
+                        paramString(paramTypes)));
+                }
+        }
+
+        // Handle case where no match at all was found
+        if (match == null) {
+            throw new MethodNotFoundException(message(
+                        null, "util.method.notfound", clazz, methodName,
+                        paramString(paramTypes)));
+        }
+
+        return getMethod(clazz, match);
+    }
+
+
+    private static final String paramString(Class<?>[] types) {
+        if (types != null) {
+            StringBuilder sb = new StringBuilder();
+            for (int i = 0; i < types.length; i++) {
+                if (types[i] == null) {
+                    sb.append("null, ");
+                } else {
+                    sb.append(types[i].getName()).append(", ");
+                }
+            }
+            if (sb.length() > 2) {
+                sb.setLength(sb.length() - 2);
+            }
+            return sb.toString();
+        }
+        return null;
+    }
+
+
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    private static Method resolveAmbiguousMethod(Set<Method> candidates,
+            Class<?>[] paramTypes) {
+        // Identify which parameter isn't an exact match
+        Method m = candidates.iterator().next();
+
+        int nonMatchIndex = 0;
+        Class<?> nonMatchClass = null;
+
+        for (int i = 0; i < paramTypes.length; i++) {
+            if (m.getParameterTypes()[i] != paramTypes[i]) {
+                nonMatchIndex = i;
+                nonMatchClass = paramTypes[i];
+                break;
+            }
+        }
+
+        if (nonMatchClass == null) {
+            // Null will always be ambiguous
+            return null;
+        }
+
+        for (Method c : candidates) {
+           if (c.getParameterTypes()[nonMatchIndex] ==
+                   paramTypes[nonMatchIndex]) {
+               // Methods have different non-matching parameters
+               // Result is ambiguous
+               return null;
+           }
+        }
+
+        // Can't be null
+        Class<?> superClass = nonMatchClass.getSuperclass();
+        while (superClass != null) {
+            for (Method c : candidates) {
+                if (c.getParameterTypes()[nonMatchIndex].equals(superClass)) {
+                    // Found a match
+                    return c;
+                }
+            }
+            superClass = superClass.getSuperclass();
+        }
+
+        // Treat instances of Number as a special case
+        Method match = null;
+        if (Number.class.isAssignableFrom(nonMatchClass)) {
+            for (Method c : candidates) {
+                Class<?> candidateType = c.getParameterTypes()[nonMatchIndex];
+                if (Number.class.isAssignableFrom(candidateType) ||
+                        candidateType.isPrimitive()) {
+                    if (match == null) {
+                        match = c;
+                    } else {
+                        // Match still ambiguous
+                        match = null;
+                        break;
                     }
                 }
             }
-            if (matchingMethod == null) {
-                throw new MethodNotFoundException(
-                        "Unable to find method [" + methodName + "] with ["
-                        + paramCount + "] parameters");
+        }
+
+        return match;
+    }
+
+
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    private static boolean isAssignableFrom(Class<?> src, Class<?> target) {
+        // src will always be an object
+        // Short-cut. null is always assignable to an object and in EL null
+        // can always be coerced to a valid value for a primitive
+        if (src == null) {
+            return true;
+        }
+
+        Class<?> targetClass;
+        if (target.isPrimitive()) {
+            if (target == Boolean.TYPE) {
+                targetClass = Boolean.class;
+            } else if (target == Character.TYPE) {
+                targetClass = Character.class;
+            } else if (target == Byte.TYPE) {
+                targetClass = Byte.class;
+            } else if (target == Short.TYPE) {
+                targetClass = Short.class;
+            } else if (target == Integer.TYPE) {
+                targetClass = Integer.class;
+            } else if (target == Long.TYPE) {
+                targetClass = Long.class;
+            } else if (target == Float.TYPE) {
+                targetClass = Float.class;
+            } else {
+                targetClass = Double.class;
             }
+        } else {
+            targetClass = target;
+        }
+        return targetClass.isAssignableFrom(src);
+    }
+
+
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    private static boolean isCoercibleFrom(Object src, Class<?> target) {
+        // TODO: This isn't pretty but it works. Significant refactoring would
+        //       be required to avoid the exception.
+        try {
+            getExpressionFactory().coerceToType(src, target);
+        } catch (ELException e) {
+            return false;
+        }
+        return true;
+    }
+
+
+    private static Class<?>[] getTypesFromValues(Object[] values) {
+        if (values == null) {
+            return null;
         }
 
-        return matchingMethod;
+        Class<?> result[] = new Class<?>[values.length];
+        for (int i = 0; i < values.length; i++) {
+            if (values[i] == null) {
+                result[i] = null;
+            } else {
+                result[i] = values[i].getClass();
+            }
+        }
+        return result;
     }
 
 
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
     static Method getMethod(Class<?> type, Method m) {
         if (m == null || Modifier.isPublic(type.getModifiers())) {
             return m;

Modified: tomcat/trunk/java/org/apache/el/parser/AstValue.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/parser/AstValue.java?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/el/parser/AstValue.java (original)
+++ tomcat/trunk/java/org/apache/el/parser/AstValue.java Wed Aug 28 19:02:39 
2013
@@ -154,8 +154,9 @@ public final class AstValue extends Simp
                     }
                 }
                 // This is a method
-                base = resolver.invoke(ctx, base, suffix, null,
-                        mps.getParameters(ctx));
+                Object[] paramValues = mps.getParameters(ctx);
+                base = resolver.invoke(ctx, base, suffix,
+                        getTypesFromValues(paramValues), paramValues);
                 i+=2;
             } else {
                 // This is a property

Modified: tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java (original)
+++ tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java Wed Aug 28 
19:02:39 2013
@@ -18,6 +18,7 @@ package org.apache.el.util;
 
 import java.lang.reflect.Array;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -105,7 +106,7 @@ public class ReflectionUtil {
     }
 
     /**
-     * Returns a method based on the criteria
+     * Returns a method based on the criteria.
      * @param base the object that owns the method
      * @param property the name of the method
      * @param paramTypes the parameter types to use
@@ -113,6 +114,10 @@ public class ReflectionUtil {
      * @return the method specified
      * @throws MethodNotFoundException
      */
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     @SuppressWarnings("null")
     public static Method getMethod(Object base, Object property,
             Class<?>[] paramTypes, Object[] paramValues)
@@ -201,7 +206,7 @@ public class ReflectionUtil {
             // If a method is found where every parameter matches exactly,
             // return it
             if (exactMatch == paramCount) {
-                return m;
+                getMethod(base.getClass(), m);
             }
 
             candidates.put(m, Integer.valueOf(exactMatch));
@@ -247,9 +252,13 @@ public class ReflectionUtil {
                         paramString(paramTypes)));
         }
 
-        return match;
+        return getMethod(base.getClass(), match);
     }
 
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     private static Method resolveAmbiguousMethod(Set<Method> candidates,
             Class<?>[] paramTypes) {
         // Identify which parameter isn't an exact match
@@ -281,23 +290,45 @@ public class ReflectionUtil {
         }
 
         // Can't be null
-        nonMatchClass = nonMatchClass.getSuperclass();
-        while (nonMatchClass != null) {
+        Class<?> superClass = nonMatchClass.getSuperclass();
+        while (superClass != null) {
             for (Method c : candidates) {
-                if (c.getParameterTypes()[nonMatchIndex].equals(
-                        nonMatchClass)) {
+                if (c.getParameterTypes()[nonMatchIndex].equals(superClass)) {
                     // Found a match
                     return c;
                 }
             }
-            nonMatchClass = nonMatchClass.getSuperclass();
+            superClass = superClass.getSuperclass();
         }
 
-        return null;
+        // Treat instances of Number as a special case
+        Method match = null;
+        if (Number.class.isAssignableFrom(nonMatchClass)) {
+            for (Method c : candidates) {
+                Class<?> candidateType = c.getParameterTypes()[nonMatchIndex];
+                if (Number.class.isAssignableFrom(candidateType) ||
+                        candidateType.isPrimitive()) {
+                    if (match == null) {
+                        match = c;
+                    } else {
+                        // Match still ambiguous
+                        match = null;
+                        break;
+                    }
+                }
+            }
+        }
+
+        return match;
     }
 
-    // src will always be an object
+
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     private static boolean isAssignableFrom(Class<?> src, Class<?> target) {
+        // src will always be an object
         // Short-cut. null is always assignable to an object and in EL null
         // can always be coerced to a valid value for a primitive
         if (src == null) {
@@ -329,6 +360,11 @@ public class ReflectionUtil {
         return targetClass.isAssignableFrom(src);
     }
 
+
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     private static boolean isCoercibleFrom(Object src, Class<?> target) {
         // TODO: This isn't pretty but it works. Significant refactoring would
         //       be required to avoid the exception.
@@ -340,7 +376,45 @@ public class ReflectionUtil {
         return true;
     }
 
-    protected static final String paramString(Class<?>[] types) {
+
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
+    private static Method getMethod(Class<?> type, Method m) {
+        if (m == null || Modifier.isPublic(type.getModifiers())) {
+            return m;
+        }
+        Class<?>[] inf = type.getInterfaces();
+        Method mp = null;
+        for (int i = 0; i < inf.length; i++) {
+            try {
+                mp = inf[i].getMethod(m.getName(), m.getParameterTypes());
+                mp = getMethod(mp.getDeclaringClass(), mp);
+                if (mp != null) {
+                    return mp;
+                }
+            } catch (NoSuchMethodException e) {
+                // Ignore
+            }
+        }
+        Class<?> sup = type.getSuperclass();
+        if (sup != null) {
+            try {
+                mp = sup.getMethod(m.getName(), m.getParameterTypes());
+                mp = getMethod(mp.getDeclaringClass(), mp);
+                if (mp != null) {
+                    return mp;
+                }
+            } catch (NoSuchMethodException e) {
+                // Ignore
+            }
+        }
+        return null;
+    }
+
+
+    private static final String paramString(Class<?>[] types) {
         if (types != null) {
             StringBuilder sb = new StringBuilder();
             for (int i = 0; i < types.length; i++) {

Added: tomcat/trunk/test/javax/el/TestUtil.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestUtil.java?rev=1518328&view=auto
==============================================================================
--- tomcat/trunk/test/javax/el/TestUtil.java (added)
+++ tomcat/trunk/test/javax/el/TestUtil.java Wed Aug 28 19:02:39 2013
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javax.el;
+
+import java.util.Date;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestUtil {
+
+    @Test
+    public void test01() {
+        ELProcessor processor = new ELProcessor();
+        processor.defineBean("sb", new StringBuilder());
+        Assert.assertEquals("a", processor.eval("sb.append('a'); 
sb.toString()"));
+    }
+
+
+    // Commented out as this is not yet fixed @Test
+    public void test02() {
+        ELProcessor processor = new ELProcessor();
+        processor.getELManager().importClass("java.util.Date");
+        Date result = (Date) processor.eval("Date(86400)");
+        Assert.assertEquals(86400, result.getTime());
+    }
+}

Propchange: tomcat/trunk/test/javax/el/TestUtil.java
------------------------------------------------------------------------------
    svn:eol-style = native



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to