Add column add and remove functionality to Grid (#13334)

Change-Id: I967a71cafb85fcba8a708add2f5b0f5dc04d4c59

3 files changed, 164 insertions, 54 deletions

diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 5afb0038e0..75c1148382 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -41,6 +41,7 @@ import com.vaadin.data.Container.PropertySetChangeListener;
 import com.vaadin.data.Container.PropertySetChangeNotifier;
 import com.vaadin.data.Container.Sortable;
 import com.vaadin.data.Item;
+import com.vaadin.data.Property;
 import com.vaadin.data.RpcDataProviderExtension;
 import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper;
 import com.vaadin.data.fieldgroup.FieldGroup;
@@ -118,7 +119,7 @@ import elemental.json.JsonValue;
  * <p>
  * <code><pre>
  * Grid grid = new Grid(myContainer);
- * GridColumn column = grid.getColumn(STRING_DATE_PROPERTY);
+ * Column column = grid.getColumn(STRING_DATE_PROPERTY);
  * column.setConverter(new StringToDateConverter());
  * column.setRenderer(new MyColorfulDateRenderer());
  * </pre></code>
@@ -924,7 +925,7 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
         private Converter<?, Object> converter;
 
         /**
-         * A check for allowing the {@link #GridColumn(Grid, GridColumnState)
+         * A check for allowing the {@link #Column(Grid, GridColumnState)
          * constructor} to call {@link #setConverter(Converter)} with a
          * <code>null</code>, even if model and renderer aren't compatible.
          */
@@ -2211,12 +2212,6 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
             clearSortOrder();
         }
 
-        // Remove all old columns
-        Set<Object> properties = new HashSet<Object>(columns.keySet());
-        for (Object propertyId : properties) {
-            removeColumn(propertyId);
-        }
-
         datasourceExtension = new RpcDataProviderExtension(container);
         datasourceExtension.extend(this, columnKeys);
 
@@ -2241,15 +2236,17 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
 
         setLastFrozenPropertyId(null);
 
