United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-4937059 : REGRESSION: java.beans.PropertyChangeSupport.add/remove/getPCL(s): unclear doc

Details
Type:
Bug
Submit Date:
2003-10-14
Status:
Closed
Updated Date:
2004-03-20
Project Name:
JDK
Resolved Date:
2004-02-28
Component:
client-libs
OS:
solaris_2.6
Sub-Component:
java.beans
CPU:
sparc
Priority:
P3
Resolution:
Fixed
Affected Versions:
5.0
Fixed Versions:
5.0 (b41)

Related Reports
Relates:

Sub Tasks

Description

Name: sdR10048			Date: 10/14/2003


Filed By      : SPB JCK team (###@###.###)
JDK           : java full version "1.5.0-beta-b23"
JCK           : 1.5
Platform[s]   : Solaris
switch/Mode   : 
JCK test owner : http://javaweb.eng/jct/sqe/JCK-tck/usr/owners.jto
Failing Test [s] :
api/java_beans/PropertyChangeSupport/descriptions.html#PropertySupport [PropertyChangeSupport0006] 
api/java_beans/PropertyChangeSupport/descriptions.html#PropertySupport12 [PropertyChangeSupport0015] 

Specification excerpt:
======================
--------- J2SE API spec v.1.5 ---------
...
public void addPropertyChangeListener(PropertyChangeListener listener)
Add a PropertyChangeListener to the listener list. The listener is registered for all properties. 

Parameters:
listener - The PropertyChangeListener to be added

===

public void removePropertyChangeListener(PropertyChangeListener listener)
Remove a PropertyChangeListener from the listener list. This removes a PropertyChangeListener that was registered for all properties. 

Parameters:
listener - The PropertyChangeListener to be removed

===

public void addPropertyChangeListener(String propertyName,
                                      PropertyChangeListener listener)
Add a PropertyChangeListener for a specific property. The listener will be invoked only when a call on firePropertyChange names that specific property. 

Parameters:
propertyName - The name of the property to listen on.
listener - The PropertyChangeListener to be added

===

public void removePropertyChangeListener(String propertyName,
                                         PropertyChangeListener listener)
Remove a PropertyChangeListener for a specific property. 

Parameters:
propertyName - The name of the property that was listened on.
listener - The PropertyChangeListener to be removed

===

public PropertyChangeListener[] getPropertyChangeListeners(String propertyName)
Returns an array of all the listeners which have been associated with the named property. 

Returns:
all of the PropertyChangeListeners associated with the named property or an empty array if no listeners have been added

...
---------- end-of-excerpt ---------------

Problem description
===================
The doc for designated methods does not specify the expected behaviour
in the following cases:

1. addPropertyChangeListener(PropertyChangeListener listener)
   if listener == null;

2. addPropertyChangeListener(PropertyChangeListener listener)
   if listener specified is already added;

3. removePropertyChangeListener(PropertyChangeListener listener)
   if listener == null

4. removePropertyChangeListener(PropertyChangeListener listener)
   if there is no such listener has been added

5. addPropertyChangeListener(String propertyName,
                                      PropertyChangeListener listener)
   if propertyName == null;

6. addPropertyChangeListener(String propertyName,
                                      PropertyChangeListener listener)
   if listener == null;

7. addPropertyChangeListener(String propertyName,
                                      PropertyChangeListener listener)
   if the listener has been added already

8. removePropertyChangeListener(String propertyName,
                                         PropertyChangeListener listener)
   if propertyName == null;

9. removePropertyChangeListener(String propertyName,
                                         PropertyChangeListener listener)
   if listener == null;

10. getPropertyChangeListeners(String propertyName)
    if listener == null;

Please refer to the "Requirements for Writing Java API Specifications"
 (http://java.sun.com/j2se/javadoc/writingapispecs/index.html#method)
for more information.

JCK test source location:
==========================
/java/re/jck/1.5/promoted/latest/JCK-runtime-15/tests

======================================================================

                                    

Comments
CONVERTED DATA

BugTraq+ Release Management Values

COMMIT TO FIX:
tiger-beta2

FIXED IN:
tiger-beta2

INTEGRATED IN:
tiger-b41
tiger-beta2

VERIFIED IN:
tiger-b40


                                     
2004-06-14
SUGGESTED FIX

------- PropertyChangeSupport.java -------
*** /tmp/sccs.ewairm    Wed Feb  4 16:39:42 2004
--- PropertyChangeSupport.java  Tue Feb  3 19:58:48 2004
***************
*** 49,54 ****
--- 49,58 ----
      /**
       * Add a PropertyChangeListener to the listener list.
       * The listener is registered for all properties.
+      * The same listener object may be added more than once, and will be called
+      * as many times as it is added.
+      * If <code>listener</code> is null, no exception is thrown and no action
+      * is taken.
       *
       * @param listener  The PropertyChangeListener to be added
       */
***************
*** 76,81 ****
--- 80,87 ----
       * Remove a PropertyChangeListener from the listener list.
       * This removes a PropertyChangeListener that was registered
       * for all properties.
+      * If <code>listener</code> is null, or was never added, no exception is
+      * thrown and no action is taken.
       *
       * @param listener  The PropertyChangeListener to be removed
       */
***************
*** 162,167 ****
--- 168,178 ----
       * Add a PropertyChangeListener for a specific property.  The listener
       * will be invoked only when a call on firePropertyChange names that
       * specific property.
+      * The same listener object may be added more than once.  For each
+      * property,  the listener will be invoked the number of times it was added
+      * for that property.
+      * If <code>listener</code> is null, no exception is thrown and no action
+      * is taken.
       *
       * @param propertyName  The name of the property to listen on.
       * @param listener  The PropertyChangeListener to be added
***************
*** 168,204 ****
       */
  
      public synchronized void addPropertyChangeListener(
!                               String propertyName,
!                               PropertyChangeListener listener) {
!       if (children == null) {
!           children = new java.util.Hashtable();
!       }
!       PropertyChangeSupport child = (PropertyChangeSupport)children.get(propertyName);
!       if (child == null) {
!           child = new PropertyChangeSupport(source);
!           children.put(propertyName, child);
!       }
!       child.addPropertyChangeListener(listener);
      }
  
      /**
       * Remove a PropertyChangeListener for a specific property.
       *
       * @param propertyName  The name of the property that was listened on.
       * @param listener  The PropertyChangeListener to be removed
       */
  
      public synchronized void removePropertyChangeListener(
!                               String propertyName,
!                               PropertyChangeListener listener) {
!       if (children == null) {
!           return;
!       }
!       PropertyChangeSupport child = (PropertyChangeSupport)children.get(propertyName);
!       if (child == null) {
!           return;
!       }
!       child.removePropertyChangeListener(listener);
      }
  
      /**
--- 179,224 ----
       */
  
      public synchronized void addPropertyChangeListener(
!                 String propertyName,
!                 PropertyChangeListener listener) {
!         if (listener == null) {
!             return;
!         }
!         if (children == null) {
!             children = new java.util.Hashtable();
!         }
!         PropertyChangeSupport child = (PropertyChangeSupport)children.get(propertyName);
!         if (child == null) {
!             child = new PropertyChangeSupport(source);
!             children.put(propertyName, child);
!         }
!         child.addPropertyChangeListener(listener);
      }
  
      /**
       * Remove a PropertyChangeListener for a specific property.
       *
+      * If <code>listener</code> is null, or was never added for the specified
+      * property, no exception is thrown and no action is taken.
+      *
       * @param propertyName  The name of the property that was listened on.
       * @param listener  The PropertyChangeListener to be removed
       */
  
      public synchronized void removePropertyChangeListener(
!                 String propertyName,
!                 PropertyChangeListener listener) {
!         if (listener == null) {
!             return;
!         }
!         if (children == null) {
!             return;
!         }
!         PropertyChangeSupport child = (PropertyChangeSupport)children.get(propertyName);
!         if (child == null) {
!             return;
!         }
!         child.removePropertyChangeListener(listener);
      }
  
      /**

###@###.### 2004-02-04
                                     
2004-02-04
EVALUATION


java.beans.PropertyChangeSupport

The following items correspond to those listed in the bug report.

1. addPropertyChangeListener(PropertyChangeListener listener)
   if listener == null;

   The implementation checks for null listener and simply returns if true.
   We could specify that passing null to this method will have no effect.

2. addPropertyChangeListener(PropertyChangeListener listener)
   if listener specified is already added;

   The current implementation allows the same listener object to be added more than once.
   The descision as to whether or not this is the intended behavior must be made by engineering.
   This can be tested by creating a PropertyChangeSupport object, invoking this method
   with the same parameter two times, and querying the PropertyChangeSupport for its
   registered listeners. Calling toString on the returned listeners in this case shows that
   the registered listeners are the same:

   MyPropertyChangeListener@1ba34f2
   MyPropertyChangeListener@1ba34f2

   If the implementation is correct, we could specify that it is
   possible to add the same listener multiple times.

3. removePropertyChangeListener(PropertyChangeListener listener)
   if listener == null

   The implementation checks for null listener and simply returns if true.
   We could specify that passing null to this method will have no effect.

4. removePropertyChangeListener(PropertyChangeListener listener)
   if there is no such listener has been added

   The current implementation does nothing when attempting to remove a listener
   that has not been added. We could specify that such an attempt will have no effect.
   The decision as to whether or not this is the intended behavior must be made by engineering.


5. addPropertyChangeListener(String propertyName, PropertyChangeListener listener)
   if propertyName == null;

   The implementation of this method does not check for null propertyName.
   It attempts to get the propertyName from the "children" Hashtable object.
   This case is covered by the generic null parameter documentation at the
   package level.

6. addPropertyChangeListener(String propertyName, PropertyChangeListener listener)
   if listener == null;

   The implementation does not check for null listener.
   It passes the listener to child.addPropertyChangeListener(listener),
   where "child" is a PropertyChangeSupport object.
   See item 1 above. We could specify that passing null to this method will
   have no effect.


7. addPropertyChangeListener(String propertyName, PropertyChangeListener listener)
   if the listener has been added already

   The current implementation allows the same listener object to be added more than once.
   This can be tested by creating a PropertyChangeSupport object, invoking this method
   with the same parameters two times, and querying the PropertyChangeSupport for its
   registered listeners. Calling toString on the returned listeners in this case shows that
   the registered listeners are different even though the same object refs were passed
   as params to both calls:

   java.beans.PropertyChangeListenerProxy@45a877
   java.beans.PropertyChangeListenerProxy@1372a1a

   Engineering must decide how to specify this method.


8. removePropertyChangeListener(String propertyName, PropertyChangeListener listener)
   if propertyName == null;

   The implementation does not check for null propertyName. It passes propertyName to
   children.get(propertyName), where "children" is a Hashtable object.
   This case is covered by the generic null parameter documentation at the
   package level.

9. removePropertyChangeListener(String propertyName, PropertyChangeListener listener)
   if listener == null;

   The implementation checks to see if listener is null. It simply returns if true.
   We could specify that passing null listener to this method will have no effect.


10. getPropertyChangeListeners(String propertyName)
    if listener == null;

    This must mean if propertyName==null.

   The implementation does not check for null propertyName. It passes propertyName to
   children.get(propertyName), where "children" is a Hashtable object.
   This case is covered by the generic null parameter documentation at the
   package level.


###@###.### 2003-10-30


Update for the above:

1. JCK requests that we specify
addPropertyChangeListner(PropertyChangeListener listener)
for the case of listener being null. The current implementation has no
effect if this parameter is null, but engineering does not want this
current behavior specified. Engineering says that we should only document
null when it either changes the semantics or throws a NPE.


2. JCK requests that we specify 
addPropertyChangeListener(PropertyChangeListener listener)
for adding the same listener object more than once. The current 
implementation allows the same object to be registered as a listener more
than once. Engineering does not want this behavior documented. In the
future they may change the implementation so that only unique listeners
can be added.

3. JCK requests that we specify 
removePropertyChangeListener(PropertyChangeListener listener)
for the case of listener being null. Passing null to this method in the
current implementation has no effect. Engineering does not want this
documented for reasons stated in #1 above.

4. JCK requests that we specify
removePropertyChangeListener(PropertyChangeListener listener)
for removing a listener that has not been added. In the current
implementation, attempting to do so will have no effect. Engineering does
not want this behavior documented for reasons stated in #1 above.

5. JCK requests that we specify addPropertyChangeListener(String
propertyName, PropertyChangeListener listener) for the case of 
propertyName being null. The current implementation
does not does not check for null propertyName. It attempts to get the 
propertyName from an internal Hashtable. Engineering regards this
as a code bug, saying that propertyName == null should probably call
addPCL(PCL). Will file a separate bug report against this method.

6. JCK requests that we specify addPropertyChangeListener(String
propertyName, PropertyChangeListener listener) for the case of listener
being null. Passing null to this method has no effect in the current
implementation. Engineering does not want this behavior documented for
reasons stated in #1 above.

7. JCK requests that we specify addPropertyChangeListener(String
propertyName, PropertyChangeListener listener) if the listener has already
been added. The current implementation allows the same listener to be
added more than once. Engineering does not want us to specify this current
behavior for reasons stated in #2 above.

8. JCK requests that we specify removePropertyChangeListener(String
propertyName, PropertyChangeListener listener) for the case of listener
being null. The current implementation does
not check for a null listener. Engineering regards this as a bug, saying
that it should probably call removePCL(PCL).
This will be covered in the new bug report for #5 above.

9. JCK requests that we specify removePropertyChangeListener(String
propertyName, PropertyChangeListener listener) for the case of listener
being null. Passing null to this method has no effect in the current
implementation. Engineering does not want this behavior documented for
reasons stated in #1 above.

10. JCK requests that we specify getPropertyChangeListeners(String
propertyName) for the case of propertyName being null. The current
implementation does not check for null in this case.  
Engineering regards this as a bug, saying that it should probably call
getPropertyChangeListeners().
This will be included in the new bug report for #5 above.


To summarize:

1. null parameters with no effects should not be specified. This covers
cases 1,3,4,6, and 9 above.
2. duplicate listeners are allowed, but shouldn't be documented since it's
an implementation detail. This covers cases 2 and 7 above.
3. Named properties where the propertyName is null should call the
non-named versions. A separate bug report will be created to address
this. This includes cases 5, 8, and 10 above.



###@###.### 2003-11-03

A separate bug has been filed for cases 5, 8, and 10.  See 4985020.
###@###.### 2004-01-28

Though most of thes items are specified in other places, it could be helpful to have it repeated in the JavaDoc itself.  I've decided on a solution along those lines.
###@###.### 2004-01-30

Suggested Fix contains the latest iteration of the doc changes.
###@###.### 2004-02-04
                                     
2004-01-30



Hardware and Software, Engineered to Work Together