On 26.05.16 19:26, Semyon Sadetsky wrote:
The thing is that you are making decision, which method to discard, at
the moment you analyze a particular type within the class hierarchy. But
this cannot be correct regardless which filter is used for
Class.getMethods() the random or alphabetical. The correct decision may
be only made at the moment you have collected methods from all types of
the hierarchy that is subject for introspection. Otherwise you always
has probability to discard methods that may be finally needed to create
the correct property descriptors of the selected properties, also the
property selection itself may be wrong if it is done without all
available methods.
The current decision is correct and works according the specification,
if the code contains the properties which contradicts each other we can
select any of them, because this behavior is unspecified, it does not
matter are mislead properties comes from one class or from some other
parent class. Currently for each class we generate the list of
properties and then merge each property if subclass provide an extension
of this property.
I think the JDK-8157828 issue you've created is about the same problem
as the issue you are trying to fix here. It seems it cannot be solved on
the Class.getMethods() level, it should be solved in the introspector
itself.
That's the separate bug, this one is about "Unstable behavior of
PropertyDescriptor's getWriteMethod() in case of overloaded setters" and
the root cause here is that the same code which is executed on a
different systems behave differently. After the fix in all cases the
same code will work unconditionally from the system where it is
executed. Do you have an objections that the change is not fix this
instability?
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
--
Best regards, Sergey.
