6630275: The spec on VetoableChangeSupport.fireVetoableChange should be updated

Reviewed-by: peterz, rupashka

src/share/classes/java/beans/PropertyChangeSupport.java

 /*
- * Copyright 1996-2006 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright 1996-2008 Sun Microsystems, Inc.  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
@@ -208,91 +208,91 @@ public class PropertyChangeSupport implements Serializable {
     }
 
     /**
-     * Report a bound property update to any registered listeners.
-     * No event is fired if old and new are equal and non-null.
-     *
+     * Reports a bound property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
+     * <p>
+     * No event is fired if old and new values are equal and non-null.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * firePropertyChange method that takes {@code
-     * PropertyChangeEvent} value.
+     * {@link #firePropertyChange(PropertyChangeEvent)} method.
      *
-     * @param propertyName  The programmatic name of the property
-     *          that was changed.
-     * @param oldValue  The old value of the property.
-     * @param newValue  The new value of the property.
+     * @param propertyName  the programmatic name of the property that was changed
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
      */
-    public void firePropertyChange(String propertyName,
-                                        Object oldValue, Object newValue) {
-        if (oldValue != null && newValue != null && oldValue.equals(newValue)) {
-            return;
+    public void firePropertyChange(String propertyName, Object oldValue, Object newValue) {
+        if (oldValue == null || newValue == null || !oldValue.equals(newValue)) {
+            firePropertyChange(new PropertyChangeEvent(this.source, propertyName, oldValue, newValue));
         }
-        firePropertyChange(new PropertyChangeEvent(source, propertyName,
-                                                   oldValue, newValue));
     }
 
     /**
-     * Report an int bound property update to any registered listeners.
-     * No event is fired if old and new are equal.
+     * Reports an integer bound property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
+     * <p>
+     * No event is fired if old and new values are equal.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * firePropertyChange method that takes Object values.
+     * {@link #firePropertyChange(String, Object, Object)}  method.
      *
-     * @param propertyName  The programmatic name of the property
-     *          that was changed.
-     * @param oldValue  The old value of the property.
-     * @param newValue  The new value of the property.
+     * @param propertyName  the programmatic name of the property that was changed
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
      */
-    public void firePropertyChange(String propertyName,
-                                        int oldValue, int newValue) {
-        if (oldValue == newValue) {
-            return;
+    public void firePropertyChange(String propertyName, int oldValue, int newValue) {
+        if (oldValue != newValue) {
+            firePropertyChange(propertyName, Integer.valueOf(oldValue), Integer.valueOf(newValue));
         }
-        firePropertyChange(propertyName, Integer.valueOf(oldValue), Integer.valueOf(newValue));
     }
 
     /**
-     * Report a boolean bound property update to any registered listeners.
-     * No event is fired if old and new are equal.
+     * Reports a boolean bound property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
+     * <p>
+     * No event is fired if old and new values are equal.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * firePropertyChange method that takes Object values.
+     * {@link #firePropertyChange(String, Object, Object)}  method.
      *
-     * @param propertyName  The programmatic name of the property
-     *          that was changed.
-     * @param oldValue  The old value of the property.
-     * @param newValue  The new value of the property.
+     * @param propertyName  the programmatic name of the property that was changed
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
      */
-    public void firePropertyChange(String propertyName,
-                                        boolean oldValue, boolean newValue) {
-        if (oldValue == newValue) {
-            return;
+    public void firePropertyChange(String propertyName, boolean oldValue, boolean newValue) {
+        if (oldValue != newValue) {
+            firePropertyChange(propertyName, Boolean.valueOf(oldValue), Boolean.valueOf(newValue));
         }
-        firePropertyChange(propertyName, Boolean.valueOf(oldValue), Boolean.valueOf(newValue));
     }
 
     /**
-     * Fire an existing PropertyChangeEvent to any registered listeners.
-     * No event is fired if the given event's old and new values are
-     * equal and non-null.
-     * @param evt  The PropertyChangeEvent object.
+     * Fires a property change event to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
+     * <p>
+     * No event is fired if the given event's old and new values are equal and non-null.
+     *
+     * @param event  the {@code PropertyChangeEvent} to be fired
      */
-    public void firePropertyChange(PropertyChangeEvent evt) {
-        Object oldValue = evt.getOldValue();
-        Object newValue = evt.getNewValue();
-        String propertyName = evt.getPropertyName();
-        if (oldValue != null && newValue != null && oldValue.equals(newValue)) {
-            return;
-        }
-        PropertyChangeListener[] common = this.map.get(null);
-        PropertyChangeListener[] named = (propertyName != null)
-                    ? this.map.get(propertyName)
-                    : null;
+    public void firePropertyChange(PropertyChangeEvent event) {
+        Object oldValue = event.getOldValue();
+        Object newValue = event.getNewValue();
+        if (oldValue == null || newValue == null || !oldValue.equals(newValue)) {
+            String name = event.getPropertyName();
 
-        fire(common, evt);
-        fire(named, evt);
+            PropertyChangeListener[] common = this.map.get(null);
+            PropertyChangeListener[] named = (name != null)
+                        ? this.map.get(name)
+                        : null;
+
+            fire(common, event);
+            fire(named, event);
+        }
     }
 
-    private void fire(PropertyChangeListener[] listeners, PropertyChangeEvent event) {
+    private static void fire(PropertyChangeListener[] listeners, PropertyChangeEvent event) {
         if (listeners != null) {
             for (PropertyChangeListener listener : listeners) {
                 listener.propertyChange(event);
@@ -301,78 +301,69 @@ public class PropertyChangeSupport implements Serializable {
     }
 
     /**
-     * Report a bound indexed property update to any registered
-     * listeners.
+     * Reports a bound indexed property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
      * <p>
-     * No event is fired if old and new values are equal
-     * and non-null.
-     *
+     * No event is fired if old and new values are equal and non-null.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * firePropertyChange method that takes {@code PropertyChangeEvent} value.
+     * {@link #firePropertyChange(PropertyChangeEvent)} method.
      *
-     * @param propertyName The programmatic name of the property that
-     *                     was changed.
-     * @param index        index of the property element that was changed.
-     * @param oldValue     The old value of the property.
-     * @param newValue     The new value of the property.
+     * @param propertyName  the programmatic name of the property that was changed
+     * @param index         the index of the property element that was changed
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
      * @since 1.5
      */
-    public void fireIndexedPropertyChange(String propertyName, int index,
-                                          Object oldValue, Object newValue) {
-        firePropertyChange(new IndexedPropertyChangeEvent
-            (source, propertyName, oldValue, newValue, index));
+    public void fireIndexedPropertyChange(String propertyName, int index, Object oldValue, Object newValue) {
+        if (oldValue == null || newValue == null || !oldValue.equals(newValue)) {
+            firePropertyChange(new IndexedPropertyChangeEvent(source, propertyName, oldValue, newValue, index));
+        }
     }
 
     /**
-     * Report an <code>int</code> bound indexed property update to any registered
-     * listeners.
+     * Reports an integer bound indexed property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
      * <p>
      * No event is fired if old and new values are equal.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * fireIndexedPropertyChange method which takes Object values.
+     * {@link #fireIndexedPropertyChange(String, int, Object, Object)} method.
      *
-     * @param propertyName The programmatic name of the property that
-     *                     was changed.
-     * @param index        index of the property element that was changed.
-     * @param oldValue     The old value of the property.
-     * @param newValue     The new value of the property.
+     * @param propertyName  the programmatic name of the property that was changed
+     * @param index         the index of the property element that was changed
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
      * @since 1.5
      */
-    public void fireIndexedPropertyChange(String propertyName, int index,
-                                          int oldValue, int newValue) {
-        if (oldValue == newValue) {
-            return;
+    public void fireIndexedPropertyChange(String propertyName, int index, int oldValue, int newValue) {
+        if (oldValue != newValue) {
+            fireIndexedPropertyChange(propertyName, index, Integer.valueOf(oldValue), Integer.valueOf(newValue));
         }
-        fireIndexedPropertyChange(propertyName, index,
-                                  Integer.valueOf(oldValue),
-                                  Integer.valueOf(newValue));
     }
 
     /**
-     * Report a <code>boolean</code> bound indexed property update to any
-     * registered listeners.
+     * Reports a boolean bound indexed property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
      * <p>
      * No event is fired if old and new values are equal.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * fireIndexedPropertyChange method which takes Object values.
+     * {@link #fireIndexedPropertyChange(String, int, Object, Object)} method.
      *
-     * @param propertyName The programmatic name of the property that
-     *                     was changed.
-     * @param index        index of the property element that was changed.
-     * @param oldValue     The old value of the property.
-     * @param newValue     The new value of the property.
+     * @param propertyName  the programmatic name of the property that was changed
+     * @param index         the index of the property element that was changed
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
      * @since 1.5
      */
-    public void fireIndexedPropertyChange(String propertyName, int index,
-                                          boolean oldValue, boolean newValue) {
-        if (oldValue == newValue) {
-            return;
+    public void fireIndexedPropertyChange(String propertyName, int index, boolean oldValue, boolean newValue) {
+        if (oldValue != newValue) {
+            fireIndexedPropertyChange(propertyName, index, Boolean.valueOf(oldValue), Boolean.valueOf(newValue));
         }
-        fireIndexedPropertyChange(propertyName, index, Boolean.valueOf(oldValue),
-                                  Boolean.valueOf(newValue));
     }
 
     /**

src/share/classes/java/beans/VetoableChangeSupport.java

 /*
- * Copyright 1996-2006 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright 1996-2008 Sun Microsystems, Inc.  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
@@ -208,126 +208,149 @@ public class VetoableChangeSupport implements Serializable {
     }
 
     /**
-     * Report a vetoable property update to any registered listeners.  If
-     * anyone vetos the change, then fire a new event reverting everyone to
-     * the old value and then rethrow the PropertyVetoException.
+     * Reports a constrained property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
      * <p>
-     * No event is fired if old and new are equal and non-null.
+     * Any listener can throw a {@code PropertyVetoException} to veto the update.
+     * If one of the listeners vetoes the update, this method passes
+     * a new "undo" {@code PropertyChangeEvent} that reverts to the old value
+     * to all listeners that already confirmed this update
+     * and throws the {@code PropertyVetoException} again.
+     * <p>
+     * No event is fired if old and new values are equal and non-null.
+     * <p>
+     * This is merely a convenience wrapper around the more general
+     * {@link #fireVetoableChange(PropertyChangeEvent)} method.
      *
-     * @param propertyName  The programmatic name of the property
-     *          that is about to change..
-     * @param oldValue  The old value of the property.
-     * @param newValue  The new value of the property.
-     * @exception PropertyVetoException if the recipient wishes the property
-     *              change to be rolled back.
+     * @param propertyName  the programmatic name of the property that is about to change
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
+     * @throws PropertyVetoException if one of listeners vetoes the property update
      */
-    public void fireVetoableChange(String propertyName,
-                                        Object oldValue, Object newValue)
-                                        throws PropertyVetoException {
-        if (oldValue != null && newValue != null && oldValue.equals(newValue)) {
-            return;
+    public void fireVetoableChange(String propertyName, Object oldValue, Object newValue)
+            throws PropertyVetoException {
+        if (oldValue == null || newValue == null || !oldValue.equals(newValue)) {
+            fireVetoableChange(new PropertyChangeEvent(this.source, propertyName, oldValue, newValue));
         }
-        PropertyChangeEvent evt = new PropertyChangeEvent(source, propertyName,
-                                                            oldValue, newValue);
-        fireVetoableChange(evt);
     }
 
     /**
-     * Report a int vetoable property update to any registered listeners.
-     * No event is fired if old and new are equal.
+     * Reports an integer constrained property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
+     * <p>
+     * Any listener can throw a {@code PropertyVetoException} to veto the update.
+     * If one of the listeners vetoes the update, this method passes
+     * a new "undo" {@code PropertyChangeEvent} that reverts to the old value
+     * to all listeners that already confirmed this update
+     * and throws the {@code PropertyVetoException} again.
+     * <p>
+     * No event is fired if old and new values are equal.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * fireVetoableChange method that takes Object values.
+     * {@link #fireVetoableChange(String, Object, Object)} method.
      *
-     * @param propertyName  The programmatic name of the property
-     *          that is about to change.
-     * @param oldValue  The old value of the property.
-     * @param newValue  The new value of the property.
+     * @param propertyName  the programmatic name of the property that is about to change
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
+     * @throws PropertyVetoException if one of listeners vetoes the property update
      */
-    public void fireVetoableChange(String propertyName,
-                                        int oldValue, int newValue)
-                                        throws PropertyVetoException {
-        if (oldValue == newValue) {
-            return;
+    public void fireVetoableChange(String propertyName, int oldValue, int newValue)
+            throws PropertyVetoException {
+        if (oldValue != newValue) {
+            fireVetoableChange(propertyName, Integer.valueOf(oldValue), Integer.valueOf(newValue));
         }
-        fireVetoableChange(propertyName, Integer.valueOf(oldValue), Integer.valueOf(newValue));
     }
 
     /**
-     * Report a boolean vetoable property update to any registered listeners.
-     * No event is fired if old and new are equal.
+     * Reports a boolean constrained property update to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
+     * <p>
+     * Any listener can throw a {@code PropertyVetoException} to veto the update.
+     * If one of the listeners vetoes the update, this method passes
+     * a new "undo" {@code PropertyChangeEvent} that reverts to the old value
+     * to all listeners that already confirmed this update
+     * and throws the {@code PropertyVetoException} again.
+     * <p>
+     * No event is fired if old and new values are equal.
      * <p>
      * This is merely a convenience wrapper around the more general
-     * fireVetoableChange method that takes Object values.
+     * {@link #fireVetoableChange(String, Object, Object)} method.
      *
-     * @param propertyName  The programmatic name of the property
-     *          that is about to change.
-     * @param oldValue  The old value of the property.
-     * @param newValue  The new value of the property.
+     * @param propertyName  the programmatic name of the property that is about to change
+     * @param oldValue      the old value of the property
+     * @param newValue      the new value of the property
+     * @throws PropertyVetoException if one of listeners vetoes the property update
      */
-    public void fireVetoableChange(String propertyName,
-                                        boolean oldValue, boolean newValue)
-                                        throws PropertyVetoException {
-        if (oldValue == newValue) {
-            return;
+    public void fireVetoableChange(String propertyName, boolean oldValue, boolean newValue)
+            throws PropertyVetoException {
+        if (oldValue != newValue) {
+            fireVetoableChange(propertyName, Boolean.valueOf(oldValue), Boolean.valueOf(newValue));
         }
-        fireVetoableChange(propertyName, Boolean.valueOf(oldValue), Boolean.valueOf(newValue));
     }
 
     /**
-     * Fire a vetoable property update to any registered listeners.  If
-     * anyone vetos the change, then fire a new event reverting everyone to
-     * the old value and then rethrow the PropertyVetoException.
+     * Fires a property change event to listeners
+     * that have been registered to track updates of
+     * all properties or a property with the specified name.
      * <p>
-     * No event is fired if old and new are equal and non-null.
+     * Any listener can throw a {@code PropertyVetoException} to veto the update.
+     * If one of the listeners vetoes the update, this method passes
+     * a new "undo" {@code PropertyChangeEvent} that reverts to the old value
+     * to all listeners that already confirmed this update
+     * and throws the {@code PropertyVetoException} again.
+     * <p>
+     * No event is fired if the given event's old and new values are equal and non-null.
      *
-     * @param evt  The PropertyChangeEvent to be fired.
-     * @exception PropertyVetoException if the recipient wishes the property
-     *              change to be rolled back.
+     * @param event  the {@code PropertyChangeEvent} to be fired
+     * @throws PropertyVetoException if one of listeners vetoes the property update
      */
-    public void fireVetoableChange(PropertyChangeEvent evt)
-                                        throws PropertyVetoException {
+    public void fireVetoableChange(PropertyChangeEvent event)
+            throws PropertyVetoException {
+        Object oldValue = event.getOldValue();
+        Object newValue = event.getNewValue();
+        if (oldValue == null || newValue == null || !oldValue.equals(newValue)) {
+            String name = event.getPropertyName();
 
-        Object oldValue = evt.getOldValue();
-        Object newValue = evt.getNewValue();
-        String propertyName = evt.getPropertyName();
-        if (oldValue != null && newValue != null && oldValue.equals(newValue)) {
-            return;
-        }
-        VetoableChangeListener[] common = this.map.get(null);
-        VetoableChangeListener[] named = (propertyName != null)
-                    ? this.map.get(propertyName)
-                    : null;
-        fire(common, evt);
-        fire(named, evt);
-    }
+            VetoableChangeListener[] common = this.map.get(null);
+            VetoableChangeListener[] named = (name != null)
+                        ? this.map.get(name)
+                        : null;
 
-    private void fire(VetoableChangeListener[] listeners, PropertyChangeEvent event) throws PropertyVetoException {
-        if (listeners != null) {
-            VetoableChangeListener current = null;
-            try {
-                for (VetoableChangeListener listener : listeners) {
-                    current = listener;
-                    listener.vetoableChange(event);
-                }
-            } catch (PropertyVetoException veto) {
-                // Create an event to revert everyone to the old value.
-                event = new PropertyChangeEvent( this.source,
-                                                 event.getPropertyName(),
-                                                 event.getNewValue(),
-                                                 event.getOldValue() );
-                for (VetoableChangeListener listener : listeners) {
-                    if (current == listener) {
-                        break;
+            VetoableChangeListener[] listeners;
+            if (common == null) {
+                listeners = named;
+            }
+            else if (named == null) {
+                listeners = common;
+            }
+            else {
+                listeners = new VetoableChangeListener[common.length + named.length];
+                System.arraycopy(common, 0, listeners, 0, common.length);
+                System.arraycopy(named, 0, listeners, common.length, named.length);
+            }
+            if (listeners != null) {
+                int current = 0;
+                try {
+                    while (current < listeners.length) {
+                        listeners[current].vetoableChange(event);
+                        current++;
                     }
-                    try {
-                        listener.vetoableChange(event);
-                    } catch (PropertyVetoException ex) {
-                         // We just ignore exceptions that occur during reversions.
+                }
+                catch (PropertyVetoException veto) {
+                    event = new PropertyChangeEvent(this.source, name, newValue, oldValue);
+                    for (int i = 0; i < current; i++) {
+                        try {
+                            listeners[i].vetoableChange(event);
+                        }
+                        catch (PropertyVetoException exception) {
+                            // ignore exceptions that occur during rolling back
+                        }
                     }
+                    throw veto; // rethrow the veto exception
                 }
-                // And now rethrow the PropertyVetoException.
-                throw veto;
             }
         }
     }

test/java/beans/VetoableChangeSupport/Test6630275.java

+/*
+ * Copyright 2008 Sun Microsystems, Inc.  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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test
+ * @bug 6630275
+ * @summary Tests VetoableChangeSupport specification
+ * @author Sergey Malenkov
+ */
+
+import java.beans.PropertyChangeEvent;
+import java.beans.PropertyVetoException;
+import java.beans.VetoableChangeListener;
+import java.beans.VetoableChangeSupport;
+
+public class Test6630275 {
+    private static final String PROPERTY = "property"; // NON-NLS: predefined property name
+
+    public static void main(String[] args) {
+        CheckListener first = new CheckListener(false);
+        CheckListener second = new CheckListener(true);
+        CheckListener third = new CheckListener(false);
+
+        VetoableChangeSupport vcs = new VetoableChangeSupport(Test6630275.class);
+        vcs.addVetoableChangeListener(first);
+        vcs.addVetoableChangeListener(PROPERTY, first);
+        vcs.addVetoableChangeListener(PROPERTY, second);
+        vcs.addVetoableChangeListener(PROPERTY, third);
+        try {
+            vcs.fireVetoableChange(PROPERTY, true, false);
+        } catch (PropertyVetoException exception) {
+            first.validate();
+            second.validate();
+            third.validate();
+            return; // expected exception
+        }
+        throw new Error("exception should be thrown");
+    }
+
+    private static class CheckListener implements VetoableChangeListener {
+        private final boolean veto;
+        private boolean odd; // even/odd check for notification
+
+        private CheckListener(boolean veto) {
+            this.veto = veto;
+        }
+
+        private void validate() {
+            if (this.veto != this.odd)
+                throw new Error(this.odd
+                        ? "undo event expected"
+                        : "unexpected undo event");
+        }
+
+        public void vetoableChange(PropertyChangeEvent event) throws PropertyVetoException {
+            this.odd = !this.odd;
+            if (this.veto)
+                throw new PropertyVetoException("disable all changes", event);
+        }
+    }
+}