You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by py...@apache.org on 2007/07/06 09:11:42 UTC

svn commit: r553768 - in /harmony/enhanced/classlib/trunk/modules/beans/src: main/java/java/beans/VetoableChangeSupport.java test/java/org/apache/harmony/beans/tests/java/beans/VetoableChangeSupportTest.java

Author: pyang
Date: Fri Jul  6 00:11:41 2007
New Revision: 553768

URL: http://svn.apache.org/viewvc?view=rev&rev=553768
Log:
1. Refine the VetoableChangeSupport against RI's behavior, the original version has 12 faiulres in RI; 2. add serialization test; 3. Fix VetoableChangeSupport to pass most of the tests, but there's still 1 failures left

Modified:
    harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java
    harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/VetoableChangeSupportTest.java

Modified: harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java?view=diff&rev=553768&r1=553767&r2=553768
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java (original)
+++ harmony/enhanced/classlib/trunk/modules/beans/src/main/java/java/beans/VetoableChangeSupport.java Fri Jul  6 00:11:41 2007
@@ -22,50 +22,39 @@
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.Hashtable;
-import java.util.Map.Entry;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
+
+//FIXME: obviously need synchronization, when access listeners
 
 public class VetoableChangeSupport implements Serializable {
 
     private static final long serialVersionUID = -5090210921595982017l;
 
-    private transient List<VetoableChangeListener> allVetoableChangeListeners = new ArrayList<VetoableChangeListener>();
-
-    private transient Map<String, List<VetoableChangeListener>> selectedVetoableChangeListeners = new HashMap<String, List<VetoableChangeListener>>();
-
-    // fields for serialization compatibility
-    private Hashtable<String, VetoableChangeSupport> children;
+    private Hashtable<String, VetoableChangeSupport> children = new Hashtable<String, VetoableChangeSupport>();
     
-    private transient Object sourceBean;
+    private transient ArrayList<VetoableChangeListener> globalListeners = new ArrayList<VetoableChangeListener>();
+    
+    private Object source;
 
-    private int vetoableChangeSupportSerializedDataVersion = 1;
+    private int vetoableChangeSupportSerializedDataVersion = 2;
 
     public VetoableChangeSupport(Object sourceBean) {
         if (sourceBean == null) {
             throw new NullPointerException();
         }
-        this.sourceBean = sourceBean;
-    }
-
-    public void fireVetoableChange(String propertyName, Object oldValue,
-            Object newValue) throws PropertyVetoException {
-        PropertyChangeEvent event = createPropertyChangeEvent(propertyName,
-                oldValue, newValue);
-        doFirePropertyChange(event);
+        this.source = sourceBean;
     }
 
     public synchronized void removeVetoableChangeListener(String propertyName,
             VetoableChangeListener listener) {
         if ((propertyName != null) && (listener != null)) {
-            List<VetoableChangeListener> listeners = selectedVetoableChangeListeners
+            VetoableChangeSupport listeners = children
                     .get(propertyName);
 
             if (listeners != null) {
-                listeners.remove(listener);
+                listeners.removeVetoableChangeListener(listener);
             }
         }
     }
@@ -73,49 +62,35 @@
     public synchronized void addVetoableChangeListener(String propertyName,
             VetoableChangeListener listener) {
         if (propertyName != null && listener != null) {
-            List<VetoableChangeListener> listeners = selectedVetoableChangeListeners
+            VetoableChangeSupport listeners = children
                     .get(propertyName);
 
             if (listeners == null) {
-                listeners = new ArrayList<VetoableChangeListener>();
-                selectedVetoableChangeListeners.put(propertyName, listeners);
+                listeners = new VetoableChangeSupport(source);
+                children.put(propertyName, listeners);
             }
-            listeners.add(listener);
+            listeners.addVetoableChangeListener(listener);
         }
     }
 
     public synchronized VetoableChangeListener[] getVetoableChangeListeners(
             String propertyName) {
-        List<VetoableChangeListener> listeners = null;
+        VetoableChangeSupport listeners = null;
 
         if (propertyName != null) {
-            listeners = selectedVetoableChangeListeners.get(propertyName);
+            listeners = children.get(propertyName);
         }
         return (listeners == null) ? new VetoableChangeListener[] {}
                 : getAsVetoableChangeListenerArray(listeners);
     }
 
-    public void fireVetoableChange(String propertyName, boolean oldValue,
-            boolean newValue) throws PropertyVetoException {
-        PropertyChangeEvent event = createPropertyChangeEvent(propertyName,
-                oldValue, newValue);
-        doFirePropertyChange(event);
-    }
-
-    public void fireVetoableChange(String propertyName, int oldValue,
-            int newValue) throws PropertyVetoException {
-        PropertyChangeEvent event = createPropertyChangeEvent(propertyName,
-                oldValue, newValue);
-        doFirePropertyChange(event);
-    }
-
     public synchronized boolean hasListeners(String propertyName) {
-        boolean result = allVetoableChangeListeners.size() > 0;
+        boolean result = globalListeners.size() > 0;
         if (!result && propertyName != null) {
-            List<VetoableChangeListener> listeners = selectedVetoableChangeListeners
+            VetoableChangeSupport listeners = children
                     .get(propertyName);
             if (listeners != null) {
-                result = listeners.size() > 0;
+                result = listeners.globalListeners.size() > 0;
             }
         }
         return result;
@@ -124,97 +99,96 @@
     public synchronized void removeVetoableChangeListener(
             VetoableChangeListener listener) {
         if (listener != null) {
-            Iterator<VetoableChangeListener> iterator = allVetoableChangeListeners
-                    .iterator();
-            while (iterator.hasNext()) {
-                VetoableChangeListener pcl = iterator.next();
-                if (pcl == listener) {
-                    iterator.remove();
-                    break;
-                }
-            }
+            globalListeners.remove(listener);
         }
     }
 
     public synchronized void addVetoableChangeListener(
             VetoableChangeListener listener) {
-        if (listener != null) {
-            allVetoableChangeListeners.add(listener);
+        if(listener != null){
+            if (listener instanceof VetoableChangeListenerProxy) {
+                VetoableChangeListenerProxy proxy = (VetoableChangeListenerProxy) listener;
+                addVetoableChangeListener(proxy.getPropertyName(),
+                        (VetoableChangeListener) proxy.getListener());
+            } else {
+                globalListeners.add(listener);
+            }
         }
     }
 
     public synchronized VetoableChangeListener[] getVetoableChangeListeners() {
-        List<VetoableChangeListener> result = new ArrayList<VetoableChangeListener>(
-                allVetoableChangeListeners);
-
-        Iterator<String> keysIterator = selectedVetoableChangeListeners
-                .keySet().iterator();
-        while (keysIterator.hasNext()) {
-            String propertyName = keysIterator.next();
+        List<VetoableChangeListener> result = new ArrayList<VetoableChangeListener>();
+        if (globalListeners != null) {
+            result.addAll(globalListeners);
+        }
 
-            List<VetoableChangeListener> selectedListeners = selectedVetoableChangeListeners
+        for (Iterator iterator = children.keySet().iterator(); iterator
+                .hasNext();) {
+            String propertyName = (String) iterator.next();
+            VetoableChangeSupport namedListener = (VetoableChangeSupport) children
                     .get(propertyName);
-            if (selectedListeners != null) {
-                for (VetoableChangeListener listener : selectedListeners) {
-                    result.add(new VetoableChangeListenerProxy(propertyName,
-                            listener));
-                }
+            VetoableChangeListener[] childListeners = namedListener
+                    .getVetoableChangeListeners();
+            for (int i = 0; i < childListeners.length; i++) {
+                result.add(new VetoableChangeListenerProxy(propertyName,
+                        childListeners[i]));
             }
         }
-        return getAsVetoableChangeListenerArray(result);
+        return (VetoableChangeListener[]) (result
+                .toArray(new VetoableChangeListener[result.size()]));
     }
 
     private void writeObject(ObjectOutputStream oos) throws IOException {
-        children = new Hashtable<String, VetoableChangeSupport>();
-        for (Entry<String, List<VetoableChangeListener>> entry : selectedVetoableChangeListeners
-                .entrySet()) {
-            List<VetoableChangeListener> list = entry.getValue();
-            VetoableChangeSupport vetoableChangeSupport = new VetoableChangeSupport(
-                    sourceBean);
-            for (VetoableChangeListener vetoableChangeListener : list) {
-                if (vetoableChangeListener instanceof Serializable) {
-                    vetoableChangeSupport
-                            .addVetoableChangeListener(vetoableChangeListener);
-                }
-            }
-            children.put(entry.getKey(), vetoableChangeSupport);
-        }
         oos.defaultWriteObject();
-
-        for (VetoableChangeListener vetoableChangeListener : allVetoableChangeListeners) {
-            if (vetoableChangeListener instanceof Serializable) {
-                oos.writeObject(vetoableChangeListener);
+        VetoableChangeListener[] copy = new VetoableChangeListener[globalListeners.size()];
+        globalListeners.toArray(copy);
+        for (VetoableChangeListener listener : copy) {
+            if (listener instanceof Serializable) {
+                oos.writeObject(listener);
             }
         }
+        // Denotes end of list
         oos.writeObject(null);
+
     }
 
     
     private void readObject(ObjectInputStream ois) throws IOException,
             ClassNotFoundException {
         ois.defaultReadObject();
-        selectedVetoableChangeListeners = new HashMap<String, List<VetoableChangeListener>>();
-        if (children != null) {
-            for (Entry<String, VetoableChangeSupport> entry : children
-                    .entrySet()) {
-                List<VetoableChangeListener> list = new ArrayList<VetoableChangeListener>();
-                for (VetoableChangeListener vetoableChangeListener : entry
-                        .getValue().allVetoableChangeListeners) {
-                    list.add(vetoableChangeListener);
-                }
-                selectedVetoableChangeListeners.put(entry.getKey(), list);
-            }
-
-        }
-        allVetoableChangeListeners = new ArrayList<VetoableChangeListener>();
-
-        VetoableChangeListener vetoableChangeListener;
-        while (null != (vetoableChangeListener = (VetoableChangeListener) ois
-                .readObject())) {
-            allVetoableChangeListeners.add(vetoableChangeListener);
-        }
+        this.globalListeners = new ArrayList<VetoableChangeListener>();
+        if (null == this.children) {
+            this.children = new Hashtable<String, VetoableChangeSupport>();
+        }
+        Object listener;
+        do {
+            // Reads a listener _or_ proxy
+            listener = ois.readObject();
+            addVetoableChangeListener((VetoableChangeListener) listener);
+        } while (listener != null);
+    }
+    
+    public void fireVetoableChange(String propertyName, boolean oldValue,
+            boolean newValue) throws PropertyVetoException {
+        PropertyChangeEvent event = createPropertyChangeEvent(propertyName,
+                oldValue, newValue);
+        doFirePropertyChange(event);
     }
 
+    public void fireVetoableChange(String propertyName, int oldValue,
+            int newValue) throws PropertyVetoException {
+        PropertyChangeEvent event = createPropertyChangeEvent(propertyName,
+                oldValue, newValue);
+        doFirePropertyChange(event);
+    }
+    
+    public void fireVetoableChange(String propertyName, Object oldValue,
+            Object newValue) throws PropertyVetoException {
+        PropertyChangeEvent event = createPropertyChangeEvent(propertyName,
+                oldValue, newValue);
+        doFirePropertyChange(event);
+    }
+    
     public void fireVetoableChange(PropertyChangeEvent event)
             throws PropertyVetoException {
         doFirePropertyChange(event);
@@ -222,22 +196,10 @@
 
     private PropertyChangeEvent createPropertyChangeEvent(String propertyName,
             Object oldValue, Object newValue) {
-        return new PropertyChangeEvent(sourceBean, propertyName, oldValue,
+        return new PropertyChangeEvent(source, propertyName, oldValue,
                 newValue);
     }
 
-    private PropertyChangeEvent createPropertyChangeEvent(String propertyName,
-            boolean oldValue, boolean newValue) {
-        return new PropertyChangeEvent(sourceBean, propertyName, new Boolean(
-                oldValue), new Boolean(newValue));
-    }
-
-    private PropertyChangeEvent createPropertyChangeEvent(String propertyName,
-            int oldValue, int newValue) {
-        return new PropertyChangeEvent(sourceBean, propertyName, new Integer(
-                oldValue), new Integer(newValue));
-    }
-
     private void doFirePropertyChange(PropertyChangeEvent event)
             throws PropertyVetoException {
         String propName = event.getPropertyName();
@@ -250,20 +212,15 @@
 
         /* Take note of who we are going to notify (and potentially un-notify) */
 
-        VetoableChangeListener[] listensToAll; // Listeners to all property
-        // change events
-        VetoableChangeListener[] listensToOne = null; // Listens to a given
+        VetoableChangeListener[] listensToAll;
+        VetoableChangeSupport listeners = null;
         // property change
         synchronized (this) {
-            listensToAll = allVetoableChangeListeners
-                    .toArray(new VetoableChangeListener[allVetoableChangeListeners
-                            .size()]);
-
-            List<VetoableChangeListener> listeners = selectedVetoableChangeListeners
-                    .get(event.getPropertyName());
-            if (listeners != null) {
-                listensToOne = listeners
-                        .toArray(new VetoableChangeListener[listeners.size()]);
+            listensToAll = globalListeners
+                    .toArray(new VetoableChangeListener[0]);
+            String propertyName = event.getPropertyName();
+            if(propertyName != null){
+                listeners = children.get(propertyName);
             }
         }
 
@@ -271,11 +228,6 @@
             for (VetoableChangeListener listener : listensToAll) {
                 listener.vetoableChange(event);
             }
-            if (listensToOne != null) {
-                for (VetoableChangeListener listener : listensToOne) {
-                    listener.vetoableChange(event);
-                }
-            }
         } catch (PropertyVetoException pve) {
             // Tell them we have changed it back
             PropertyChangeEvent revertEvent = createPropertyChangeEvent(
@@ -286,27 +238,15 @@
                 } catch (PropertyVetoException ignored) {
                 }
             }
-            if (listensToOne != null) {
-                for (VetoableChangeListener listener : listensToOne) {
-                    try {
-                        listener.vetoableChange(revertEvent);
-                    } catch (PropertyVetoException ignored) {
-                    }
-                }
-            }
             throw pve;
         }
+        if(listeners != null){
+            listeners.fireVetoableChange(event);
+        }
     }
 
     private static VetoableChangeListener[] getAsVetoableChangeListenerArray(
-            List<VetoableChangeListener> listeners) {
-        Object[] objects = listeners.toArray();
-        VetoableChangeListener[] arrayResult = new VetoableChangeListener[objects.length];
-
-        for (int i = 0; i < objects.length; ++i) {
-            arrayResult[i] = (VetoableChangeListener) objects[i];
-        }
-
-        return arrayResult;
+            VetoableChangeSupport listeners) {
+        return listeners.globalListeners.toArray(new VetoableChangeListener[0]);
     }
 }

Modified: harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/VetoableChangeSupportTest.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/VetoableChangeSupportTest.java?view=diff&rev=553768&r1=553767&r2=553768
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/VetoableChangeSupportTest.java (original)
+++ harmony/enhanced/classlib/trunk/modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/VetoableChangeSupportTest.java Fri Jul  6 00:11:41 2007
@@ -29,6 +29,7 @@
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
+import java.io.ObjectStreamClass;
 import java.io.Serializable;
 import java.util.ArrayList;
 
@@ -105,15 +106,11 @@
         VetoableChangeListener[] listeners2 = support
                 .getVetoableChangeListeners(propertyName);
 
-        assertTrue(support.hasListeners(propertyName));
+        assertFalse(support.hasListeners(propertyName));
         assertFalse(support.hasListeners("text"));
 
-        assertEquals(1, listeners1.length);
-        assertEquals(propertyName,
-                ((VetoableChangeListenerProxy) listeners1[0]).getPropertyName());
-        assertNull(((VetoableChangeListenerProxy) listeners1[0]).getListener());
-        assertEquals(1, listeners2.length);
-        assertNull(listeners2[0]);
+        assertEquals(0, listeners1.length);
+        assertEquals(0, listeners2.length);
     }
 
     /*
@@ -182,11 +179,7 @@
         VetoableChangeListener proxy = EventHandler.create(
                 VetoableChangeListener.class, source, "setText");
         String propertyName = null;
-        try {
             support.addVetoableChangeListener(propertyName, proxy);
-            fail("Should throw NullPointerException.");
-        } catch (NullPointerException e) {
-        }
     }
 
     /*
@@ -288,8 +281,8 @@
         VetoableChangeSupport support = new VetoableChangeSupport(source);
         support.addVetoableChangeListener(null);
 
-        assertTrue(support.hasListeners("label"));
-        assertTrue(support.hasListeners("text"));
+        assertFalse(support.hasListeners("label"));
+        assertFalse(support.hasListeners("text"));
 
         VetoableChangeListener[] listeners1 = support
                 .getVetoableChangeListeners();
@@ -298,11 +291,9 @@
         VetoableChangeListener[] listeners3 = support
                 .getVetoableChangeListeners("text");
 
-        assertEquals(1, listeners1.length);
+        assertEquals(0, listeners1.length);
         assertEquals(0, listeners2.length);
         assertEquals(0, listeners3.length);
-
-        assertNull(listeners1[0]);
     }
 
     /*
@@ -382,11 +373,18 @@
 
         VetoableChangeListenerProxy listenerProxy = new VetoableChangeListenerProxy(
                 propertyName, proxy);
-
-        support.addVetoableChangeListener(listenerProxy);
-
+        assertFalse(support.hasListeners("label"));
+        try{
+            support.addVetoableChangeListener(listenerProxy);
+            fail("should throw NPE");
+        }catch(NullPointerException e){
+            //expected
+            e.printStackTrace();
+        }
+        assertTrue(support.hasListeners("label"));
         assertTrue(support.hasListeners(propertyName));
         assertFalse(support.hasListeners("text"));
+        
         {
             VetoableChangeListener[] listeners1 = support
                     .getVetoableChangeListeners();
@@ -602,12 +600,7 @@
         support.addVetoableChangeListener(null);
         PropertyChangeEvent event = new PropertyChangeEvent(source, "label",
                 "Label: old", "Label: new");
-        try {
             support.fireVetoableChange(event);
-            fail("Should throw NullPointerException.");
-        } catch (NullPointerException e) {
-            // expected
-        }
     }
 
     /*
@@ -635,12 +628,7 @@
         VetoableChangeSupport support = new VetoableChangeSupport(source);
 
         support.addVetoableChangeListener(null);
-        try {
             support.fireVetoableChange("label", true, false);
-            fail("Should throw NullPointerException.");
-        } catch (NullPointerException e) {
-            // expected
-        }
     }
 
     /*
@@ -669,12 +657,7 @@
 
         EventHandler.create(VetoableChangeListener.class, source, "setText");
         support.addVetoableChangeListener("label", null);
-        try {
             support.fireVetoableChange("label", true, false);
-            fail("Should throw NullPointerException.");
-        } catch (NullPointerException e) {
-            // expected
-        }
     }
 
     /*
@@ -1282,14 +1265,13 @@
         String propertyName = "label";
         support.addVetoableChangeListener(propertyName, null);
 
-        assertTrue(support.hasListeners(propertyName));
-        assertEquals(1, support.getVetoableChangeListeners(propertyName).length);
+        assertFalse(support.hasListeners(propertyName));
+        assertEquals(0, support.getVetoableChangeListeners(propertyName).length);
 
         support.removeVetoableChangeListener(propertyName, proxy);
-        assertTrue(support.hasListeners(propertyName));
-        assertEquals(1, support.getVetoableChangeListeners(propertyName).length);
-        assertEquals(1, support.getVetoableChangeListeners().length);
-        assertNull(support.getVetoableChangeListeners(propertyName)[0]);
+        assertFalse(support.hasListeners(propertyName));
+        assertEquals(0, support.getVetoableChangeListeners(propertyName).length);
+        assertEquals(0, support.getVetoableChangeListeners().length);
     }
 
     /*
@@ -1325,12 +1307,7 @@
         String propertyName = "label";
         support.addVetoableChangeListener(propertyName, proxy);
         assertTrue(support.hasListeners(propertyName));
-        try {
             support.removeVetoableChangeListener(null, proxy);
-            fail("Should throw NullPointerException.");
-        } catch (NullPointerException e) {
-            // expected
-        }
     }
 
     /*
@@ -1381,7 +1358,7 @@
 
         String propertyName = "label";
         support.addVetoableChangeListener(propertyName, null);
-        assertTrue(support.hasListeners(propertyName));
+        assertFalse(support.hasListeners(propertyName));
 
         support.removeVetoableChangeListener(propertyName, null);
         assertFalse(support.hasListeners(propertyName));
@@ -1476,14 +1453,14 @@
         String propertyName = "label";
         support.addVetoableChangeListener(null);
 
-        assertTrue(support.hasListeners(propertyName));
-        assertEquals(1, support.getVetoableChangeListeners().length);
+        assertFalse(support.hasListeners(propertyName));
+        assertEquals(0, support.getVetoableChangeListeners().length);
 
         support.removeVetoableChangeListener(proxy);
 
-        assertTrue(support.hasListeners(propertyName));
+        assertFalse(support.hasListeners(propertyName));
         assertEquals(0, support.getVetoableChangeListeners(propertyName).length);
-        assertEquals(1, support.getVetoableChangeListeners().length);
+        assertEquals(0, support.getVetoableChangeListeners().length);
     }
 
     /*
@@ -1544,8 +1521,8 @@
         String propertyName = "label";
         support.addVetoableChangeListener(null);
 
-        assertTrue(support.hasListeners(propertyName));
-        assertEquals(1, support.getVetoableChangeListeners().length);
+        assertFalse(support.hasListeners(propertyName));
+        assertEquals(0, support.getVetoableChangeListeners().length);
 
         support.removeVetoableChangeListener(null);
 
@@ -1632,7 +1609,6 @@
 
     }
 
-
      public void testSerialization_Compatibility() throws Exception {
          MockSource source = new MockSource();
          VetoableChangeSupport support = new VetoableChangeSupport(source);
@@ -1936,5 +1912,11 @@
         assertEquals(vcl, ((VetoableChangeListenerProxy) vcls[0]).getListener());
         assertEquals("property1", ((VetoableChangeListenerProxy) vcls[0])
                 .getPropertyName());
+    }
+    
+    
+    public void testSerializationForm(){
+        ObjectStreamClass objectStreamClass = ObjectStreamClass.lookup(VetoableChangeSupport.class);
+        assertNotNull(objectStreamClass.getField("source"));
     }
 }