7172865: PropertyDescriptor fails to work with setter method name if setter is non-void

Reviewed-by: art, alexsch

jdk/src/share/classes/java/beans/IndexedPropertyDescriptor.java

@@ -41,8 +41,8 @@ import java.lang.reflect.Method;
 public class IndexedPropertyDescriptor extends PropertyDescriptor {
 
     private Reference<? extends Class<?>> indexedPropertyTypeRef;
-    private Reference<Method> indexedReadMethodRef;
-    private Reference<Method> indexedWriteMethodRef;
+    private final MethodRef indexedReadMethodRef = new MethodRef();
+    private final MethodRef indexedWriteMethodRef = new MethodRef();
 
     private String indexedReadMethodName;
     private String indexedWriteMethodName;
@@ -173,11 +173,11 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
      * May return null if the property isn't indexed or is write-only.
      */
     public synchronized Method getIndexedReadMethod() {
-        Method indexedReadMethod = getIndexedReadMethod0();
+        Method indexedReadMethod = this.indexedReadMethodRef.get();
         if (indexedReadMethod == null) {
             Class<?> cls = getClass0();
             if (cls == null ||
-                (indexedReadMethodName == null && indexedReadMethodRef == null)) {
+                (indexedReadMethodName == null && !this.indexedReadMethodRef.isSet())) {
                 // the Indexed readMethod was explicitly set to null.
                 return null;
             }
@@ -215,20 +215,19 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
 
         // the indexed property type is set by the reader.
         setIndexedPropertyType(findIndexedPropertyType(readMethod,
-                                                       getIndexedWriteMethod0()));
+                                                       this.indexedWriteMethodRef.get()));
         setIndexedReadMethod0(readMethod);
     }
 
     private void setIndexedReadMethod0(Method readMethod) {
+        this.indexedReadMethodRef.set(readMethod);
         if (readMethod == null) {
             indexedReadMethodName = null;
-            indexedReadMethodRef = null;
             return;
         }
         setClass0(readMethod.getDeclaringClass());
 
         indexedReadMethodName = readMethod.getName();
-        this.indexedReadMethodRef = getSoftReference(readMethod);
         setTransient(readMethod.getAnnotation(Transient.class));
     }
 
@@ -241,11 +240,11 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
      * May return null if the property isn't indexed or is read-only.
      */
     public synchronized Method getIndexedWriteMethod() {
-        Method indexedWriteMethod = getIndexedWriteMethod0();
+        Method indexedWriteMethod = this.indexedWriteMethodRef.get();
         if (indexedWriteMethod == null) {
             Class<?> cls = getClass0();
             if (cls == null ||
-                (indexedWriteMethodName == null && indexedWriteMethodRef == null)) {
+                (indexedWriteMethodName == null && !this.indexedWriteMethodRef.isSet())) {
                 // the Indexed writeMethod was explicitly set to null.
                 return null;
             }
@@ -301,15 +300,14 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
     }
 
     private void setIndexedWriteMethod0(Method writeMethod) {
+        this.indexedWriteMethodRef.set(writeMethod);
         if (writeMethod == null) {
             indexedWriteMethodName = null;
-            indexedWriteMethodRef = null;
             return;
         }
         setClass0(writeMethod.getDeclaringClass());
 
         indexedWriteMethodName = writeMethod.getName();
-        this.indexedWriteMethodRef = getSoftReference(writeMethod);
         setTransient(writeMethod.getAnnotation(Transient.class));
     }
 
@@ -349,18 +347,6 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
                 : null;
     }
 
