Returning all validation errors in the exception cause when submitting a field group (#14742)

Following the similar logic as in AbstractField when multiple validation errors occur. Catching all InvalidValueExceptions and putting them together wrapped into the exception.

Change-Id: Ied08fd2155412539b28ef94bc74e6c989c62f709

server/src/com/vaadin/data/fieldgroup/FieldGroup.java

@@ -26,6 +26,7 @@ import java.util.List;
 
 import com.vaadin.data.Item;
 import com.vaadin.data.Property;
+import com.vaadin.data.Validator;
 import com.vaadin.data.Validator.InvalidValueException;
 import com.vaadin.data.util.TransactionalPropertyWrapper;
 import com.vaadin.ui.AbstractField;
@@ -452,6 +453,54 @@ public class FieldGroup implements Serializable {
             // Not using buffered mode, nothing to do
             return;
         }
+
+        startTransactions();
+
+        try {
+            firePreCommitEvent();
+
+            List<InvalidValueException> invalidValueExceptions = commitFields();
+
+            if(invalidValueExceptions.isEmpty()) {
+                firePostCommitEvent();
+                commitTransactions();
+            } else {
+                throwInvalidValueException(invalidValueExceptions);
+            }
+        } catch (Exception e) {
+            rollbackTransactions();
+
+            throw new CommitException("Commit failed", e);
+        }
+
+    }
+
+    private List<InvalidValueException> commitFields() {
+        List<InvalidValueException> invalidValueExceptions = new ArrayList<InvalidValueException>();
+
+        for (Field<?> f : fieldToPropertyId.keySet()) {
+            try {
+                f.commit();
+            } catch (InvalidValueException e) {
+                invalidValueExceptions.add(e);
+            }
+        }
+
+        return invalidValueExceptions;
+    }
+
+    private void throwInvalidValueException(List<InvalidValueException> invalidValueExceptions) {
+        if(invalidValueExceptions.size() == 1) {
+            throw invalidValueExceptions.get(0);
+        } else {
+            InvalidValueException[] causes = invalidValueExceptions.toArray(
+                    new InvalidValueException[invalidValueExceptions.size()]);
+
+            throw new InvalidValueException(null, causes);
+        }
+    }
+
+    private void startTransactions() throws CommitException {
         for (Field<?> f : fieldToPropertyId.keySet()) {
             Property.Transactional<?> property = (Property.Transactional<?>) f
                     .getPropertyDataSource();
@@ -462,33 +511,24 @@ public class FieldGroup implements Serializable {
             }
             property.startTransaction();
         }
-        try {
-            firePreCommitEvent();
-            // Commit the field values to the properties
-            for (Field<?> f : fieldToPropertyId.keySet()) {
-                f.commit();
-            }
-            firePostCommitEvent();
+    }
 
-            // Commit the properties
-            for (Field<?> f : fieldToPropertyId.keySet()) {
-                ((Property.Transactional<?>) f.getPropertyDataSource())
-                        .commit();
-            }
+    private void commitTransactions() {
+        for (Field<?> f : fieldToPropertyId.keySet()) {
+            ((Property.Transactional<?>) f.getPropertyDataSource())
+                    .commit();
+        }
+    }
 
-        } catch (Exception e) {
-            for (Field<?> f : fieldToPropertyId.keySet()) {
-                try {
-                    ((Property.Transactional<?>) f.getPropertyDataSource())
-                            .rollback();
-                } catch (Exception rollbackException) {
-                    // FIXME: What to do ?
-                }
+    private void rollbackTransactions() {
+        for (Field<?> f : fieldToPropertyId.keySet()) {
+            try {
+                ((Property.Transactional<?>) f.getPropertyDataSource())
+                        .rollback();
+            } catch (Exception rollbackException) {
+                // FIXME: What to do ?
             }
-
-            throw new CommitException("Commit failed", e);
         }
-
     }
 
     /**

uitest/ivy.xml

@@ -27,6 +27,8 @@
 
         <dependency org="javax.validation" name="validation-api"
             rev="1.0.0.GA" conf="build,ide -> default,sources" />
+        <dependency org="org.hibernate" name="hibernate-validator"
+                    rev="4.2.0.Final" conf="build,ide -> default" />
         <!-- Google App Engine -->
         <dependency org="com.google.appengine" name="appengine-api-1.0-sdk"
             rev="1.2.1" conf="build-provided,ide -> default" />

uitest/src/com/vaadin/tests/fieldgroup/MultipleValidationErrors.java

@@ -0,0 +1,72 @@
+package com.vaadin.tests.fieldgroup;
+
+import com.vaadin.data.Validator;
+import com.vaadin.data.fieldgroup.FieldGroup;
+import com.vaadin.data.util.BeanItem;
+import com.vaadin.data.validator.BeanValidator;
+import com.vaadin.server.AbstractErrorMessage;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TextField;
+import org.apache.commons.lang.StringEscapeUtils;
+
+public class MultipleValidationErrors extends AbstractTestUI {
+
+    public static final String FIRST_NAME_NOT_NULL_VALIDATION_MESSAGE = "first name is null";
+    public static final String LAST_NAME_NOT_NULL_VALIDATION_MESSAGE = "last name is null";
+    public static final String FIRST_NAME_NOT_EMPTY_VALIDATION_MESSAGE = "first name is empty";
+    public static final String LAST_NAME_NOT_EMPTY_VALIDATION_MESSAGE = "last name is empty";
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        BeanItem<PersonBeanWithValidationAnnotations> item = new BeanItem<PersonBeanWithValidationAnnotations>(
+                new PersonBeanWithValidationAnnotations());
+        final FieldGroup fieldGroup = new FieldGroup(item);
+
+        bindTextField(item, fieldGroup, "First Name", "firstName");
+        bindTextField(item, fieldGroup, "Last Name", "lastName");
+
+        final Label validationErrors = new Label();
+        validationErrors.setId("validationErrors");
+        addComponent(validationErrors);
+
+        addButton("Submit", new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                validationErrors.setValue("");
+                try {
+                    fieldGroup.commit();
+                } catch (FieldGroup.CommitException e) {
+                    if (e.getCause() != null && e.getCause() instanceof Validator.InvalidValueException) {
+                        validationErrors.setValue(StringEscapeUtils.unescapeHtml(
+                            AbstractErrorMessage.getErrorMessageForException(e.getCause()).getFormattedHtmlMessage()));
+                    }
+                }
+
+
+            }
+        });
+    }
+
+    private void bindTextField(BeanItem<PersonBeanWithValidationAnnotations> item, FieldGroup fieldGroup,
+                               String caption, String propertyId) {
+        TextField textfield = new TextField(caption, item.getItemProperty(propertyId));
+        textfield.addValidator(new BeanValidator(PersonBeanWithValidationAnnotations.class, propertyId));
+
+        fieldGroup.bind(textfield, propertyId);
+
+        addComponent(textfield);
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 14742;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "All validation errors should be included when committing a field group.";
+    }
+}

uitest/src/com/vaadin/tests/fieldgroup/MultipleValidationErrorsTest.java

@@ -0,0 +1,37 @@
+package com.vaadin.tests.fieldgroup;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class MultipleValidationErrorsTest extends MultiBrowserTest {
+
+    private void commitTextFields() {
+        $(ButtonElement.class).caption("Submit").first().click();
+    }
+
+    private void clearTextField(String caption) {
+        TextFieldElement textField = $(TextFieldElement.class).caption(caption).first();
+        textField.clear();
+    }
+
+    @Test
+    public void validationErrorsIncludeBothErrors() {
+        openTestURL();
+
+        clearTextField("First Name");
+        clearTextField("Last Name");
+
+        commitTextFields();
+
+        String validationErrors = $(LabelElement.class).id("validationErrors").getText();
+
+        assertThat(validationErrors, containsString(MultipleValidationErrors.FIRST_NAME_NOT_EMPTY_VALIDATION_MESSAGE));
+        assertThat(validationErrors, containsString(MultipleValidationErrors.LAST_NAME_NOT_EMPTY_VALIDATION_MESSAGE));
+    }
+}

uitest/src/com/vaadin/tests/fieldgroup/PersonBeanWithValidationAnnotations.java

@@ -0,0 +1,31 @@
+package com.vaadin.tests.fieldgroup;
+
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
+import java.io.Serializable;
+
+public class PersonBeanWithValidationAnnotations implements Serializable {
+    @NotNull(message = MultipleValidationErrors.FIRST_NAME_NOT_NULL_VALIDATION_MESSAGE)
+    @Size(min = 1, message = MultipleValidationErrors.FIRST_NAME_NOT_EMPTY_VALIDATION_MESSAGE)
+    private String firstName;
+
+    @NotNull(message = MultipleValidationErrors.LAST_NAME_NOT_NULL_VALIDATION_MESSAGE)
+    @Size(min = 1, message = MultipleValidationErrors.LAST_NAME_NOT_EMPTY_VALIDATION_MESSAGE)
+    private String lastName;
+
+    public String getFirstName() {
+        return firstName;
+    }
+
+    public void setFirstName(String firstName) {
+        this.firstName = firstName;
+    }
+
+    public String getLastName() {
+        return lastName;
+    }
+
+    public void setLastName(String lastName) {
+        this.lastName = lastName;
+    }
+}