Skip to content

Commit 963a7f8

Browse files
vaadin-botArtur-
andauthored
fix: merge property descriptors when getter and setter come from different interfaces (#23939) (#23942)
BeanUtil's dedup loop was replacing a read-only descriptor with a write-only one (or vice versa) instead of merging them. This caused BeanPropertySet to lose the getter when a parent interface provided it and a child interface provided the setter. Fixes #20814 Co-authored-by: Artur Signell <artur@vaadin.com>
1 parent af21ee3 commit 963a7f8

File tree

3 files changed

+109
-0
lines changed

3 files changed

+109
-0
lines changed

flow-data/src/test/java/com/vaadin/flow/data/binder/BeanPropertySetTest.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.HashSet;
2828
import java.util.List;
2929
import java.util.Map;
30+
import java.util.Optional;
3031
import java.util.Set;
3132
import java.util.regex.Matcher;
3233
import java.util.regex.Pattern;
@@ -467,6 +468,72 @@ public void setFirstName(String firstName) {
467468
}
468469
}
469470

471+
public interface Measurement {
472+
double getValue();
473+
}
474+
475+
public interface MutableMeasurement extends Measurement {
476+
void setValue(double value);
477+
}
478+
479+
@Test
480+
public void propertyWithGetterAndSetterFromSeparateInterfaces() {
481+
// When getter is in a parent interface and setter in a sub-interface,
482+
// the property should be discovered with both accessors.
483+
// This tests the interface type directly, as used e.g. in
484+
// Grid<MutableMeasurement>.
485+
PropertySet<MutableMeasurement> set = BeanPropertySet
486+
.get(MutableMeasurement.class);
487+
488+
Assert.assertTrue(
489+
"Property 'value' should be discovered when getter is in "
490+
+ "a parent interface and setter in a sub-interface",
491+
set.getProperty("value").isPresent());
492+
PropertyDefinition<MutableMeasurement, ?> def = set.getProperty("value")
493+
.get();
494+
Assert.assertNotNull(def.getGetter());
495+
Assert.assertTrue(def.getSetter().isPresent());
496+
}
497+
498+
// Reproducer from https://github.com/vaadin/flow/issues/20814
499+
interface BaseInfo {
500+
String getInfo();
501+
}
502+
503+
interface ExtendedInfo extends BaseInfo {
504+
void setInfo(String value);
505+
}
506+
507+
interface ExtendedInfoWithOverride extends BaseInfo {
508+
@Override
509+
String getInfo();
510+
511+
void setInfo(String value);
512+
}
513+
514+
@Test
515+
public void setterInChildInterfaceWithoutGetterOverride() {
516+
PropertySet<ExtendedInfo> set = BeanPropertySet.get(ExtendedInfo.class);
517+
518+
Optional<PropertyDefinition<ExtendedInfo, ?>> info = set
519+
.getProperty("info");
520+
Assert.assertTrue(info.isPresent());
521+
Assert.assertNotNull(info.get().getGetter());
522+
Assert.assertTrue(info.get().getSetter().isPresent());
523+
}
524+
525+
@Test
526+
public void setterInChildInterfaceWithGetterOverride() {
527+
PropertySet<ExtendedInfoWithOverride> set = BeanPropertySet
528+
.get(ExtendedInfoWithOverride.class);
529+
530+
Optional<PropertyDefinition<ExtendedInfoWithOverride, ?>> info = set
531+
.getProperty("info");
532+
Assert.assertTrue(info.isPresent());
533+
Assert.assertNotNull(info.get().getGetter());
534+
Assert.assertTrue(info.get().getSetter().isPresent());
535+
}
536+
470537
@Test
471538
public void includesDefaultMethodsFromInterfaces() {
472539
PropertySet<MyClass> set = BeanPropertySet.get(MyClass.class);

flow-server/src/main/java/com/vaadin/flow/internal/BeanUtil.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,31 @@ public static List<PropertyDescriptor> getBeanPropertyDescriptors(
8787
.getReadMethod().getDeclaringClass().isInterface()) {
8888
continue;
8989
}
90+
// Merge read/write methods from separate descriptors
91+
descriptor = mergeDescriptors(existing, descriptor);
9092
}
9193
descriptors.put(name, descriptor);
9294
}
9395

9496
return new ArrayList<>(descriptors.values());
9597
}
9698

99+
private static PropertyDescriptor mergeDescriptors(
100+
PropertyDescriptor existing, PropertyDescriptor incoming) {
101+
Method read = incoming.getReadMethod() != null
102+
? incoming.getReadMethod()
103+
: existing.getReadMethod();
104+
Method write = incoming.getWriteMethod() != null
105+
? incoming.getWriteMethod()
106+
: existing.getWriteMethod();
107+
try {
108+
return new PropertyDescriptor(incoming.getName(), read, write);
109+
} catch (IntrospectionException e) {
110+
// Fall back to incoming if merge fails
111+
return incoming;
112+
}
113+
}
114+
97115
private static List<PropertyDescriptor> internalGetBeanPropertyDescriptors(
98116
Class<?> beanType) throws IntrospectionException {
99117

flow-server/src/test/java/com/vaadin/flow/internal/BeanUtilTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.vaadin.flow.internal;
22

3+
import java.beans.IntrospectionException;
34
import java.beans.PropertyDescriptor;
45
import java.util.List;
56

@@ -73,4 +74,27 @@ public void duplicatesAreRemoved() throws Exception {
7374
oneInterfaceProperty.getReadMethod().getDeclaringClass());
7475

7576
}
77+
78+
public interface ReadOnlyMeasurement {
79+
double getValue();
80+
}
81+
82+
public interface WritableMeasurement extends ReadOnlyMeasurement {
83+
void setValue(double value);
84+
}
85+
86+
@Test
87+
public void getterAndSetterFromSeparateInterfacesAreMerged()
88+
throws IntrospectionException {
89+
List<PropertyDescriptor> descriptors = BeanUtil
90+
.getBeanPropertyDescriptors(WritableMeasurement.class);
91+
92+
PropertyDescriptor valueDescriptor = descriptors.stream()
93+
.filter(d -> "value".equals(d.getName())).findFirst()
94+
.orElse(null);
95+
96+
Assert.assertNotNull("Should find 'value' property", valueDescriptor);
97+
Assert.assertNotNull("Should have read method from parent interface", valueDescriptor.getReadMethod());
98+
Assert.assertNotNull("Should have write method from child interface", valueDescriptor.getWriteMethod());
99+
}
76100
}

0 commit comments

Comments
 (0)