BeanPropertySet doesn't find bean properties from interface default method getters & setters

[archiecobbs]

trafficstars

Description of the bug

BeanPropertySet is supposed to identify Java bean properties, which are defined by the existence of getter and setter methods.

However, BeanPropertySet fails to find getter and setter methods that are inherited as default methods from implemented interfaces.

If the methods are defined in an abstract superclass instead of an interface, everything works as it should... but there should be no difference in behavior in these two scenarios, as they are equally valid.

Expected behavior

BeanPropertySet should find inherited getter and setter methods no matter how they are inherited.

Minimal reproducible example

This test program demonstrates the problem:

import com.vaadin.flow.data.binder.*;
import java.util.regex.*;

public class BeanPropertySetTest {

    public interface HasName {

        default String getName() {
            return this.getLastName() + ", " + this.getFirstName();
        }
        default void setName(String name) {
            final Matcher matcher = Pattern.compile("^(.+), (.+)$").matcher(name);
            this.setLastName(matcher.group(1));
            this.setFirstName(matcher.group(2));
        }

        String getLastName();
        void setLastName(String lastName);

        String getFirstName();
        void setFirstName(String firstName);
    }   

    public class MyClass implements HasName {

        private String lastName;
        private String firstName;

        @Override
        public String getLastName() {
            return this.lastName;
        }
        @Override
        public void setLastName(String lastName) {
            this.lastName = lastName;
        }

        @Override
        public String getFirstName() {
            return this.firstName;
        }
        @Override
        public void setFirstName(String firstName) {
            this.firstName = firstName;
        }
    }
    
    public static void main(String[] args) {
        final PropertySet<MyClass> propertySet = BeanPropertySet.get(MyClass.class);
        for (String propertyName : new String[] { "lastName", "firstName", "name" }) {
            System.out.println(String.format("\"%s\" -> %s",
              propertyName, propertySet.getProperty(propertyName)));
        }
    }
}

There is a "name" property defined in the MyClass class, but BeanPropertySet doesn't find it.

As a result the test program above outputs this:

"lastName" -> Optional[com.vaadin.flow.data.binder.BeanPropertySet$BeanPropertyDefinition@312b1dae]
"firstName" -> Optional[com.vaadin.flow.data.binder.BeanPropertySet$BeanPropertyDefinition@7530d0a]
"name" -> Optional.empty

Versions

• Vaadin / Flow version: 24.3.2
• Java version: 17.0.9
• OS version: Mac OS Sonoma 14.2.1

[archiecobbs]

This is caused by a JDK bug JDK-8071693 in the Introspector class.

However this bug is not fixed until JDK 21. It would be nice for Vaadin to include a workaround for JDK < 21 if possible.

[mstahv]

But JDK 21 is already here 😎

Seriously, I think this is just one more reason to drop usage of the Introspector class altogether, like Jackson did it a decade ago.

Pros:

• Discovered properties would be in some sane order (Introspector use HashMap -> random order). This would greatly improve Vaadin as a RAD tool.
• We could get rid of requirement for java.desktop module -> Vaadin could be run with a smaller JRE
• We could get this fixed, with older JVMs

Cons:

• More "our code", unless somebody finds a good replacement library

I think I started a project to replace the Introspector class with an "clone" that was used by Android developers at some point for similar reasons. But The Jackson method is probably better. IIRC we use only a fraction of Introspector API.

[mstahv]

Random idea to consider: maybe the code in Jackson could be used directly? We already have that dependency and it is (and probably will be) actively maintained library.

[archiecobbs]

Using Jackson makes sense to me. Bean property detection is a core function that Vaadin relies on so it would be good to ensure that it's being "done right".

[mstahv]

Slightly related issue from V7/8 version: https://github.com/vaadin/framework/issues/8980

[mstahv]

@archiecobbs Do you have the problem with Grid or with some other component? If it is Grid, try the latest cut of Viritin's VGrid (in.virit:viritin:2.6.3, soon in Maven central), that now supports default methods. Prototyped using Jackson's underlaying bean introspection instead of what Vaadin core uses (the legacy Introspector class from java.desktop module). Also the bean properties are in more reasonable order and support for records 😎:

https://github.com/viritin/flow-viritin/blob/v24/src/test/java/org/vaadin/firitin/AdvancedBeanIntrospectionWithGrid.java

[archiecobbs]

Hi Matti,

Do you have the problem with Grid or with some other component?

It's with Grid (indirectly, see below). I've not used VGrid because in general we prefer to stick with standard Vaadin components. Of course, that preference is based on an assumption that standard Vaadin components are basically working like they should... ;)

By "indirectly" I mean @GridColumn, which is part of my own collection of Vaadin enhancements. FWIW let me know if Vaadin is interested in any of this stuff, I've found these very helpful over the years and I'd be happy to contribute...

• AsyncTaskManager and AsyncDataProvider
• @GridColumn
• FieldBuilder
• NullableField
• Etc.

[mstahv]

Well, Viritin components generally work as I think Vaadin components should work 🤣 Sometimes I'm able to promote some solutions to core though. I'd guess this (not using JDK Introspector) would be one (but with less hackish code).

Gotta check throught your enhancments some day, looks like your Viritin 😎 If there is something that you think would be ready for Vaadin core, don't hesitate to propose it (enhancment issue + PR).

[archiecobbs]

FYI besides Jackson Spring's BeanUtils is another good option for introspection. In the Javadoc it also references a few other options, e.g., Apache Commons BeanUtils.

[mstahv]

IIRC, both those store properties like Introspector as well (and would be new deps for core Vaadin).