[cp-patches] RFC: java.beans.EventHandler - not finished #1

[Robert Schuster]

Hi,
I have rewritten java.beans.EventHandler.invoke() to make it follow the 
spec more closely.

Yep, I am proud to announce that this is more to the spec than Sun 
itself (run mauve gnu.testlet.java.beans.EventHandler.check14). There is 
a lot of undocumented behavior here (see another mauve testsuite named 
mauve gnu.testlet.java.beans.EventHandler.check for details on this) 
which I have reproduced as well.
I am going to add more api documentation to the last two create() method 
variants which then will describe all the stuff that is officially 
undocumented as well as fix error cases which are again weird in this 
class. :(

The main reason for posting the patch is because 'they' do something 
very odd: EventHandler implements the InvocationHandler interface which 
is part of the Proxy API. The InvocationHandler declares the invoke() 
method which has "throws Throwable" in its signature.

However in EventHandler.invoke 'throws Throwable' is missing but the 
method sometimes throws checked exceptions. These are 
NoSuchMethodExceptions and all exceptions that are thrown by the 
reflectively called methods (= InvocationTargetException.getCause() ).

I think for a lame low-tech API like EventHandler its a shame that 
'they' use such trickery. What I want to know is how we should handle this.

My suggestion is to say good-bye to 100% JAPI-compatibility and let it 
show that it may throw Throwables and add an extra 'Note: Sun bad. GNU 
good. ;)' to the doc. The majority of users will not touch this method 
anyway because the doc advises them to use the static create() methods 
which do not return an EventHandler but an interface implemented by the 
Proxy API.

cu
Robert

This is what the ChangeLog would look like:
2005-03-09  Robert Schuster <address@hidden>

   * java/beans/EventHandler.java: Reworked documentation.
   (invoke): Fixed behavior to match spec.


Index: java/beans/EventHandler.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/beans/EventHandler.java,v
retrieving revision 1.2
diff -u -r1.2 EventHandler.java
--- java/beans/EventHandler.java        30 Sep 2004 14:50:51 -0000      1.2
+++ java/beans/EventHandler.java        9 Mar 2005 05:12:06 -0000
@@ -1,5 +1,5 @@
 /* java.beans.EventHandler
-   Copyright (C) 2004 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2005 Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -44,26 +44,18 @@
 import java.lang.reflect.Proxy;
 
 /**
- * class EventHandler
- *
- * EventHandler forms a bridge between dynamically created listeners and
- * arbitrary properties and methods.  The idea is that a Proxy that implements
- * a listener class calls the EventHandler when a listener method is called.
- * The Proxy calls invoke(), which dispatches the event to a method, called
- * the action, in another object, called the target.
- *
- * The event passed to the listener method is used to access a prespecified
- * property, which in turn is passed to the action method.
+ * <p>EventHandler forms a bridge between dynamically created listeners and
+ * arbitrary properties and methods.</p>
  * 
- * Normally, call EventHandler.create(), which constructs an EventHandler and
- * a Proxy for the listener interface.  When the listenerMethod gets called on
- * the proxy, it in turn calls invoke on the attached EventHandler.  The
- * invoke call extracts the bean property from the event object and passes it
- * to the action method of target object.
- *
- * TODO: Add examples of using this thing.
+ * <p>You can use this class to easily create listener implementations for
+ * some basic interactions between an event source and its target. Using
+ * the three static methods named <code>create</code> you can create
+ * these listener implementations.</p>
  * 
+ * <p>See the documentation of each method for usage examples.</p>
+ *  
  * @author Jerry Quinn (address@hidden)
+ * @author Robert Schuster (address@hidden)
  * @since 1.4
  */
 public class EventHandler implements InvocationHandler
@@ -89,14 +81,12 @@
   /**
    * Creates a new <code>EventHandler</code> instance.
    *
-   * Typical creation is done with the create method, not by newing an
-   * EventHandler.
+   * <p>Typical creation is done with the create method, not by knewing an
+   * EventHandler.</p>
    *
-   * This constructs an EventHandler that will connect the method
+   * <p>This constructs an EventHandler that will connect the method
    * listenerMethodName to target.action, extracting eventPropertyName from
-   * the first argument of listenerMethodName. and sending it to action.
-   *
-   *
+   * the first argument of listenerMethodName. and sending it to action.</p>
    *
    * @param target Object that will perform the action.
    * @param action A property or method of the target.
@@ -114,7 +104,7 @@
   }
 
   /**
-   * Return the event property name.
+   * Returns the event property name.
    */
   public String getEventPropertyName()
   {
@@ -122,7 +112,7 @@
   }
 
   /**
-   * Return the listener's method name.
+   * Returns the listener's method name.
    */
   public String getListenerMethodName()
   {
@@ -130,7 +120,7 @@
   }
 
   /**
-   * Return the target object.
+   * Returns the target object.
    */
   public Object getTarget()
   {
@@ -138,7 +128,7 @@
   }
 
   /**
-   * Return the action method name.
+   * Returns the action method name.
    */
   public String getAction()
   {
@@ -193,107 +183,189 @@
     return new Object[] {val, getter.getReturnType()};
   }
 
