From 11f8811eb0f7a306c403d36ab45f5d8658d7a164 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Patrik=20Lindstr=C3=B6m?= <patrik@vaadin.com>
Date: Wed, 31 Jul 2013 16:42:16 +0300
Subject: [PATCH] Make sure bean field validators are only added once (#11045)

Change-Id: I67779fa5bfd4c850101c11c22091c988eb65b808
---
 .../data/fieldgroup/BeanFieldGroup.java       | 18 +++++++-
 .../server/validation/TestBeanValidation.java | 42 +++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

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
@@ -170,17 +174,29 @@ public class BeanFieldGroup<T> extends FieldGroup {
         return super.buildAndBind(caption, propertyId);
     }
 
+    @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);
+        }
+
+    }
+
 }
-- 
2.39.5