JDK-4937059 : REGRESSION: java.beans.PropertyChangeSupport.add/remove/getPCL(s): unclear doc
  • Type: Bug
  • Component: client-libs
  • Sub-Component: java.beans
  • Affected Version: 5.0
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: solaris_2.6
  • CPU: sparc
  • Submitted: 2003-10-14
  • Updated: 2017-05-16
  • Resolved: 2004-02-28
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
5.0 b41Fixed
Related Reports
Relates :  
JDK-4985020 - Named property methods need spec for when propertyName is null
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
14-06-2004
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
04-02-2004
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
30-01-2004