-
   /**
-   * Invoke the event handler.
-   *
-   * Proxy is the object that was used, method is the method that was invoked
-   * on object, and arguments is the set of arguments passed to this method.
-   * We assume that the first argument is the event to extract a property
-   * from.
-   *
-   * Assuming that method matches the listener method specified when creating
-   * this EventHandler, the desired property is extracted from this argument.
-   * The property is passed to target.setAction(), if possible.  Otherwise
-   * target.action() is called, where action is the string fed to the
-   * constructor.
-   *
-   * For now we punt on indexed properties.  Sun docs are not clear to me
-   * about this.
-   *
-   * @param proxy The proxy object that had method invoked on it.
-   * @param method The method that was invoked.
-   * @param arguments Arguments to method.
-   * @return Result of invoking target.action on the event property
+   * Invokes the <code>EventHandler</code>.
+   * 
+   * <p>This method is normally called by the listener's proxy 
implementation.</p>
+   * 
+   * @param proxy The listener interface that is implemented using
+   * the proxy mechanism.
+   * @param method The method that was called on the proxy instance.
+   * @param arguments The arguments which where given to the method.
+   * @throws Throwable <code>NoSuchMethodException</code> is thrown when the 
EventHandler's
+   * action method or property cannot be found.
    */
   public Object invoke(Object proxy, Method method, Object[] arguments)
