Make sure bean field validators are only added once (#11045)

Change-Id: I67779fa5bfd4c850101c11c22091c988eb65b808

2 files changed, 59 insertions, 1 deletions

diff --git a/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java b/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java
index 0b4e3a8049..cb8a044f00 100644
--- a/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java
+++ b/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java
@@ -16,6 +16,8 @@
 package com.vaadin.data.fieldgroup;
 
 import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.Map;
 
 import com.vaadin.data.Item;
 import com.vaadin.data.util.BeanItem;
@@ -27,9 +29,11 @@ public class BeanFieldGroup<T> extends FieldGroup {
     private Class<T> beanType;
 
     private static Boolean beanValidationImplementationAvailable = null;
+    private final Map<Field<?>, BeanValidator> defaultValidators;
 
     public BeanFieldGroup(Class<T> beanType) {
         this.beanType = beanType;
+        this.defaultValidators = new HashMap<Field<?>, BeanValidator>();
     }
 
     @Override
@@ -171,16 +175,28 @@ public class BeanFieldGroup<T> extends FieldGroup {
     }
 
     @Override
+    public void unbind(Field<?> field) throws BindException {
+        super.unbind(field);
+
+        BeanValidator removed = defaultValidators.remove(field);
+        if (removed != null) {
+            field.removeValidator(removed);
+        }
+    }
+
+    @Override
     protected void configureField(Field<?> field) {
         super.configureField(field);
         // Add Bean validators if there are annotations
-        if (isBeanValidationImplementationAvailable()) {
+        if (isBeanValidationImplementationAvailable()
+                && !defaultValidators.containsKey(field)) {
             BeanValidator validator = new BeanValidator(beanType,
                     getPropertyId(field).toString());
             field.addValidator(validator);
             if (field.getLocale() != null) {
                 validator.setLocale(field.getLocale());
             }
+            defaultValidators.put(field, validator);
         }
     }
 
diff --git a/server/tests/src/com/vaadin/tests/server/validation/TestBeanValidation.java b/server/tests/src/com/vaadin/tests/server/validation/TestBeanValidation.java
index 8f6928fc35..e1d08a989b 100644
--- a/server/tests/src/com/vaadin/tests/server/validation/TestBeanValidation.java
+++ b/server/tests/src/com/vaadin/tests/server/validation/TestBeanValidation.java
@@ -1,10 +1,14 @@
 package com.vaadin.tests.server.validation;
 
+import junit.framework.Assert;
+
 import org.junit.Test;
 
 import com.vaadin.data.Validator.InvalidValueException;
+import com.vaadin.data.fieldgroup.BeanFieldGroup;
 import com.vaadin.data.validator.BeanValidator;
 import com.vaadin.tests.data.bean.BeanToValidate;
+import com.vaadin.ui.Field;
 
 public class TestBeanValidation {
     @Test(expected = InvalidValueException.class)
@@ -54,4 +58,42 @@ public class TestBeanValidation {
         validator.validate("123.45");
     }
 
+    @Test
+    public void testBeanValidationNotAddedTwice() {
+        // See ticket #11045
+        BeanFieldGroup<BeanToValidate> fieldGroup = new BeanFieldGroup<BeanToValidate>(
+                BeanToValidate.class);
+
+        BeanToValidate beanToValidate = new BeanToValidate();
+        beanToValidate.setFirstname("a");
+        fieldGroup.setItemDataSource(beanToValidate);
+
+        Field<?> nameField = fieldGroup.buildAndBind("firstname");
+        Assert.assertEquals(1, nameField.getValidators().size());
+
+        try {
+            nameField.validate();
+        } catch (InvalidValueException e) {
+            // NOTE: causes are empty if only one validation fails
+            Assert.assertEquals(0, e.getCauses().length);
+        }
+
+        // Create new, identical bean to cause duplicate validator unless #11045
+        // is fixed
+        beanToValidate = new BeanToValidate();
+        beanToValidate.setFirstname("a");
+        fieldGroup.setItemDataSource(beanToValidate);
+
+        Assert.assertEquals(1, nameField.getValidators().size());
+
+        try {
+            nameField.validate();
+        } catch (InvalidValueException e) {
+            // NOTE: if more than one validation fails, we get the number of
+            // failed validations
+            Assert.assertEquals(0, e.getCauses().length);
+        }
+
+    }
+
 }