RFR: JDK-6515161 If remote removeNotificationListener gets SecurityException, client no longer gets notifications

[Ujwal Vangapally]

Hi Harsha, Daniel,

updated method names as getListenerIds/getListenerId

webrev : 
http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.02/


On 5/8/2017 9:57 PM, Daniel Fuchs wrote:
> Hi Harsha,
>
> Well, I think both methods should have the same name
> and I don't see any strong reason for changing the old
> name - since there is no such thing as a 'ClientListener'
> (there are only NotificationListeners).
>
> best regards,
>
> -- daniel
>
> On 08/05/2017 17:13, Harsha Wardhana B wrote:
>> Hi Daniel,
>>
>> The term 'listenerid' is used in conjunction with method name/object
>> field which adds context about the term 'listenerid'. Having a
>> standalone method name as getClientListenerId is less ambiguous compared
>> to method name : getListenerId.
>>
>> I don't really have a strong opinion on this and am okay with either 
>> names.
>>
>> -Harsha
>>
>>
>> On Monday 08 May 2017 03:16 PM, Daniel Fuchs wrote:
>>> On 08/05/2017 09:45, Ujwal Vangapally wrote:
>>>> Thanks for the Review Daniel, Harsha
>>>>
>>>> Please find the new webrev incorporating the review comments
>>>>
>>>> webrev :
>>>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.01/
>>>
>>> Looks good. In retrospect I wonder if renaming
>>> getListenerIds/getListenerId into
>>> getClientListenerIds/getClientListenerId
>>> was such a good idea given that 'listenerId' is an established
>>> terminology in the specification [1] [2] (and 'listenerId' is used all
>>> over the place in ClientNotifForwarder anyway).
>>>
>>> Harsha, what do you think?
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> [1] See @return in
>>> http://download.java.net/java/jdk9/docs/api/javax/management/remote/rmi/RMIConnection.html#addNotificationListeners-javax.management.ObjectName:A-java.rmi.MarshalledObject:A-javax.security.auth.Subject:A- 
>>>
>>>
>>> [2] see @param listenerIds in
>>> http://download.java.net/java/jdk9/docs/api/javax/management/remote/rmi/RMIConnection.html#removeNotificationListeners-javax.management.ObjectName-java.lang.Integer:A-javax.security.auth.Subject- 
>>>
>>>
>>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Ujwal
>>>>
>>>>
>>>> On 5/8/2017 12:03 PM, Daniel Fuchs wrote:
>>>>> Hi Ujwal,
>>>>>
>>>>> For consistency I think the new getListenerIds method should:
>>>>>
>>>>> a) either return an array of Integer, even if it contains only 1
>>>>> Integer:
>>>>>
>>>>>   1. The name of the method implies that an array is returned
>>>>>   2. You will need the array when you call
>>>>>      connection.removeNotificationListeners anyway.
>>>>>
>>>>> or b) the other possibility is to remove the 's' at the end of the
>>>>> method.
>>>>>
>>>>> Both would be acceptable.
>>>>>
>>>>> best regards,
>>>>>
>>>>> -- daniel
>>>>>
>>>>> On 04/05/17 07:59, Ujwal Vangapally wrote:
>>>>>> corrected webrev link :
>>>>>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.00/ 
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 5/4/2017 12:14 PM, Ujwal Vangapally wrote:
>>>>>>>
>>>>>>> Kindly review the changes made for below bug
>>>>>>>
>>>>>>> Problem description and solution are explained in comments section
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-6515161
>>>>>>>
>>>>>>> diff for*ClientNotifForwarder.java *might be a bit confusing as it
>>>>>>> shows the method name
>>>>>>>
>>>>>>> removeNotificationListener is modified to getListenerIds and new
>>>>>>> method removeNotificationListener is introduced.
>>>>>>>
>>>>>>> Actual change is new method getListenerIds is introduced and it is
>>>>>>> called in removeNotificationListener method without
>>>>>>>
>>>>>>> affecting the functionality of removeNotificationListener.
>>>>>>>
>>>>>>> webrev :
>>>>>>> cr.openjdk.java.net/~uvangapally/webrev/2017/6515161/webrev.00/
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Ujwal,
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>