Skip to content

Commit

Permalink
fix: unbind previous binding for the same field (#18833) (#18836)
Browse files Browse the repository at this point in the history
When a field is bound multiple times, the previous bindings are
removed from the Binder internal collection, but they are not
unboud, leaving listeners attached to the field.
This change unbounds previous bindings when the field is bound
again.

Fixes #18826
Fixes #18702

Co-authored-by: Marco Collovati <marco@vaadin.com>
  • Loading branch information
vaadin-bot and mcollovati committed Feb 29, 2024
1 parent aa04f95 commit 09380a5
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 4 deletions.
12 changes: 9 additions & 3 deletions flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Original file line number Diff line number Diff line change
Expand Up @@ -976,9 +976,15 @@ public Binding<BEAN, TARGET> bind(ValueProvider<BEAN, TARGET> getter,
this, getter, setter);

// Remove existing binding for same field to avoid potential
// multiple application of converter
getBinder().bindings.removeIf(
registeredBinding -> registeredBinding.getField() == field);
// multiple application of converter and value change listeners
List<Binding<BEAN, ?>> bindingsToRemove = getBinder().bindings
.stream().filter(registeredBinding -> registeredBinding
.getField() == field)
.toList();
if (!bindingsToRemove.isEmpty()) {
bindingsToRemove.forEach(Binding::unbind);
getBinder().bindings.removeAll(bindingsToRemove);
}
getBinder().bindings.add(binding);
if (getBinder().getBean() != null) {
binding.initFieldValue(getBinder().getBean(), true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import java.math.BigDecimal;
import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -65,8 +67,8 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

public class BinderTest extends BinderTestBase<Binder<Person>, Person> {

Expand Down Expand Up @@ -1319,6 +1321,48 @@ public void removed_binding_not_updates_value() {

}

@Test
public void replace_binding_previousBindingUnbound() {
List<String> bindingCalls = new ArrayList<>();
Binding<Person, Integer> binding1 = binder.forField(ageField)
.withConverter(new StringToIntegerConverter("Can't convert"))
.bind(p -> {
bindingCalls.add("READ FIRST");
return p.getAge();
}, (p, v) -> {
bindingCalls.add("WRITE FIRST");
p.setAge(v);
});

binder.setBean(item);
Assert.assertEquals(List.of("READ FIRST"), bindingCalls);

bindingCalls.clear();
ageField.setValue("99");
Assert.assertEquals(List.of("READ FIRST", "WRITE FIRST"), bindingCalls);

Binding<Person, Integer> binding2 = binder.forField(ageField)
.withConverter(new StringToIntegerConverter("Can't convert"))
.bind(p -> {
bindingCalls.add("READ SECOND");
return p.getAge();
}, (p, v) -> {
bindingCalls.add("WRITE SECOND");
p.setAge(v);
});

bindingCalls.clear();
ageField.setValue("33");
Assert.assertEquals(List.of("READ SECOND", "WRITE SECOND"),
bindingCalls);

assertNull("Expecting first binding to be unbound",
binding1.getField());
assertSame("Expecting second binding to be bound", ageField,
binding2.getField());

}

static class MyBindingHandler implements BindingValidationStatusHandler {

boolean expectingError = false;
Expand Down

0 comments on commit 09380a5

Please sign in to comment.