-    throws Exception
+    throws Throwable
   {
-    // Do we actually need the proxy?
-    if (method == null)
-      throw new RuntimeException("Invoking null method");
+      // The method instance of the target object. We have to find out which
+      // one we have to invoke.
+      Method actionMethod = null;
 
     // Listener methods that weren't specified are ignored.  If listenerMethod
     // is null, then all listener methods are processed.
     if (listenerMethod != null && !method.getName().equals(listenerMethod))
       return null;
 
-    // Extract the first arg from arguments and do getProperty on arg
-    if (arguments == null || arguments.length == 0)
-      return null;
-    Object event = arguments[0]; // We hope :-)
+    // If a property is defined we definitely need a valid object at
+    // arguments[0] that can be used to retrieve a value to which the
+    // property of the target gets set.
+    if(property != null) {
+      // Extracts the argument. We will let it fail with a NullPointerException
+      // the caller used a listener method that has no arguments.
+      Object event = arguments[0];
+
+      // Obtains the property XXX propertyType keeps showing up null - why?
+      // because the object inside getProperty changes, but the ref variable
+      // can't change this way, dolt!  need a better way to get both values out
+      // - need method and object to do the invoke and get return type
+      Object v[] = getProperty(event, property);
+      Object[] args = new Object[] { v[0] };
+      
+      // Changes the class array that controls which method signature we are 
going
+      // to look up in the target object.
+      Class[] argTypes = new Class[] { (Class) v[1] };
+    
+      // Tries to  find a setter method to which we can apply the
+      while(argTypes[0] != null) {
+      try
+      {
+        // Look for a property setter for action.
+        actionMethod = target.getClass().getMethod("set" + capitalize(action), 
argTypes);
 
-    // Obtain the property XXX propertyType keeps showing up null - why?
-    // because the object inside getProperty changes, but the ref variable
-    // can't change this way, dolt!  need a better way to get both values out
-    // - need method and object to do the invoke and get return type
-    Object v[] = getProperty(event, property);
-    Object val = v[0];
-    Class propertyType = (Class) v[1];
-
-    // Find the actual method of target to invoke.  We can't do this in the
-    // constructor since we don't know the type of the property we extracted
-    // from the event then.
-    //
-    // action can be either a property or a method.  Sun's docs seem to imply
-    // that action should be treated as a property first, and then a method,
-    // but don't specifically say it.
-    //
-    // XXX check what happens with native type wrappers.  The better thing to
-    // do is look at the return type of the method
-    Method actionMethod;
+        return actionMethod.invoke(target, args);
+      }
+    catch (NoSuchMethodException e)
+      {
+        // If action as property didn't work, try as method later.
+      }
+    
+      argTypes[0] = superclass(argTypes[0]);
+      }
+      
+      // We could not find a suitable setter method. Now we try again 
interpreting
+      // action as the method name itself.
+      // Since we probably have changed the block local argTypes array 
+      // we need to rebuild it.
+      argTypes = new Class[] { (Class) v[1] };
+    
+      // Tries to  find a setter method to which we can apply the
+      while(argTypes[0] != null) {
+        try
+        {
+          actionMethod = target.getClass().getMethod(action, argTypes);
+
+          return actionMethod.invoke(target, args);
+        }
+        catch (NoSuchMethodException e)
+        {
+        }
+        
+        argTypes[0] = superclass(argTypes[0]);
+      }
+        
+      }      
+  
+    // If property was null we will search for a no-argument method here.
+    // Note: The ordering of method lookups is important because we want to 
prefer no-argument
+    // calls like the JDK does. This means if we have actionMethod() and 
actionMethod(Event) we will
+    // call the first *EVEN* if we have a valid argument for the second 
method. This is behavior compliant
+    // to the JDK.
+    // If actionMethod() is not available but there is a actionMethod(Event) 
we take this. That makes us
+    // more specification compliant than the JDK itself because this one will 
fail in such a case.
     try
       {
-       // Look for a property setter for action.
-       actionMethod = 
-         target.getClass().getMethod("set" + capitalize(action),
-                                     new Class[] {propertyType});
+      actionMethod = target.getClass().getMethod(action, null);
       }
-    catch (NoSuchMethodException e)
+    catch(NoSuchMethodException nsme)
       {
-       // If action as property didn't work, try as method.
-       try
-         {
-           actionMethod = 
-             target.getClass().getMethod(action, new Class[] {propertyType});
-         }
-       catch (NoSuchMethodException e1)
-         {
-           // When event property is null, we may call action with no args
-           if (property == null)
-             {
-               actionMethod =
-                 target.getClass().getMethod(action, null);
-               return actionMethod.invoke(target, null);
-             }
-           else
-             throw e1;
-         }
+        // Note: If we want to be really strict the specification says that a 
no-argument method should
+        // accept an EventObject (or subclass I guess). However since the 
official implementation is broken
+        // anyways, it's more flexible without the EventObject restriction and 
we are compatible on everything
+        // else this can stay this way.
+        if(arguments != null && arguments.length >= 1/* && arguments[0] 
instanceof EventObject*/) {
+            Class[] targetArgTypes = new Class[] { arguments[0].getClass() };
+            
+            while(targetArgTypes[0] != null) {
+                try
+                {
+                  // If no property exists we expect the first element of the 
arguments to be
+                  // an EventObject which is then applied to the target method.
+      
+                  actionMethod = target.getClass().getMethod(action, 
targetArgTypes);
+              
+                  return actionMethod.invoke(target, new Object[] { 
arguments[0] });
+                }
+                catch(NoSuchMethodException nsme2)
+                {
+                    
+                }
+                
+                targetArgTypes[0] = superclass(targetArgTypes[0]);
+            }
+          
+        }
       }
 
     // Invoke target.action(property)
-    return actionMethod.invoke(target, new Object[] {val});
+    return actionMethod.invoke(target, null);
   }
 
   /**
-   * Construct a new object to dispatch events.
-   *
-   * Equivalent to:
-   * create(listenerInterface, target, action, null, null)
+   * <p>A small helper method that makes it easier to overcome the behavior of 
+   * the primitive types.</p>
+   * 
+   * <p>For invoke() we need to walk through the superclasses if we cannot find
+   * a method of a certain type. When it comes to the primitives the behavior
+   * should be the same as if we had their wrapper class: Boolean.TYPE's 
+   * superclass should be derived from Object and the others should be a
+   * subclass of Number.</p>
+   * 
+   * @param klass
+   * @return
+   */
+  private Class superclass(Class klass) {
+   if(klass.isPrimitive()) {
+       if(klass == Boolean.TYPE)
+           return Object.class;
+       
+       return Number.class;
+   }
+   
+   return klass.getSuperclass();
+  }
+  
+  /**
+   * <p>Constructs a new implementation of <code>listenerInterface</code>
+   * to dispatch events.</p>
+   * 
+   * <p>Call this method like:</p>
+   * <code>
+   * button.addActionListener((ActionListener)
+   *    EventHandler.create(ActionListener.class, target, "dispose"));
+   * </code>
+   * 
+   * <p>to achieve the following behavior:</p>
+   * <code>
+   * button.addActionListener(new ActionListener() {
+   *    public void actionPerformed(ActionEvent ae) {
+   *        target.dispose();
+   *    }
+   * });
+   * </code>
+   * 
+   * <p>That means if you need a listener implementation that simply calls a
+   * a no-argument method on a given instance for <strong>each</strong>
+   * method of the listener interface.</p>
+   * 
+   * <p>Note: The action is interpreted as a method name. If your target object
+   * has no no-argument method of the given name the EventHandler tries to find
+   * a method with the same name but which can accept the first argument of the
+   * listener method. Usually this will be an event object but any other object
+   * will be forwarded, too.<p/>
    *
-   * I.e. all listenerInterface methods are mapped to
-   * target.action(EventObject) or target.action(), if the first doesn't
-   * exist.
+   * <p>A call to this method is equivalent to:
+   * <code>create(listenerInterface, target, action, null, null)</code></p>
    *
    * @param listenerInterface Listener interface to implement.
    * @param target Object to invoke action on.
@@ -306,7 +378,7 @@
   }
 
   /**
-   * Construct a new object to dispatch events.
+   * Constructs a new object to dispatch events.
    *
    * Equivalent to:
    * create(listenerInterface, target, action, eventPropertyName, null)
@@ -329,7 +401,7 @@
 
 
   /**
-   * Construct a new object to dispatch events.
+   * Constructs a new object to dispatch events.
    *
    * This creates an object that acts as a proxy for the method
    * listenerMethodName in listenerInterface.  When the listener method is