On Thu, 6 Feb 2025 14:13:50 GMT, Roman Marchenko <rmarche...@openjdk.org> wrote:

>> Fixed `com.sun.beans.introspect.MethodInfo#MethodOrder` to make 
>> `Introspector.addMethod()` working properly when filtering methods out.
>> 
>> Also fixed the test, and added the approptiate test case.
>
> Roman Marchenko has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update test/jdk/java/beans/Introspector/DefaultMethodBeanPropertyTest.java
>   
>   Co-authored-by: Aleksandr Zvegintsev 
> <77687766+azveg...@users.noreply.github.com>

> > Or, most likely, you can skip adding the default method in MethodInfo.get() 
> > if non-default methods are already added.
> 
> As I commented in JDK-8347826, I think the issue surely can be fixed in 
> different ways. Currently, all methods are put into a list, are sorted, and 
> then methods with the same signature are filtered out in 
> `Introspector.addMethod()` basing on the 

> From my point of view, the method list wasn't actually stable (after 
> JDK-8071693), because it didn't take into account if a method is default or 
> not, so `default` is located in the aorted list depending on a name of a 
> returned type, what causes a wrong method list processing in 
> `Introspector.addMethod()`, and exactly what I'm trying to fix here.

But the current sorting is good enouth to reproduced the bugs similar to the 
current one.

> > Is the next output expected()?
> > `public default void Test$A.setFoo(java.lang.Integer)`
> 
> It looks OK, it was properly chosen by `PropertyInfo` logic, or am I wrong? 
> If it's OK, I will add it to the test cases.

Then why the property is based on the implemented interface? For classes the 
method from the current class will be selected.
`public default void Test$A.setFoo(java.lang.Integer)` vs `public void 
Test$DC.setFoo(java.lang.Number)`

> As far as I see, `Parent3` actually has 2 methods falling into 
> `com.sun.beans.introspect.MethodInfo.get()`:
> 
>     * `java.beans.MethodDescriptor[name=getValue; method=public abstract 
> java.lang.Runnable Parent3.getValueX()] isSynthetic()=false`
> 
>     * `java.beans.MethodDescriptor[name=getValue; method=public default 
> java.lang.Object Parent3.getValueX()] isSynthetic()=true`
> 
> 
> While processing the `Parent3` class (in case we call 
> `Introspector.getBeanInfo(Parent3.class)`), the default synthetic method is 
> filtered out in `java.beans.Introspector.addMethod()` as synthetic. While 
> processing the derived `Child`, it takes the default method regardless if 
> it's synthetic or not, as JDK-8071693 implemented. However I'm not sure if 
> the synthetic `default java.lang.Object Parent3.getValueX()` method should 
> exist at all in the list. I think this issue is out of scope of this PR, and 
> doesn't relate to the proposed fix, so I'd suggest to process it separately.


But it is not filtered out for Child class:

import java.beans.*;
import java.lang.reflect.Method;
import java.util.Arrays;

interface Parent1<T> {
    T getValue();
}

interface Parent2 {
    Runnable getValue();
}

interface Parent3 extends Parent2, Parent1<Object> {
    Runnable getValue();
}

abstract class Child implements Parent3 {
    public void setValue(Runnable value) {
    }
}

public class BeansMainInt {
    public static void main(String[] args) throws Exception {
        BeanInfo beanInfo = Introspector.getBeanInfo(Child.class);
        for (MethodDescriptor methodDescriptor : 
beanInfo.getMethodDescriptors()) {
            System.out.println("methodDescriptor = " + methodDescriptor);
        }
        for (PropertyDescriptor pd : beanInfo.getPropertyDescriptors()) {
            Method writeMethod = pd.getWriteMethod();
            Method readMethod = pd.getReadMethod();
            System.out.println(pd.getName()+ ":\n ** " + pd.getPropertyType()
                    + "\n >> " + readMethod + "\n << " + writeMethod);
        }
    }
}

There is only one variant of getValue in the output(so both "property 
descriptor" and "method descriptor" are wrong):
`methodDescriptor = java.beans.MethodDescriptor[name=getValue; method=public 
default java.lang.Object Parent3.getValue()]
`
Or am I missing something?

It is related to this issue as it is part of 8071693, it seems it is not enouth 
to just add the list of default methods in the MethodInfo. you also need to 
"merge" different varants of the same method, select the correct one and filter 
out others.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/23443#issuecomment-2651557788

Reply via email to