Return-Path: Delivered-To: apmail-harmony-commits-archive@www.apache.org Received: (qmail 64367 invoked from network); 6 Jul 2007 07:24:14 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 6 Jul 2007 07:24:14 -0000 Received: (qmail 96225 invoked by uid 500); 6 Jul 2007 07:17:33 -0000 Delivered-To: apmail-harmony-commits-archive@harmony.apache.org Received: (qmail 96198 invoked by uid 500); 6 Jul 2007 07:17:33 -0000 Mailing-List: contact commits-help@harmony.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@harmony.apache.org Delivered-To: mailing list commits@harmony.apache.org Received: (qmail 96184 invoked by uid 99); 6 Jul 2007 07:17:33 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 06 Jul 2007 00:17:33 -0700 X-ASF-Spam-Status: No, hits=-99.5 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 06 Jul 2007 00:12:03 -0700 Received: by eris.apache.org (Postfix, from userid 65534) id E07EF1A981C; Fri, 6 Jul 2007 00:11:42 -0700 (PDT) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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 Date: Fri, 06 Jul 2007 07:11:42 -0000 To: commits@harmony.apache.org From: pyang@apache.org X-Mailer: svnmailer-1.1.0 Message-Id: <20070706071142.E07EF1A981C@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 allVetoableChangeListeners = new ArrayList(); - - private transient Map> selectedVetoableChangeListeners = new HashMap>(); - - // fields for serialization compatibility - private Hashtable children; + private Hashtable children = new Hashtable(); - private transient Object sourceBean; + private transient ArrayList globalListeners = new ArrayList(); + + 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 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 listeners = selectedVetoableChangeListeners + VetoableChangeSupport listeners = children .get(propertyName); if (listeners == null) { - listeners = new ArrayList(); - 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 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 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 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 result = new ArrayList( - allVetoableChangeListeners); - - Iterator keysIterator = selectedVetoableChangeListeners - .keySet().iterator(); - while (keysIterator.hasNext()) { - String propertyName = keysIterator.next(); + List result = new ArrayList(); + if (globalListeners != null) { + result.addAll(globalListeners); + } - List 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(); - for (Entry> entry : selectedVetoableChangeListeners - .entrySet()) { - List 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>(); - if (children != null) { - for (Entry entry : children - .entrySet()) { - List list = new ArrayList(); - for (VetoableChangeListener vetoableChangeListener : entry - .getValue().allVetoableChangeListeners) { - list.add(vetoableChangeListener); - } - selectedVetoableChangeListeners.put(entry.getKey(), list); - } - - } - allVetoableChangeListeners = new ArrayList(); - - VetoableChangeListener vetoableChangeListener; - while (null != (vetoableChangeListener = (VetoableChangeListener) ois - .readObject())) { - allVetoableChangeListeners.add(vetoableChangeListener); - } + this.globalListeners = new ArrayList(); + if (null == this.children) { + this.children = new Hashtable(); + } + 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 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 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")); } }