spring - Why did PropertyDescriptor behavior change from Java 1.6 to 1.7?

Question

Update: Oracle has confirmed this as a bug.

Summary: Certain custom BeanInfos and PropertyDescriptors that work in JDK 1.6 fail in JDK 1.7, and some only fail after Garbage Collection has run and cleared certain SoftReferences.

Edit: This will also break the ExtendedBeanInfo in Spring 3.1 as noted at the bottom of the post.

Edit: If you invoke sections 7.1 or 8.3 of the JavaBeans spec, explain exactly where those parts of the spec require anything. The language is not imperative or normative in those sections. The language in those sections is that of examples, which are at best ambiguous as a specification. Furthermore, the BeanInfo API specifically allows one to change the default behavior, and it is clearly broken in the second example below.

The Java Beans specification looks for default setter methods with a void return type, but it allows customization of the getter and setter methods through a java.beans.PropertyDescriptor. The simplest way to use it has been to specify the names of the getter and setter.

new PropertyDescriptor("foo", MyClass.class, "getFoo", "setFoo");

This has worked in JDK 1.5 and JDK 1.6 to specify the setter name even when its return type is not void as in the test case below:

import java.beans.IntrospectionException;
import java.beans.PropertyDescriptor;
import org.testng.annotations.*;

/**
 * Shows what has worked up until JDK 1.7.
 */
public class PropertyDescriptorTest
{
    private int i;
    public int getI() { return i; }
    // A setter that my people call "fluent".
    public PropertyDescriptorTest setI(final int i) { this.i = i; return this; }

    @Test
    public void fluentBeans() throws IntrospectionException
    {
        // This throws an exception only in JDK 1.7.
        final PropertyDescriptor pd = new PropertyDescriptor("i",
                           PropertyDescriptorTest.class, "getI", "setI");

        assert pd.getReadMethod() != null;
        assert pd.getWriteMethod() != null;
    }
}

The example of custom BeanInfos, which allow the programmatic control of PropertyDescriptors in the Java Beans specification all use void return types for their setters, but nothing in the specification indicates that those examples are normative, and now the behavior of this low-level utility has changed in the new Java classes, which happens to have broken some code on which I am working.

There are numerous changes in in the java.beans package between JDK 1.6 and 1.7, but the one that causes this test to fail appears to be in this diff:

@@ -240,11 +289,16 @@
        }

        if (writeMethodName == null) {
-       writeMethodName = "set" + getBaseName();
+                writeMethodName = Introspector.SET_PREFIX + getBaseName();
        }

-       writeMethod = Introspector.findMethod(cls, writeMethodName, 1, 
-                 (type == null) ? null : new Class[] { type });
+            Class[] args = (type == null) ? null : new Class[] { type };
+            writeMethod = Introspector.findMethod(cls, writeMethodName, 1, args);
+            if (writeMethod != null) {
+                if (!writeMethod.getReturnType().equals(void.class)) {
+                    writeMethod = null;
+                }
+            }
        try {
        setWriteMethod(writeMethod);
        } catch (IntrospectionException ex) {

Instead of simply accepting the method with the correct name and parameters, the PropertyDescriptor is now also checking the return type to see whether it is null, so the fluent setter no longer gets used. The PropertyDescriptor throws an IntrospectionException in this case: "Method not found: setI".

However, the problem is much more insidious than the simple test above. Another way to specify the getter and setter methods in the PropertyDescriptor for a custom BeanInfo is to use the actual Method objects:

@Test
public void fluentBeansByMethod()
    throws IntrospectionException, NoSuchMethodException
{
    final Method readMethod = PropertyDescriptorTest.class.getMethod("getI");
    final Method writeMethod = PropertyDescriptorTest.class.getMethod("setI",
                                                                 Integer.TYPE);

    final PropertyDescriptor pd = new PropertyDescriptor("i", readMethod,
                                                         writeMethod);

    assert pd.getReadMethod() != null;
    assert pd.getWriteMethod() != null;
}

Now the above code will pass a unit test in both 1.6 and in 1.7, but the code will begin to fail at some point in time during the life of the JVM instance owing to the very same change that causes the first example to fail immediately. In the second example the only indication that anything has gone wrong comes when trying to use the custom PropertyDescriptor. The setter is null, and most utility code takes that to mean that the property is read-only.

The code in the diff is inside PropertyDescriptor.getWriteMethod(). It executes when the SoftReference holding the actual setter Method is empty. This code is invoked by the PropertyDescriptor constructor in the first example that takes the accessor method names above because initially there is no Method saved in the SoftReferences holding the actual getter and setter.

In the second example the read method and write method are stored in SoftReference objects in the PropertyDescriptor by the constructor, and at first these will contain references to the readMethod and writeMethod getter and setter Methods given to the constructor. If at some point those Soft references are cleared as the Garbage Collector is allowed to do (and it will do), then the getWriteMethod() code will see that the SoftReference gives back null, and it will try to discover the setter. This time, using the same code path inside PropertyDescriptor that causes the first example to fail in JDK 1.7, it will set the write Method to null because the return type is not void. (The return type is not part of a Java method signature.)

Having the behavior change like this over time when using a custom BeanInfo can be extremely confusing. Trying to duplicate the conditions that cause the Garbage Collector to clear those particular SoftReferences is also tedious (though maybe some instrumenting mocking may help.)

The Spring ExtendedBeanInfo class has tests similar to those above. Here is an actual Spring 3.1.1 unit test from ExtendedBeanInfoTest that will pass in unit test mode, but the code being tested will fail in the post-GC insidious mode::

@Test
public void nonStandardWriteMethodOnly() throws IntrospectionException {
    @SuppressWarnings("unused") class C {
        public C setFoo(String foo) { return this; }
    }

    BeanInfo bi = Introspector.getBeanInfo(C.class);
    ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);

    assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
    assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));

    assertThat(hasReadMethodForProperty(ebi, "foo"), is(false));
    assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
}

One suggestion is that we can keep the current code working with the non-void setters by preventing the setter methods from being only softly reachable. That seems like it would work, but that is rather a hack around the changed behavior in JDK 1.7.

Q: Is there some definitive specification stating that non-void setters should be anathema? I've found nothing, and I currently consider this a bug in the JDK 1.7 libraries. Am I wrong, and why?

Answer 1

Looks like the specification hasn't changed (it requires void setter) but the implementation has been updated to only allow void setters.

Specification:

http://www.oracle.com/technetwork/java/javase/documentation/spec-136004.html

More specifically see section 7.1 (accessor methods) and 8.3 (design patterns for simple properties)

See some of the later answers in this stackoverflow question:

Does Java bean's setter permit return this?