From 425c2357d5619c23fb2d4631d5e68d83be8aa977 Mon Sep 17 00:00:00 2001
From: Soroosh Taefi <taefi.soroosh@gmail.com>
Date: Fri, 1 Jul 2022 18:25:03 +0300
Subject: [PATCH] feat: remove validation status change listener upon unbind
 (#14111)

Listener removal was missing when unbind
was happening.

Fixes #14110
---
 .../com/vaadin/flow/data/binder/Binder.java   | 26 +++++++++-----
 ...derValidationStatusChangeListenerTest.java | 36 +++++++++++++++++++
 2 files changed, 54 insertions(+), 8 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 1e14c3faa17..27d235af172 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
@@ -882,12 +882,6 @@ public Binding<BEAN, TARGET> bind(ValueProvider<BEAN, TARGET> getter,
             }
             this.binding = binding;
 
-            if (field instanceof HasValidator) {
-                HasValidator<FIELDVALUE> hasValidatorField = (HasValidator<FIELDVALUE>) field;
-                hasValidatorField.addValidationStatusChangeListener(
-                        event -> this.binding.validate());
-            }
-
             return binding;
         }
 
@@ -1113,6 +1107,8 @@ protected static class BindingImpl<BEAN, FIELDVALUE, TARGET>
 
         private boolean convertBackToPresentation = false;
 
+        private Registration onValidationStatusChange;
+
         public BindingImpl(BindingBuilderImpl<BEAN, FIELDVALUE, TARGET> builder,
                 ValueProvider<BEAN, TARGET> getter,
                 Setter<BEAN, TARGET> setter) {
@@ -1125,6 +1121,13 @@ public BindingImpl(BindingBuilderImpl<BEAN, FIELDVALUE, TARGET> builder,
             onValueChange = getField().addValueChangeListener(
                     event -> handleFieldValueChange(event));
 
+            if (getField() instanceof HasValidator) {
+                HasValidator<FIELDVALUE> hasValidatorField = (HasValidator<FIELDVALUE>) getField();
+                onValidationStatusChange = hasValidatorField
+                        .addValidationStatusChangeListener(
+                                event -> this.validate());
+            }
+
             this.getter = getter;
             this.setter = setter;
             readOnly = setter == null;
@@ -1168,8 +1171,10 @@ public BindingValidationStatus<TARGET> validate(boolean fireEvent) {
 
         /**
          * Removes this binding from its binder and unregisters the
-         * {@code ValueChangeListener} from any bound {@code HasValue}. It does
-         * nothing if it is called for an already unbound binding.
+         * {@code ValueChangeListener} from any bound {@code HasValue}, and
+         * {@code ValidationStatusChangeListener} from any bound
+         * {@code HasValidator}. It does nothing if it is called for an already
+         * unbound binding.
          */
         @Override
         public void unbind() {
@@ -1178,6 +1183,11 @@ public void unbind() {
                 onValueChange = null;
             }
 
+            if (onValidationStatusChange != null) {
+                onValidationStatusChange.remove();
+                onValidationStatusChange = null;
+            }
+
             if (binder != null) {
                 binder.removeBindingInternal(this);
                 binder = null;
diff --git a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderValidationStatusChangeListenerTest.java b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderValidationStatusChangeListenerTest.java
index cdb949bda78..d130af2d782 100644
--- a/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderValidationStatusChangeListenerTest.java
+++ b/flow-data/src/test/java/com/vaadin/flow/data/binder/BinderValidationStatusChangeListenerTest.java
@@ -1,5 +1,21 @@
+/*
+ * Copyright 2000-2022 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
 package com.vaadin.flow.data.binder;
 
+import java.time.LocalDate;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -127,4 +143,24 @@ public void fieldWithHasValidatorOnlyAddListenerOverriddenAndCustomValidation_fi
         Assert.assertNull(componentErrors.get(field));
     }
 
+    @Test
+    public void fieldWithHasValidatorFullyOverridden_boundFieldGetsUnbind_validationStatusChangeListenerInBindingIsRemoved() {
+        TestHasValidatorDatePicker.DataPickerHasValidatorOverridden field = new TestHasValidatorDatePicker.DataPickerHasValidatorOverridden();
+        Binder.Binding<Person, LocalDate> binding = binder.bind(field,
+                BIRTH_DATE_PROPERTY);
+        Assert.assertEquals(0, componentErrors.size());
+
+        field.fireValidationStatusChangeEvent(false);
+        Assert.assertEquals(1, componentErrors.size());
+        Assert.assertEquals(INVALID_DATE_FORMAT, componentErrors.get(field));
+
+        binding.unbind();
+
+        field.fireValidationStatusChangeEvent(true);
+        // after unbind is called, validationStatusChangeListener
+        // in the binding is not working anymore, errors are not cleared:
+        Assert.assertEquals(1, componentErrors.size());
+        Assert.assertEquals(INVALID_DATE_FORMAT, componentErrors.get(field));
+    }
+
 }