-        // Add columns
-        HeaderRow row = getHeader().getDefaultRow();
-        for (Object propertyId : datasource.getContainerPropertyIds()) {
-            Column column = appendColumn(propertyId);
+        if (columns.isEmpty()) {
+            // Add columns
+            for (Object propertyId : datasource.getContainerPropertyIds()) {
+                Column column = appendColumn(propertyId);
 
-            // Initial sorting is defined by container
-            if (datasource instanceof Sortable) {
-                column.setSortable(((Sortable) datasource)
-                        .getSortableContainerPropertyIds().contains(propertyId));
+                // Initial sorting is defined by container
+                if (datasource instanceof Sortable) {
+                    column.setSortable(((Sortable) datasource)
+                            .getSortableContainerPropertyIds().contains(
+                                    propertyId));
+                }
             }
         }
     }
@@ -2277,6 +2274,70 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
     /**
      * Used internally by the {@link Grid} to get a {@link Column} by
      * referencing its generated state id. Also used by {@link Column} to verify
+     * if it has been detached from the {@link Grid}. Adds a new Column to Grid.
+     * Also adds the property to container with data type String, if property
+     * for column does not exist in it. Default value for the new property is an
+     * empty String.
+     * 
+     * @param propertyId
+     *            the property id of the new column
+     * @return the new column
+     */
+    public Column addColumn(Object propertyId) {
+        addColumnProperty(propertyId, String.class, "");
+        return getColumn(propertyId);
+    }
+
+    /**
+     * Adds a new Column to Grid. Also adds the property to container with given
+     * Number data type, if property for column does not exist already in it.
+     * Default value for the new property is 0.
+     * 
+     * @param propertyId
+     *            the property id of the new column
+     * @return the new column
+     */
+    public <T extends Number> Column addColumn(Object propertyId, Class<T> type) {
+        addColumnProperty(propertyId, type, 0);
+        return getColumn(propertyId);
+    }
+
+    protected void addColumnProperty(Object propertyId, Class<?> type,
+            Object defaultValue) {
+        if (!columns.containsKey(propertyId)) {
+            if (!datasource.getContainerPropertyIds().contains(propertyId)) {
+                datasource.addContainerProperty(propertyId, type, defaultValue);
+            } else {
+                Property<?> containerProperty = datasource
+                        .getContainerProperty(datasource.firstItemId(),
+                                propertyId);
+                if (containerProperty.getType() == type) {
+                    appendColumn(propertyId);
+                } else {
+                    throw new IllegalStateException(
+                            "DataSource already has the given property "
+                                    + propertyId + " with a different type");
+                }
+            }
+        } else {
+            throw new IllegalStateException(
+                    "Grid already has a column for property " + propertyId);
+        }
+    }
+
+    /**
+     * Removes all columns from this Grid.
+     */
+    public void removeAllColumns() {
+        Set<Object> properties = new HashSet<Object>(columns.keySet());
+        for (Object propertyId : properties) {
+            removeColumn(propertyId);
+        }
+    }
+
+    /**
+     * Used internally by the {@link Grid} to get a {@link Column} by
+     * referencing its generated state id. Also used by {@link Column} to verify
      * if it has been detached from the {@link Grid}.
      * 
      * @param columnId
@@ -2344,10 +2405,10 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
     /**
      * Removes a column from Grid based on a property id.
      * 
-     * @param datasourcePropertyId
-     *            The property id of a property in the datasource
+     * @param propertyId
+     *            The property id of column to be removed
      */
-    private void removeColumn(Object propertyId) {
+    public void removeColumn(Object propertyId) {
         header.removeColumn(propertyId);
         footer.removeColumn(propertyId);
         Column column = columns.remove(propertyId);
diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/GridColumnBuildTest.java b/server/tests/src/com/vaadin/tests/server/component/grid/GridColumnBuildTest.java
new file mode 100644
index 0000000000..ca11ffe885
--- /dev/null
+++ b/server/tests/src/com/vaadin/tests/server/component/grid/GridColumnBuildTest.java
@@ -0,0 +1,83 @@
+/*
+ * 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.server.component.grid;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import com.vaadin.data.Container;
+import com.vaadin.data.Property;
+import com.vaadin.ui.Grid;
+
+public class GridColumnBuildTest {
+
+    Grid grid = new Grid();
+    Container.Indexed container;
+
+    @Before
+    public void setUp() {
+        container = grid.getContainerDatasource();
+        container.addItem();
+    }
+
+    @Test
+    public void testAddColumn() {
+        grid.addColumn("foo");
+
+        Property<?> property = container.getContainerProperty(
+                container.firstItemId(), "foo");
+        assertEquals(property.getType(), String.class);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testAddColumnTwice() {
+        grid.addColumn("foo");
+        grid.addColumn("foo");
+    }
+
+    @Test
+    public void testAddRemoveAndAddAgainColumn() {
+        grid.addColumn("foo");
+        grid.removeColumn("foo");
+
+        // Removing a column, doesn't remove the property
+        Property<?> property = container.getContainerProperty(
+                container.firstItemId(), "foo");
+        assertEquals(property.getType(), String.class);
+        grid.addColumn("foo");
+    }
+
+    public void testAddNumberColumns() {
+        grid.addColumn("bar", Integer.class);
+        grid.addColumn("baz", Double.class);
+
+        Property<?> property = container.getContainerProperty(
+                container.firstItemId(), "bar");
+        assertEquals(property.getType(), Integer.class);
+        property = container.getContainerProperty(container.firstItemId(),
+                "baz");
+        assertEquals(property.getType(), Double.class);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testAddDifferentTypeColumn() {
+        grid.addColumn("foo");
+        grid.removeColumn("foo");
+        grid.addColumn("foo", Integer.class);
+    }
+}
diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java b/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java
index a7857bb4ff..1c624b0866 100644
--- a/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java
+++ b/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java
@@ -30,7 +30,6 @@ import java.util.Set;
 import org.junit.Before;
 import org.junit.Test;
 
-import com.vaadin.data.util.GeneratedPropertyContainer;
 import com.vaadin.data.util.IndexedContainer;
 import com.vaadin.server.KeyMapper;
 import com.vaadin.shared.ui.grid.GridColumnState;
@@ -51,6 +50,7 @@ public class GridColumns {
     private KeyMapper<Object> columnIdMapper;
 
     @Before
+    @SuppressWarnings("unchecked")
     public void setup() throws Exception {
         IndexedContainer ds = new IndexedContainer();
         for (int c = 0; c < 10; c++) {
@@ -86,40 +86,6 @@ public class GridColumns {
     }
 
     @Test
-    public void testColumnGenerationOnContainerChange() {
-        int colCount = state.columns.size();
-        assertEquals(colCount, state.columnOrder.size());
-
-        // Change old columns so they wouldn't pass the check.
-        for (Object propertyId : grid.getContainerDatasource()
-                .getContainerPropertyIds()) {
-            Column column = grid.getColumn(propertyId);
-            column.setHeaderCaption("Old " + column.getHeaderCaption());
-        }
-
-        // creates a new "view" of the existing container
-        grid.setContainerDataSource(new GeneratedPropertyContainer(grid
-                .getContainerDatasource()));
-
-        // Should still contain the same amount of columns
-        assertEquals(colCount, state.columns.size());
-        assertEquals(colCount, state.columnOrder.size());
-
-        int i = 0;
-        for (Object propertyId : grid.getContainerDatasource()
-                .getContainerPropertyIds()) {
-
-            // All property ids should get a column
-            Column column = grid.getColumn(propertyId);
-            assertNotNull(column);
-
-            // Property id should be the column header by default
-            assertEquals(propertyId.toString(), grid.getDefaultHeaderRow()
-                    .getCell(propertyId).getText());
-        }
-    }
-
-    @Test
     public void testModifyingColumnProperties() throws Exception {
 
         // Modify first column