8156043: Unstable behavior of PropertyDescriptor's getWriteMethod() in case of overloaded setters

Reviewed-by: alexsch, ssadetsky

jdk/src/java.desktop/share/classes/com/sun/beans/introspect/MethodInfo.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -22,6 +22,7 @@
  * or visit www.oracle.com if you need additional information or have any
  * questions.
  */
+
 package com.sun.beans.introspect;
 
 import com.sun.beans.TypeResolver;
@@ -31,7 +32,9 @@ import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 
 final class MethodInfo {
@@ -87,8 +90,60 @@ final class MethodInfo {
                 }
             }
         }
-        return (list != null)
-                ? Collections.unmodifiableList(list)
-                : Collections.emptyList();
+        if (list != null) {
+            list.sort(MethodOrder.instance);
+            return Collections.unmodifiableList(list);
+        }
+        return Collections.emptyList();
+    }
+
+    /**
+     * A comparator that defines a total order so that methods have the same
+     * name and identical signatures appear next to each others. The methods are
+     * sorted in such a way that methods which override each other will sit next
+     * to each other, with the overridden method last - e.g. is Integer getFoo()
+     * placed before Object getFoo().
+     **/
+    private static final class MethodOrder implements Comparator<Method> {
+
+        /*
+         * Code particularly was copied from com.sun.jmx.mbeanserver.MethodOrder
+         */
+        @Override
+        public int compare(final Method a, final Method b) {
+            int cmp = a.getName().compareTo(b.getName());
+            if (cmp != 0) {
+                return cmp;
+            }
+            final Class<?>[] aparams = a.getParameterTypes();
+            final Class<?>[] bparams = b.getParameterTypes();
+            if (aparams.length != bparams.length) {
+                return aparams.length - bparams.length;
+            }
+            for (int i = 0; i < aparams.length; ++i) {
+                final Class<?> aparam = aparams[i];
+                final Class<?> bparam = bparams[i];
+                if (aparam == bparam) {
+                    continue;
+                }
+                cmp = aparam.getName().compareTo(bparam.getName());
+                if (cmp != 0) {
+                    return cmp;
+                }
+            }
+            final Class<?> aret = a.getReturnType();
+            final Class<?> bret = b.getReturnType();
+            if (aret == bret) {
+                return 0;
+            }
+
+            // Super type comes last: Integer, Number, Object
+            if (aret.isAssignableFrom(bret)) {
+                return 1;
+            }
+            return -1;
+        }
+
+        static final MethodOrder instance = new MethodOrder();
     }
 }

jdk/test/java/beans/Introspector/TestMethodOrderDependence.java

@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.beans.BeanInfo;
+import java.beans.Introspector;
+import java.beans.PropertyDescriptor;
+import java.io.Serializable;
+import java.util.AbstractList;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Vector;
+
+/**
+ * @test
+ * @bug 8156043
+ */
+public final class TestMethodOrderDependence {
+
+    public static class Base {
+
+        public Object getI() {
+            return null;
+        }
+
+        public Object getE() {
+            return null;
+        }
+    }
+
+    public static class Super extends Base {
+
+        public Number getI() {
+            return null;
+        }
+        public Comparable<?> getE() {
+            return null;
+        }
+    }
+
+    public static class Sub extends Super {
+
+        public Integer getI() {
+            return null;
+        }
+
+        public void setFoo(Character foo) {
+        }
+
+        public void setFoo(String foo) {
+        }
+
+        public void setFoo(Object[] foo) {
+        }
+
+        public void setFoo(Enum foo) {
+        }
+
+        public void setFoo(Long foo) {
+        }
+
+        public void setFoo(Long[] foo) {
+        }
+
+        public void setFoo(Long foo, int i) {
+        }
+
+        public void setFoo(Object foo) {
+        }
+
+        public void setFoo(AbstractList foo) {
+        }
+
+        public void setFoo(ArrayList foo) {
+        }
+
+        public void setFoo(Integer foo) {
+        }
+
+        public void setFoo(Number foo) {
+        }
+
+        public void setFoo(Comparable<?> foo) {
+        }
+
+        public void setFoo(Serializable foo) {
+        }
+
+        public void setFoo(Vector<?> foo) {
+        }
+
+        public void setFoo(long foo) {
+        }
+
+        public void setFoo(int foo) {
+        }
+
+        public Enum getE() {
+            return null;
+        }
+
+        public void setE(Enum e) {
+        }
+
+        public void setE(Float e) {
+        }
+
+        public void setE(Long e) {
+        }
+    }
+
+    public static void main(final String[] args) throws Exception {
+        final BeanInfo beanInfo = Introspector.getBeanInfo(Sub.class);
+        final PropertyDescriptor[] pds = beanInfo.getPropertyDescriptors();
+        for (final PropertyDescriptor pd : pds) {
+            System.err.println("pd = " + pd);
+            final Class<?> type = pd.getPropertyType();
+            if (type != Class.class && type != Long[].class
+                    && type != Integer.class && type != Enum.class) {
+                throw new RuntimeException(Arrays.toString(pds));
+            }
+        }
+    }
+}