From 09380a506f6c45fad8401eecf721853bf340187a Mon Sep 17 00:00:00 2001
From: Vaadin Bot <vaadin-bot@users.noreply.github.com>
Date: Thu, 29 Feb 2024 13:37:55 +0100
Subject: [PATCH] fix: unbind previous binding for the same field (#18833)
 (#18836)

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>
---
 .../com/vaadin/flow/data/binder/Binder.java   | 12 +++--
 .../vaadin/flow/data/binder/BinderTest.java   | 46 ++++++++++++++++++-
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
index 072c0724e44..6df4131b07b 100644
--- a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
+++ b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
@@ -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);
diff --git a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java
index 0bdd9b8a4c3..296038aacfc 100644
--- a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java
+++ b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java
@@ -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;
@@ -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> {
 
@@ -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;