feat: remove validation status change listener upon unbind (#14111) (#…

…14128) Listener removal was missing when unbind was happening. Fixes #14110 Co-authored-by: Soroosh Taefi <[email protected]>
vaadin · Jul 1, 2022 · f0bb6d0 · f0bb6d0
1 parent 4b3fef5
commit f0bb6d0
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 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
@@ -971,12 +971,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;
         }
 
@@ -1219,6 +1213,8 @@ protected static class BindingImpl<BEAN, FIELDVALUE, TARGET>
 
         private boolean convertBackToPresentation = true;
 
+        private Registration onValidationStatusChange;
+
         public BindingImpl(BindingBuilderImpl<BEAN, FIELDVALUE, TARGET> builder,
                 ValueProvider<BEAN, TARGET> getter,
                 Setter<BEAN, TARGET> setter) {
@@ -1231,6 +1227,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;
@@ -1274,8 +1277,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() {
@@ -1284,6 +1289,11 @@ public void unbind() {
                 onValueChange = null;
             }
 
+            if (onValidationStatusChange != null) {
+                onValidationStatusChange.remove();
+                onValidationStatusChange = null;
+            }
+
             if (binder != null) {
                 binder.removeBindingInternal(this);
                 binder = null;

diff --git a/...a/src/test/java/com/vaadin/flow/data/binder/BinderValidationStatusChangeListenerTest.java b/...a/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));
+    }
+
 }