Clear all bound fields when nulling the field group data source (#12371)

Change-Id: I48eb0e7851187fa6bbb9024730b2611a74501ad0

5 files changed, 198 insertions, 20 deletions

diff --git a/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java b/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java
index 1f3ee36d58..257a958f3a 100644
--- a/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java
+++ b/server/src/com/vaadin/data/fieldgroup/BeanFieldGroup.java
@@ -125,12 +125,19 @@ public class BeanFieldGroup<T> extends FieldGroup {
      * Helper method for setting the data source directly using a bean. This
      * method wraps the bean in a {@link BeanItem} and calls
      * {@link #setItemDataSource(Item)}.
+     * <p>
+     * For null values, a null item is passed to
+     * {@link #setItemDataSource(Item)} to be properly clear fields.
      * 
      * @param bean
      *            The bean to use as data source.
      */
     public void setItemDataSource(T bean) {
-        setItemDataSource(new BeanItem<T>(bean, beanType));
+        if (bean == null) {
+            setItemDataSource((Item) null);
+        } else {
+            setItemDataSource(new BeanItem<T>(bean, beanType));
+        }
     }
 
     @Override
diff --git a/server/src/com/vaadin/data/fieldgroup/FieldGroup.java b/server/src/com/vaadin/data/fieldgroup/FieldGroup.java
index bcfa8395df..c89b94ace9 100644
--- a/server/src/com/vaadin/data/fieldgroup/FieldGroup.java
+++ b/server/src/com/vaadin/data/fieldgroup/FieldGroup.java
@@ -26,7 +26,6 @@ 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;
@@ -256,6 +255,9 @@ public class FieldGroup implements Serializable {
         fieldToPropertyId.put(field, propertyId);
         propertyIdToField.put(propertyId, field);
         if (itemDataSource == null) {
+            // Clear any possible existing binding to clear the field
+            field.setPropertyDataSource(null);
+            field.clear();
             // Will be bound when data source is set
             return;
         }
@@ -461,7 +463,7 @@ public class FieldGroup implements Serializable {
 
             List<InvalidValueException> invalidValueExceptions = commitFields();
 
-            if(invalidValueExceptions.isEmpty()) {
+            if (invalidValueExceptions.isEmpty()) {
                 firePostCommitEvent();
                 commitTransactions();
             } else {
@@ -489,12 +491,14 @@ public class FieldGroup implements Serializable {
         return invalidValueExceptions;
     }
 
-    private void throwInvalidValueException(List<InvalidValueException> invalidValueExceptions) {
-        if(invalidValueExceptions.size() == 1) {
+    private void throwInvalidValueException(
+            List<InvalidValueException> invalidValueExceptions) {
+        if (invalidValueExceptions.size() == 1) {
             throw invalidValueExceptions.get(0);
         } else {
-            InvalidValueException[] causes = invalidValueExceptions.toArray(
-                    new InvalidValueException[invalidValueExceptions.size()]);
+            InvalidValueException[] causes = invalidValueExceptions
+                    .toArray(new InvalidValueException[invalidValueExceptions
+                            .size()]);
 
             throw new InvalidValueException(null, causes);
         }
@@ -515,8 +519,7 @@ public class FieldGroup implements Serializable {
 
     private void commitTransactions() {
         for (Field<?> f : fieldToPropertyId.keySet()) {
-            ((Property.Transactional<?>) f.getPropertyDataSource())
-                    .commit();
+            ((Property.Transactional<?>) f.getPropertyDataSource()).commit();
         }
     }
 
diff --git a/server/tests/src/com/vaadin/tests/server/component/fieldgroup/BeanFieldGroupTest.java b/server/tests/src/com/vaadin/tests/server/component/fieldgroup/BeanFieldGroupTest.java
index 965fb49479..9c37b91ef5 100644
--- a/server/tests/src/com/vaadin/tests/server/component/fieldgroup/BeanFieldGroupTest.java
+++ b/server/tests/src/com/vaadin/tests/server/component/fieldgroup/BeanFieldGroupTest.java
@@ -2,8 +2,6 @@ package com.vaadin.tests.server.component.fieldgroup;
 
 import static org.junit.Assert.assertEquals;
 
-import java.util.Collection;
-
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -144,15 +142,7 @@ public class BeanFieldGroupTest {
         group.setItemDataSource((MyBean) null);
 
         BeanItem<MyBean> dataSource = group.getItemDataSource();
-        Assert.assertNotNull("Data source is null for null bean", dataSource);
-
-        Collection<?> itemPropertyIds = dataSource.getItemPropertyIds();
-        Assert.assertEquals("Unexpected number of properties", 3,
-                itemPropertyIds.size());
-        for (Object id : itemPropertyIds) {
-            Assert.assertNull("Value for property " + id + " is not null",
-                    dataSource.getItemProperty(id).getValue());
-        }
+        Assert.assertNull("Data source is null for null bean", dataSource);
     }
 
     @Test
diff --git a/uitest/src/com/vaadin/tests/fieldgroup/BasicCrud.java b/uitest/src/com/vaadin/tests/fieldgroup/BasicCrud.java
new file mode 100644
index 0000000000..be0368f4ae
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/fieldgroup/BasicCrud.java
@@ -0,0 +1,110 @@
+/*
+ * Copyright 2000-2014 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.tests.fieldgroup;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.data.Property.ValueChangeEvent;
+import com.vaadin.data.Property.ValueChangeListener;
+import com.vaadin.data.fieldgroup.BeanFieldGroup;
+import com.vaadin.data.fieldgroup.PropertyId;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.util.Person;
+import com.vaadin.tests.util.PersonContainer;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.UI;
+import com.vaadin.ui.VerticalLayout;
+
+@Theme("valo")
+public class BasicCrud extends UI {
+
+    private Form form;
+
+    @Override
+    protected void init(VaadinRequest request) {
+        VerticalLayout main = new VerticalLayout();
+        main.setMargin(true);
+        main.setSpacing(true);
+        final Table t = new Table();
+        // t.setSelectionMode(SelectionMode.SINGLE);
+        t.setSelectable(true);
+
+        PersonContainer c = PersonContainer.createWithTestData();
+
+        c.addBean(new Person("first", "Last", "email", "phone", "street",
+                12332, "Turku"));
+        c.addBean(new Person("Foo", "Bar", "me@some.where", "123",
+                "homestreet 12", 10000, "Glasgow"));
+        t.setContainerDataSource(c);
+        // t.removeColumn("address");
+        // t.setColumnOrder("firstName", "lastName", "email", "phoneNumber",
+        // "address.streetAddress", "address.postalCode", "address.city");
+        t.setVisibleColumns("firstName", "lastName", "email", "phoneNumber",
+                "address.streetAddress", "address.postalCode", "address.city");
+
+        // t.addSelectionChangeListener(new SelectionChangeListener() {
+        // @Override
+        // public void selectionChange(SelectionChangeEvent event) {
+        // form.edit((Person) t.getSelectedRow());
+        // }
+        // });
+        t.addValueChangeListener(new ValueChangeListener() {
+
+            @Override
+            public void valueChange(ValueChangeEvent event) {
+                form.edit((Person) t.getValue());
+            }
+        });
+
+        form = new Form();
+
+        t.setSizeFull();
+
+        main.setSizeFull();
+        main.addComponent(t);
+        main.addComponent(form);
+        main.setExpandRatio(t, 1);
+        setContent(main);
+    }
+
+    public static class Form extends HorizontalLayout {
+        private TextField firstName = new TextField("First name");
+        private TextField lastName = new TextField("Last name");
+        private TextField email = new TextField("E-mail");
+        @PropertyId("address.streetAddress")
+        private TextField streetAddress = new TextField("Street address");
+        @PropertyId("address.postalCode")
+        private TextField postalCode = new TextField("Postal code");
+
+        BeanFieldGroup<Person> fieldGroup = new BeanFieldGroup<Person>(
+                Person.class);
+
+        public Form() {
+            setSpacing(true);
+            setId("form");
+            fieldGroup.bindMemberFields(this);
+
+            // Stupid integer binding
+            postalCode.setNullRepresentation("");
+            addComponents(firstName, lastName, email, streetAddress, postalCode);
+        }
+
+        public void edit(Person p) {
+            fieldGroup.setItemDataSource(p);
+        }
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/fieldgroup/BasicCrudTest.java b/uitest/src/com/vaadin/tests/fieldgroup/BasicCrudTest.java
new file mode 100644
index 0000000000..a41725dbc9
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/fieldgroup/BasicCrudTest.java
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2000-2014 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.tests.fieldgroup;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.AbstractOrderedLayoutElement;
+import com.vaadin.testbench.elements.TableElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class BasicCrudTest extends SingleBrowserTest {
+
+    @Test
+    public void fieldsInitiallyEmpty() {
+        openTestURL();
+        AbstractOrderedLayoutElement fieldsLayout = $(
+                AbstractOrderedLayoutElement.class).id("form");
+        List<TextFieldElement> textFields = fieldsLayout.$(
+                TextFieldElement.class).all();
+
+        for (TextFieldElement e : textFields) {
+            Assert.assertEquals("TextField should be empty", "", e.getValue());
+        }
+    }
+
+    @Test
+    public void fieldsClearedOnDeselect() {
+        openTestURL();
+        AbstractOrderedLayoutElement fieldsLayout = $(
+                AbstractOrderedLayoutElement.class).id("form");
+
+        // Select row
+        $(TableElement.class).first().getCell(2, 2).click();
+
+        List<TextFieldElement> textFields = fieldsLayout.$(
+                TextFieldElement.class).all();
+
+        for (TextFieldElement e : textFields) {
+            Assert.assertNotEquals("TextField should not be empty", "",
+                    e.getValue());
+        }
+
+        // Deselect row
+        $(TableElement.class).first().getCell(2, 2).click();
+
+        for (TextFieldElement e : textFields) {
+            Assert.assertEquals("TextField should be empty", "", e.getValue());
+        }
+
+    }
+}