<Beans Dev> [9] Review Request: 8156043 Unstable behavior of PropertyDescriptor's getWriteMethod() in case of overloaded setters

[Semyon Sadetsky]

On 5/27/2016 2:56 PM, Sergey Bylokhov wrote:
> On 27.05.16 10:51, Semyon Sadetsky wrote:
>>> Do you have some other objections about the current fix? Are there
>>> some issues in the added code, without linking some other known bugs,
>>> or you doubts that the order still unstable?
>> Yes. I don't like the rank you use to discard properties. I'd prefer to
>> have it not the biggest alphabeticaly but more meaningful.
>
> It is obviously that the added code discards nothing, it is a wrapper 
> on top of the Class.getMethods(), which make the order of methods stable.
Right. But only the first overloaded method in this order will survive. 
So one way or another this order determines which property to discard.
>
>>>>>>> And why it is not discarded if I change SUB.setFoo(Integer) to
>>>>>>> SUB.setFoo(String) for example? That seems very odd.
>>>>>>
>>>>>> I assume because in this case instead of foo=int we will select
>>>>>> foo=long and will skip foo=String.
>>>>>>
>>>>>>>
>>>>>>> And before your fix there was no such behavior. You wrote that 
>>>>>>> it was
>>>>>>> random before the fix, but I ran the test may times and could 
>>>>>>> not get
>>>>>>> the same effect.
>>>>>>> So I suppose it was introduced by the fix.
>>>>>>
>>>>>> It means that when you run the app Class.getMethods() always return
>>>>>> the same order of methods. You can add a logging to the test 
>>>>>> attached
>>>>>> to the fix and run it on current jdk9, you will see that the type of
>>>>>> foo property is different time to time.
>>>>>>
>>>>>>>
>>>>>>> Also there is yet another problem here. The default interface 
>>>>>>> methods
>>>>>>> are totally ignored. This is probably a separate issue.
>>>>>>
>>>>>> This is a known bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8139193
>>>>>>
>>>>>>>
>>>>>>> --Semyon
>>>>>>>>
>>>>>>>>>
>>>>>>>>> In my opinion, the method may return any of this two (also a
>>>>>>>>> secondary
>>>>>>>>> rank could be used), but not a mixture of them in any
>>>>>>>>> circumstances.
>>>>>>>>>
>>>>>>>>> --Semyon
>>>>>>>>>
>>>>>>>>>>>>> In my understanding, if I override the setter I should get 
>>>>>>>>>>>>> the
>>>>>>>>>>>>> over-ridden method for the top level class introspection.
>>>>>>>>>>>>> Otherwise
>>>>>>>>>>>>> I do
>>>>>>>>>>>>> not understand for what such introspector can be used:
>>>>>>>>>>>>>
>>>>>>>>>>>>> I have some object which property I want to set, but using 
>>>>>>>>>>>>> the
>>>>>>>>>>>>> introspector for that I'm calling a wrong method which
>>>>>>>>>>>>> breaks the
>>>>>>>>>>>>> inheritance and so corrupts the object state.
>>>>>>>>>>>>>
>>>>>>>>>>>>> And moreover, if I reverse the situation and override the
>>>>>>>>>>>>> getter in
>>>>>>>>>>>>> Sub
>>>>>>>>>>>>> instead of setter :
>>>>>>>>>>>>>
>>>>>>>>>>>>> class Super {
>>>>>>>>>>>>>     public void setFoo(Long i) {}
>>>>>>>>>>>>>     public Long getFoo() {return null;}
>>>>>>>>>>>>> }
>>>>>>>>>>>>> class Sub extends Super {
>>>>>>>>>>>>>     public Long getFoo() {return null;}
>>>>>>>>>>>>> }
>>>>>>>>>>>>>
>>>>>>>>>>>>> the introspector returns the over-ridden getter method for 
>>>>>>>>>>>>> the
>>>>>>>>>>>>> property :
>>>>>>>>>>>>>
>>>>>>>>>>>>> type: class java.lang.Long
>>>>>>>>>>>>> getter: public java.lang.Long Sub.getFoo()
>>>>>>>>>>>>> setter: public void Super.setFoo(java.lang.Long)
>>>>>>>>>>>>>
>>>>>>>>>>>>> This looks inconsistent.
>>>>>>>>>>>>>
>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 5/25/2016 2:14 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>>>>> On 25.05.16 11:38, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>>>> In this case the type of foo property will be Enum, 
>>>>>>>>>>>>>>>>>> before
>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>> after
>>>>>>>>>>>>>>>>>> the fix. But the write method will be found only if this
>>>>>>>>>>>>>>>>>> method is
>>>>>>>>>>>>>>>>>> added to Sub, in other case the write method is 
>>>>>>>>>>>>>>>>>> recognized
>>>>>>>>>>>>>>>>>> only
>>>>>>>>>>>>>>>>>> if we
>>>>>>>>>>>>>>>>>> remove all duplicates of set(xxx). Not sure is it 
>>>>>>>>>>>>>>>>>> intended
>>>>>>>>>>>>>>>>>> behavior in
>>>>>>>>>>>>>>>>>> jdk9 to skip such writers or not. I will file CR for 
>>>>>>>>>>>>>>>>>> that.
>>>>>>>>>>>>>>>>> That maybe an another issue.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I dig to the history and found that it was done
>>>>>>>>>>>>>>>> intentionally
>>>>>>>>>>>>>>>> when
>>>>>>>>>>>>>>>> JavaBean jep was implemented. but I filed
>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8157828 for
>>>>>>>>>>>>>>>> additional
>>>>>>>>>>>>>>>> investigation.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> But the current fix need to be checked by
>>>>>>>>>>>>>>>>> the scenario when there are several getters (over-ridden
>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> return
>>>>>>>>>>>>>>>>> type substitutability) in addition to the setters.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Tescase is updated, the case: getE + multiple setE is 
>>>>>>>>>>>>>>>> added:
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~serb/8156043/webrev.01/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 5/17/2016 3:20 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>>>>>>>>> Hello.
>>>>>>>>>>>>>>>>>>>> Please review the fix for jdk9.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> We have a number of bugs which state that our 
>>>>>>>>>>>>>>>>>>>> JavaBeans
>>>>>>>>>>>>>>>>>>>> randomly
>>>>>>>>>>>>>>>>>>>> does
>>>>>>>>>>>>>>>>>>>> not work, examples: JDK-6807471[1] , 
>>>>>>>>>>>>>>>>>>>> JDK-6788525[2], the
>>>>>>>>>>>>>>>>>>>> reason is
>>>>>>>>>>>>>>>>>>>> that the order of methods from Class.getMethods() 
>>>>>>>>>>>>>>>>>>>> is not
>>>>>>>>>>>>>>>>>>>> specified.
>>>>>>>>>>>>>>>>>>>> So I propose to fix this bug totally and sort the
>>>>>>>>>>>>>>>>>>>> methods in
>>>>>>>>>>>>>>>>>>>> some
>>>>>>>>>>>>>>>>>>>> order. Note that the resulted list is cached, and we
>>>>>>>>>>>>>>>>>>>> sort the
>>>>>>>>>>>>>>>>>>>> list
>>>>>>>>>>>>>>>>>>>> only the once.
>>>>>>>>>>>>>>>>>>>> The code partly was copied from
>>>>>>>>>>>>>>>>>>>> com.sun.jmx.mbeanserver.MethodOrder
>>>>>>>>>>>>>>>>>>>> [3], but the parameters check and the order for return
>>>>>>>>>>>>>>>>>>>> values
>>>>>>>>>>>>>>>>>>>> were
>>>>>>>>>>>>>>>>>>>> changed. After this fix our bugs(if any) can be easily
>>>>>>>>>>>>>>>>>>>> reproduced.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-6807471
>>>>>>>>>>>>>>>>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-6788525
>>>>>>>>>>>>>>>>>>>> [3]
>>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/fb38b0925915/src/java.management/share/classes/com/sun/jmx/mbeanserver/MBeanAnalyzer.java 
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156043
>>>>>>>>>>>>>>>>>>>> Webrev can be found at:
>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~serb/8156043/webrev.00
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>
>