-    private Method getIndexedReadMethod0() {
-        return (this.indexedReadMethodRef != null)
-                ? this.indexedReadMethodRef.get()
-                : null;
-    }
-
-    private Method getIndexedWriteMethod0() {
-        return (this.indexedWriteMethodRef != null)
-                ? this.indexedWriteMethodRef.get()
-                : null;
-    }
-
     private Class<?> findIndexedPropertyType(Method indexedReadMethod,
                                           Method indexedWriteMethod)
         throws IntrospectionException {
@@ -492,8 +478,8 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
      */
     IndexedPropertyDescriptor(IndexedPropertyDescriptor old) {
         super(old);
-        indexedReadMethodRef = old.indexedReadMethodRef;
-        indexedWriteMethodRef = old.indexedWriteMethodRef;
+        this.indexedReadMethodRef.set(old.indexedReadMethodRef.get());
+        this.indexedWriteMethodRef.set(old.indexedWriteMethodRef.get());
         indexedPropertyTypeRef = old.indexedPropertyTypeRef;
         indexedWriteMethodName = old.indexedWriteMethodName;
         indexedReadMethodName = old.indexedReadMethodName;
@@ -502,7 +488,7 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
     void updateGenericsFor(Class<?> type) {
         super.updateGenericsFor(type);
         try {
-            setIndexedPropertyType(findIndexedPropertyType(getIndexedReadMethod0(), getIndexedWriteMethod0()));
+            setIndexedPropertyType(findIndexedPropertyType(this.indexedReadMethodRef.get(), this.indexedWriteMethodRef.get()));
         }
         catch (IntrospectionException exception) {
             setIndexedPropertyType(null);
@@ -532,7 +518,7 @@ public class IndexedPropertyDescriptor extends PropertyDescriptor {
     void appendTo(StringBuilder sb) {
         super.appendTo(sb);
         appendTo(sb, "indexedPropertyType", this.indexedPropertyTypeRef);
-        appendTo(sb, "indexedReadMethod", this.indexedReadMethodRef);
-        appendTo(sb, "indexedWriteMethod", this.indexedWriteMethodRef);
+        appendTo(sb, "indexedReadMethod", this.indexedReadMethodRef.get());
+        appendTo(sb, "indexedWriteMethod", this.indexedWriteMethodRef.get());
     }
 }

jdk/src/share/classes/java/beans/MethodDescriptor.java

@@ -38,7 +38,7 @@ import java.util.ArrayList;
 
 public class MethodDescriptor extends FeatureDescriptor {
 
-    private Reference<Method> methodRef;
+    private final MethodRef methodRef = new MethodRef();
 
     private String[] paramNames;
 
@@ -81,7 +81,7 @@ public class MethodDescriptor extends FeatureDescriptor {
      * @return The low-level description of the method
      */
     public synchronized Method getMethod() {
-        Method method = getMethod0();
+        Method method = this.methodRef.get();
         if (method == null) {
             Class<?> cls = getClass0();
             String name = getName();
@@ -114,13 +114,7 @@ public class MethodDescriptor extends FeatureDescriptor {
             setClass0(method.getDeclaringClass());
         }
         setParams(getParameterTypes(getClass0(), method));
-        this.methodRef = getSoftReference(method);
-    }
-
-    private Method getMethod0() {
-        return (this.methodRef != null)
-                ? this.methodRef.get()
-                : null;
+        this.methodRef.set(method);
     }
 
     private synchronized void setParams(Class<?>[] param) {
@@ -177,12 +171,10 @@ public class MethodDescriptor extends FeatureDescriptor {
      */
 
     MethodDescriptor(MethodDescriptor x, MethodDescriptor y) {
-        super(x,y);
+        super(x, y);
 
-        methodRef = x.methodRef;
-        if (y.methodRef != null) {
-            methodRef = y.methodRef;
-        }
+        Method method = y.methodRef.get();
+        this.methodRef.set(null != method ? method : x.methodRef.get());
         params = x.params;
         if (y.params != null) {
             params = y.params;
@@ -205,7 +197,7 @@ public class MethodDescriptor extends FeatureDescriptor {
     MethodDescriptor(MethodDescriptor old) {
         super(old);
 
-        methodRef = old.methodRef;
+        this.methodRef.set(old.getMethod());
         params = old.params;
         paramNames = old.paramNames;
 
@@ -219,7 +211,7 @@ public class MethodDescriptor extends FeatureDescriptor {
     }
 
     void appendTo(StringBuilder sb) {
-        appendTo(sb, "method", this.methodRef);
+        appendTo(sb, "method", this.methodRef.get());
         if (this.parameterDescriptors != null) {
             sb.append("; parameterDescriptors={");
             for (ParameterDescriptor pd : this.parameterDescriptors) {

jdk/src/share/classes/java/beans/MethodRef.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2013, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package java.beans;
+
+import java.lang.ref.SoftReference;
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Method;
+
+import static sun.reflect.misc.ReflectUtil.isPackageAccessible;
+
+final class MethodRef {
+    private String signature;
+    private SoftReference<Method> methodRef;
+    private WeakReference<Class<?>> typeRef;
+
+    void set(Method method) {
+        if (method == null) {
+            this.signature = null;
+            this.methodRef = null;
+            this.typeRef = null;
+        }
+        else {
+            this.signature = method.toGenericString();
+            this.methodRef = new SoftReference<>(method);
+            this.typeRef = new WeakReference<Class<?>>(method.getDeclaringClass());
+        }
+    }
+
+    boolean isSet() {
+        return this.methodRef != null;
+    }
+
+    Method get() {
+        if (this.methodRef == null) {
+            return null;
+        }
+        Method method = this.methodRef.get();
+        if (method == null) {
+            method = find(this.typeRef.get(), this.signature);
+            if (method == null) {
+                this.signature = null;
+                this.methodRef = null;
+                this.typeRef = null;
+            }
+            else {
+                this.methodRef = new SoftReference<>(method);
+            }
+        }
+        return isPackageAccessible(method.getDeclaringClass()) ? method : null;
+    }
+
+    private static Method find(Class<?> type, String signature) {
+        if (type != null) {
+            for (Method method : type.getMethods()) {
+                if (type.equals(method.getDeclaringClass())) {
+                    if (method.toGenericString().equals(signature)) {
+                        return method;
+                    }
+                }
+            }
+        }
+        return null;
+    }
+}

jdk/src/share/classes/java/beans/PropertyDescriptor.java

@@ -36,8 +36,8 @@ import java.lang.reflect.Constructor;
 public class PropertyDescriptor extends FeatureDescriptor {
 
     private Reference<? extends Class<?>> propertyTypeRef;
-    private Reference<Method> readMethodRef;
-    private Reference<Method> writeMethodRef;
+    private final MethodRef readMethodRef = new MethodRef();
+    private final MethodRef writeMethodRef = new MethodRef();
     private Reference<? extends Class<?>> propertyEditorClassRef;
 
     private boolean bound;
@@ -68,8 +68,8 @@ public class PropertyDescriptor extends FeatureDescriptor {
     public PropertyDescriptor(String propertyName, Class<?> beanClass)
                 throws IntrospectionException {
         this(propertyName, beanClass,
-             Introspector.IS_PREFIX + NameGenerator.capitalize(propertyName),
-             Introspector.SET_PREFIX + NameGenerator.capitalize(propertyName));
+                Introspector.IS_PREFIX + NameGenerator.capitalize(propertyName),
+                Introspector.SET_PREFIX + NameGenerator.capitalize(propertyName));
     }
 
     /**
@@ -203,10 +203,10 @@ public class PropertyDescriptor extends FeatureDescriptor {
      * May return null if the property can't be read.
      */
     public synchronized Method getReadMethod() {
-        Method readMethod = getReadMethod0();
+        Method readMethod = this.readMethodRef.get();
         if (readMethod == null) {
             Class<?> cls = getClass0();
-            if (cls == null || (readMethodName == null && readMethodRef == null)) {
+            if (cls == null || (readMethodName == null && !this.readMethodRef.isSet())) {
                 // The read method was explicitly set to null.
                 return null;
             }
@@ -247,17 +247,16 @@ public class PropertyDescriptor extends FeatureDescriptor {
      */
     public synchronized void setReadMethod(Method readMethod)
                                 throws IntrospectionException {
+        this.readMethodRef.set(readMethod);
         if (readMethod == null) {
             readMethodName = null;
-            readMethodRef = null;
             return;
         }
         // The property type is determined by the read method.
-        setPropertyType(findPropertyType(readMethod, getWriteMethod0()));
+        setPropertyType(findPropertyType(readMethod, this.writeMethodRef.get()));
         setClass0(readMethod.getDeclaringClass());
 
         readMethodName = readMethod.getName();
-        this.readMethodRef = getSoftReference(readMethod);
         setTransient(readMethod.getAnnotation(Transient.class));
     }
 
@@ -268,10 +267,10 @@ public class PropertyDescriptor extends FeatureDescriptor {
      * May return null if the property can't be written.
      */
     public synchronized Method getWriteMethod() {
-        Method writeMethod = getWriteMethod0();
+        Method writeMethod = this.writeMethodRef.get();
         if (writeMethod == null) {
             Class<?> cls = getClass0();
-            if (cls == null || (writeMethodName == null && writeMethodRef == null)) {
+            if (cls == null || (writeMethodName == null && !this.writeMethodRef.isSet())) {
                 // The write method was explicitly set to null.
                 return null;
             }
@@ -318,9 +317,9 @@ public class PropertyDescriptor extends FeatureDescriptor {
      */
     public synchronized void setWriteMethod(Method writeMethod)
                                 throws IntrospectionException {
+        this.writeMethodRef.set(writeMethod);
         if (writeMethod == null) {
             writeMethodName = null;
-            writeMethodRef = null;
             return;
         }
         // Set the property type - which validates the method
@@ -328,22 +327,9 @@ public class PropertyDescriptor extends FeatureDescriptor {
         setClass0(writeMethod.getDeclaringClass());
 
         writeMethodName = writeMethod.getName();
-        this.writeMethodRef = getSoftReference(writeMethod);
         setTransient(writeMethod.getAnnotation(Transient.class));
     }
 
-    private Method getReadMethod0() {
-        return (this.readMethodRef != null)
-                ? this.readMethodRef.get()
-                : null;
-    }
-
-    private Method getWriteMethod0() {
-        return (this.writeMethodRef != null)
-                ? this.writeMethodRef.get()
-                : null;
-    }
-
     /**
      * Overridden to ensure that a super class doesn't take precedent
      */
@@ -617,8 +603,8 @@ public class PropertyDescriptor extends FeatureDescriptor {
     PropertyDescriptor(PropertyDescriptor old) {
         super(old);
         propertyTypeRef = old.propertyTypeRef;
-        readMethodRef = old.readMethodRef;
-        writeMethodRef = old.writeMethodRef;
+        this.readMethodRef.set(old.readMethodRef.get());
+        this.writeMethodRef.set(old.writeMethodRef.get());
         propertyEditorClassRef = old.propertyEditorClassRef;
 
         writeMethodName = old.writeMethodName;
@@ -632,7 +618,7 @@ public class PropertyDescriptor extends FeatureDescriptor {
     void updateGenericsFor(Class<?> type) {
         setClass0(type);
         try {
-            setPropertyType(findPropertyType(getReadMethod0(), getWriteMethod0()));
+            setPropertyType(findPropertyType(this.readMethodRef.get(), this.writeMethodRef.get()));
         }
         catch (IntrospectionException exception) {
             setPropertyType(null);
@@ -723,8 +709,8 @@ public class PropertyDescriptor extends FeatureDescriptor {
         appendTo(sb, "constrained", this.constrained);
         appendTo(sb, "propertyEditorClass", this.propertyEditorClassRef);
         appendTo(sb, "propertyType", this.propertyTypeRef);
-        appendTo(sb, "readMethod", this.readMethodRef);
-        appendTo(sb, "writeMethod", this.writeMethodRef);
+        appendTo(sb, "readMethod", this.readMethodRef.get());
+        appendTo(sb, "writeMethod", this.writeMethodRef.get());
     }
 
     private boolean isAssignable(Method m1, Method m2) {

jdk/test/java/beans/Introspector/Test7172865.java

@@ -0,0 +1,162 @@
+/*
+ * Copyright (c) 2013, 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.IndexedPropertyDescriptor;
+import java.beans.MethodDescriptor;
+import java.beans.PropertyDescriptor;
+
+/*
+ * @test
+ * @bug 7172854 7172865
+ * @summary Tests that cached methods are not lost
+ * @author Sergey Malenkov
+ */
+
+public class Test7172865 {
+    public static void main(String[] args) throws Exception {
+        int errors = 0;
+
+        MethodDescriptor md = new MethodDescriptor(Test7172865.class.getMethod("getGood"));
+
+        errors += test(PropertyDescriptor.class, "good", true);
+        PropertyDescriptor pdGoodString = new PropertyDescriptor("good", Test7172865.class, "getGood", "setGood");
+        PropertyDescriptor pdGoodMethod = new PropertyDescriptor("good",
+                Test7172865.class.getMethod("getGood"),
+                Test7172865.class.getMethod("setGood", args.getClass()));
+
+        errors += test(PropertyDescriptor.class, "bad", false);
+        PropertyDescriptor pdBadString = new PropertyDescriptor("bad", Test7172865.class, "getBad", null);
+        PropertyDescriptor pdBadMethod = new PropertyDescriptor("bad",
+                Test7172865.class.getMethod("getBad"),
+                Test7172865.class.getMethod("setBad", args.getClass()));
+
+        errors += test(IndexedPropertyDescriptor.class, "good", true);
+        IndexedPropertyDescriptor ipdGoodString = new IndexedPropertyDescriptor("good", Test7172865.class, "getGood", "setGood", "getGood", "setGood");
+        IndexedPropertyDescriptor ipdGoodMethod = new IndexedPropertyDescriptor("good",
+                Test7172865.class.getMethod("getGood"),
+                Test7172865.class.getMethod("setGood", args.getClass()),
+                Test7172865.class.getMethod("getGood", Integer.TYPE),
+                Test7172865.class.getMethod("setGood", Integer.TYPE, String.class));
+
+        errors += test(IndexedPropertyDescriptor.class, "bad", false);
+        IndexedPropertyDescriptor ipdBadString = new IndexedPropertyDescriptor("bad", Test7172865.class, "getBad", null, "getBad", null);
+        IndexedPropertyDescriptor ipdBadMethod = new IndexedPropertyDescriptor("bad",
+                Test7172865.class.getMethod("getBad"),
+                Test7172865.class.getMethod("setBad", args.getClass()),
+                Test7172865.class.getMethod("getBad", Integer.TYPE),
+                Test7172865.class.getMethod("setBad", Integer.TYPE, String.class));
+
+        for (int i = 1; i <= 2; i++) {
+            System.out.println("STEP: " + i);
+            errors += test("md", null != md.getMethod());
+
+            errors += test("pdGoodString", pdGoodString, true, true);
+            errors += test("pdGoodMethod", pdGoodMethod, true, true);
+
+            errors += test("pdBadString", pdBadString, true, false);
+            errors += test("pdBadMethod", pdBadMethod, true, true);
+
+            errors += test("ipdGoodString", ipdGoodString, true, true, true, true);
+            errors += test("ipdGoodMethod", ipdGoodMethod, true, true, true, true);
+
+            errors += test("ipdBadString", ipdBadString, true, false, true, false);
+            errors += test("ipdBadMethod", ipdBadMethod, true, true, true, true);
+
+            try {
+                int[] array = new int[1024];
+                while (true) {
+                    array = new int[array.length << 1];
+                }
+            }
+            catch (OutOfMemoryError error) {
+                System.gc();
+            }
+        }
+        if (errors > 0) {
+            throw new Error("found " + errors + " errors");
+        }
+    }
+
+    private static int test(Class<?> type, String property, boolean value) {
+        String message = type.getSimpleName() + "(" + property + ") ";
+        try {
+            type.getConstructor(String.class, Class.class).newInstance(property, Test7172865.class);
+            message += "passed";
+        }
+        catch (Exception exception) {
+            message += "failed";
+            value = !value;
+        }
+        if (value) {
+            message += " as expected";
+        }
+        System.out.println(message);
+        return value ? 0 : 1;
+    }
+
+    private static int test(String message, boolean value) {
+        System.out.println(message + ": " + (value ? "passed" : "failed"));
+        return value ? 0 : 1;
+    }
+
+    private static int test(String message, PropertyDescriptor pd, boolean rm, boolean wm) {
+        return test(message + ".Read", rm == (null != pd.getReadMethod()))
+             + test(message + ".Write", wm == (null != pd.getWriteMethod()));
+    }
+
+    private static int test(String message, IndexedPropertyDescriptor ipd, boolean rm, boolean wm, boolean irm, boolean iwm) {
+        return test(message, ipd, rm, wm)
+             + test(message + ".IndexedRead", irm == (null != ipd.getIndexedReadMethod()))
+             + test(message + ".IndexedWrite", iwm == (null != ipd.getIndexedWriteMethod()));
+    }
+
+    public String[] getGood() {
+        return null;
+    }
+
+    public String getGood(int index) {
+        return null;
+    }
+
+    public void setGood(String[] good) {
+    }
+
+    public void setGood(int index, String value) {
+    }
+
+    public String[] getBad() {
+        return null;
+    }
+
+    public String getBad(int index) {
+        return null;
+    }
+
+    public Test7172865 setBad(String[] bad) {
+        return null;
+    }
+
+    public Test7172865 setBad(int index, String value) {
+        return null;
+    }
+}