Read/write Grid item type to declarative and create columns correctly (#8769)

Fixes #8467

server/src/main/java/com/vaadin/ui/Grid.java

@@ -72,6 +72,7 @@ import com.vaadin.server.SerializableComparator;
 import com.vaadin.server.SerializableFunction;
 import com.vaadin.server.SerializableSupplier;
 import com.vaadin.server.Setter;
+import com.vaadin.server.VaadinServiceClassLoaderUtil;
 import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.Registration;
 import com.vaadin.shared.data.DataCommunicatorConstants;
@@ -135,6 +136,8 @@ import elemental.json.JsonValue;
 public class Grid<T> extends AbstractListing<T> implements HasComponents,
         HasDataProvider<T>, SortNotifier<GridSortOrder<T>> {
 
+    private static final String DECLARATIVE_DATA_ITEM_TYPE = "data-item-type";
+
     /**
      * A callback method for fetching items. The callback is provided with a
      * list of sort orders, offset index and limit.
@@ -2038,7 +2041,9 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
     private Editor<T> editor;
 
-    private final PropertySet<T> propertySet;
+    private PropertySet<T> propertySet;
+
+    private Class<T> beanType = null;
 
     /**
      * Creates a new grid without support for creating columns based on property
@@ -2079,6 +2084,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
      */
     public Grid(Class<T> beanType) {
         this(BeanPropertySet.get(beanType));
+        this.beanType = beanType;
     }
 
     /**
@@ -2093,24 +2099,14 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
      *            the property set implementation to use, not <code>null</code>.
      */
     protected Grid(PropertySet<T> propertySet) {
-        Objects.requireNonNull(propertySet, "propertySet cannot be null");
-        this.propertySet = propertySet;
-
         registerRpc(new GridServerRpcImpl());
-
         setDefaultHeaderRow(appendHeaderRow());
-
         setSelectionModel(new SingleSelectionModelImpl<>());
 
         detailsManager = new DetailsManager<>();
         addExtension(detailsManager);
         addDataGenerator(detailsManager);
 
-        editor = createEditor();
-        if (editor instanceof Extension) {
-            addExtension((Extension) editor);
-        }
-
         addDataGenerator((item, json) -> {
             String styleName = styleGenerator.apply(item);
             if (styleName != null && !styleName.isEmpty()) {
@@ -2124,11 +2120,37 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             }
         });
 
+        setPropertySet(propertySet);
+
         // Automatically add columns for all available properties
         propertySet.getProperties().map(PropertyDefinition::getName)
                 .forEach(this::addColumn);
     }
 
+    /**
+     * Sets the property set to use for this grid. Does not create or update
+     * columns in any way but will delete and re-create the editor.
+     * <p>
+     * This is only meant to be called from constructors and readDesign, at a
+     * stage where it does not matter if you throw away the editor.
+     *
+     * @param propertySet
+     *            the property set to use
+     */
+    protected void setPropertySet(PropertySet<T> propertySet) {
+        Objects.requireNonNull(propertySet, "propertySet cannot be null");
+        this.propertySet = propertySet;
+
+        if (editor instanceof Extension) {
+            removeExtension((Extension) editor);
+        }
+        editor = createEditor();
+        if (editor instanceof Extension) {
+            addExtension((Extension) editor);
+        }
+
+    }
+
     /**
      * Creates a grid using a custom {@link PropertySet} implementation for
      * creating a default set of columns and for resolving property names with
@@ -2201,6 +2223,19 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         this(caption, DataProvider.ofCollection(items));
     }
 
+    /**
+     * Gets the bean type used by this grid.
+     * <p>
+     * The bean type is used to automatically set up a column added using a
+     * property name.
+     *
+     * @return the used bean type or <code>null</code> if no bean type has been
+     *         defined
+     */
+    public Class<T> getBeanType() {
+        return beanType;
+    }
+
     public <V> void fireColumnVisibilityChangeEvent(Column<T, V> column,
             boolean hidden, boolean userOriginated) {
         fireEvent(new ColumnVisibilityChangeEvent(this, column, hidden,
@@ -3574,6 +3609,11 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
     @Override
     protected void doReadDesign(Element design, DesignContext context) {
         Attributes attrs = design.attributes();
+        if (design.hasAttr(DECLARATIVE_DATA_ITEM_TYPE)) {
+            String itemType = design.attr(DECLARATIVE_DATA_ITEM_TYPE);
+            setBeanType(itemType);
+        }
+
         if (attrs.hasKey("selection-mode")) {
             setSelectionMode(DesignAttributeHandler.readAttribute(
                     "selection-mode", attrs, SelectionMode.class));
@@ -3598,9 +3638,59 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         }
     }
 
+    /**
+     * Sets the bean type to use for property mapping.
+     * <p>
+     * This method is responsible also for setting or updating the property set
+     * so that it matches the given bean type.
+     * <p>
+     * Protected mostly for Designer needs, typically should not be overridden
+     * or even called.
+     *
+     * @param beanTypeClassName
+     *            the fully qualified class name of the bean type
+     */
+    @SuppressWarnings("unchecked")
+    protected void setBeanType(String beanTypeClassName) {
+        setBeanType((Class<T>) resolveClass(beanTypeClassName));
+    }
+
+    /**
+     * Sets the bean type to use for property mapping.
+     * <p>
+     * This method is responsible also for setting or updating the property set
+     * so that it matches the given bean type.
+     * <p>
+     * Protected mostly for Designer needs, typically should not be overridden
+     * or even called.
+     *
+     * @param beanType
+     *            the bean type class
+     */
+    protected void setBeanType(Class<T> beanType) {
+        this.beanType = beanType;
+        setPropertySet(BeanPropertySet.get(beanType));
+    }
+
+    private Class<?> resolveClass(String qualifiedClassName) {
+        try {
+            Class<?> resolvedClass = Class.forName(qualifiedClassName, true,
+                    VaadinServiceClassLoaderUtil.findDefaultClassLoader());
+            return resolvedClass;
+        } catch (ClassNotFoundException | SecurityException e) {
+            throw new IllegalArgumentException(
+                    "Unable to find class " + qualifiedClassName, e);
+        }
+
+    }
+
     @Override
     protected void doWriteDesign(Element design, DesignContext designContext) {
         Attributes attr = design.attributes();
+        if (this.beanType != null) {
+            design.attr(DECLARATIVE_DATA_ITEM_TYPE,
+                    this.beanType.getCanonicalName());
+        }
         DesignAttributeHandler.writeAttribute("selection-allowed", attr,
                 isReadOnly(), false, Boolean.class, designContext);
 
@@ -3679,14 +3769,24 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         for (Element col : colgroups.get(0).getElementsByTag("col")) {
             String id = DesignAttributeHandler.readAttribute("column-id",
                     col.attributes(), null, String.class);
-            DeclarativeValueProvider<T> provider = new DeclarativeValueProvider<>();
-            Column<T, String> column = new Column<>(provider,
-                    new HtmlRenderer());
-            addColumn(getGeneratedIdentifier(), column);
-            if (id != null) {
-                column.setId(id);
+
+            // If there is a property with a matching name available,
+            // map to that
+            Optional<PropertyDefinition<T, ?>> property = propertySet
+                    .getProperties().filter(p -> p.getName().equals(id))
+                    .findFirst();
+            Column<T, ?> column;
+            if (property.isPresent()) {
+                column = addColumn(id);
+            } else {
+                DeclarativeValueProvider<T> provider = new DeclarativeValueProvider<>();
+                column = new Column<>(provider, new HtmlRenderer());
+                addColumn(getGeneratedIdentifier(), column);
+                if (id != null) {
+                    column.setId(id);
+                }
+                providers.add(provider);
             }
-            providers.add(provider);
             column.readDesign(col, context);
         }
 

server/src/test/java/com/vaadin/tests/server/component/grid/GridCustomPropertySetTest.java

@@ -0,0 +1,176 @@
+/*
+ * Copyright 2000-2016 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 java.util.Optional;
+import java.util.stream.Stream;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.data.PropertyDefinition;
+import com.vaadin.data.PropertySet;
+import com.vaadin.data.ValueProvider;
+import com.vaadin.server.Setter;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.Column;
+
+public class GridCustomPropertySetTest {
+
+    public static class MyBeanWithoutGetters {
+        public String str;
+        public int number;
+
+        public MyBeanWithoutGetters(String str, int number) {
+            this.str = str;
+            this.number = number;
+        }
+    }
+
+    public static class GridWithCustomPropertySet
+            extends Grid<MyBeanWithoutGetters> {
+
+        private final class MyBeanPropertySet
+                implements PropertySet<MyBeanWithoutGetters> {
+
+            private PropertyDefinition<MyBeanWithoutGetters, String> strDef = new StrDefinition(
+                    this);
+            private PropertyDefinition<MyBeanWithoutGetters, Integer> numberDef = new NumberDefinition(
+                    this);
+
+            @Override
+            public Stream<PropertyDefinition<MyBeanWithoutGetters, ?>> getProperties() {
+                return Stream.of(strDef, numberDef);
+            }
+
+            @Override
+            public Optional<PropertyDefinition<MyBeanWithoutGetters, ?>> getProperty(
+                    String name) {
+                return getProperties().filter(pd -> pd.getName().equals(name))
+                        .findFirst();
+            }
+        }
+
+        private final class StrDefinition
+                implements PropertyDefinition<MyBeanWithoutGetters, String> {
+            private PropertySet<MyBeanWithoutGetters> propertySet;
+
+            public StrDefinition(
+                    PropertySet<MyBeanWithoutGetters> propertySet) {
+                this.propertySet = propertySet;
+            }
+
+            @Override
+            public ValueProvider<MyBeanWithoutGetters, String> getGetter() {
+                return bean -> bean.str;
+            }
+
+            @Override
+            public Optional<Setter<MyBeanWithoutGetters, String>> getSetter() {
+                return Optional.of((bean, value) -> bean.str = value);
+            }
+
+            @Override
+            public Class<String> getType() {
+                return String.class;
+            }
+
+            @Override
+            public String getName() {
+                return "string";
+            }
+
+            @Override
+            public String getCaption() {
+                return "The String";
+            }
+
+            @Override
+            public PropertySet<MyBeanWithoutGetters> getPropertySet() {
+                return propertySet;
+            }
+
+        }
+
+        private final class NumberDefinition
+                implements PropertyDefinition<MyBeanWithoutGetters, Integer> {
+            private PropertySet<MyBeanWithoutGetters> propertySet;
+
+            public NumberDefinition(
+                    PropertySet<MyBeanWithoutGetters> propertySet) {
+                this.propertySet = propertySet;
+            }
+
+            @Override
+            public ValueProvider<MyBeanWithoutGetters, Integer> getGetter() {
+                return bean -> bean.number;
+            }
+
+            @Override
+            public Optional<Setter<MyBeanWithoutGetters, Integer>> getSetter() {
+                return Optional.of((bean, value) -> bean.number = value);
+            }
+
+            @Override
+            public Class<Integer> getType() {
+                return Integer.class;
+            }
+
+            @Override
+            public String getName() {
+                return "numbah";
+            }
+
+            @Override
+            public String getCaption() {
+                return "The Number";
+            }
+
+            @Override
+            public PropertySet<MyBeanWithoutGetters> getPropertySet() {
+                return propertySet;
+            }
+
+        }
+
+        public GridWithCustomPropertySet() {
+            super();
+            setPropertySet(new MyBeanPropertySet());
+        }
+
+    }
+
+    @Test
+    public void customPropertySet() {
+        GridWithCustomPropertySet customGrid = new GridWithCustomPropertySet();
+        Assert.assertEquals(0, customGrid.getColumns().size());
+
+        Column<MyBeanWithoutGetters, Integer> numberColumn = (Column<MyBeanWithoutGetters, Integer>) customGrid
+                .addColumn("numbah");
+        Assert.assertEquals(1, customGrid.getColumns().size());
+        Assert.assertEquals("The Number", numberColumn.getCaption());
+        Assert.assertEquals(24, (int) numberColumn.getValueProvider()
+                .apply(new MyBeanWithoutGetters("foo", 24)));
+
+        Column<MyBeanWithoutGetters, String> stringColumn = (Column<MyBeanWithoutGetters, String>) customGrid
+                .addColumn("string");
+        Assert.assertEquals(2, customGrid.getColumns().size());
+        Assert.assertEquals("The String", stringColumn.getCaption());
+        Assert.assertEquals("foo", stringColumn.getValueProvider()
+                .apply(new MyBeanWithoutGetters("foo", 24)));
+    }
+
+}

server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java

@@ -20,8 +20,12 @@ import java.lang.reflect.Method;
 import java.util.List;
 import java.util.Locale;
 
+import org.jsoup.Jsoup;
+import org.jsoup.nodes.Document;
 import org.jsoup.nodes.Element;
 import org.jsoup.parser.Tag;
+import org.jsoup.select.Elements;
+import org.jsoup.select.Selector;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -31,7 +35,10 @@ import com.vaadin.data.provider.DataProvider;
 import com.vaadin.data.provider.Query;
 import com.vaadin.shared.ui.ContentMode;
 import com.vaadin.shared.ui.grid.HeightMode;
+import com.vaadin.tests.data.bean.Address;
+import com.vaadin.tests.data.bean.Country;
 import com.vaadin.tests.data.bean.Person;
+import com.vaadin.tests.data.bean.Sex;
 import com.vaadin.tests.server.component.abstractlisting.AbstractListingDeclarativeTest;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.Column;
@@ -744,4 +751,141 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         return person;
     }
 
+    @Test
+    public void beanItemType() throws Exception {
+        Class<Person> beanClass = Person.class;
+        String beanClassName = beanClass.getName();
+        //@formatter:off
+        String design = String.format( "<%s data-item-type=\"%s\"></%s>",
+                getComponentTag() , beanClassName, getComponentTag());
+        //@formatter:on
+
+        @SuppressWarnings("unchecked")
+        Grid<Person> grid = read(design);
+        Assert.assertEquals(beanClass, grid.getBeanType());
+
+        testWrite(design, grid);
+    }
+
+    @Test
+    public void beanGridDefaultColumns() {
+        Grid<Person> grid = new Grid<>(Person.class);
+        String design = write(grid, false);
+        assertDeclarativeColumnCount(11, design);
+
+        Person testPerson = new Person("the first", "the last", "The email", 64,
+                Sex.MALE, new Address("the street", 12313, "The city",
+                        Country.SOUTH_AFRICA));
+        @SuppressWarnings("unchecked")
+        Grid<Person> readGrid = read(design);
+
+        assertColumns(11, grid.getColumns(), readGrid.getColumns(), testPerson);
+    }
+
+    private void assertDeclarativeColumnCount(int i, String design) {
+        Document html = Jsoup.parse(design);
+        Elements cols = Selector.select("vaadin-grid", html)
+                .select("colgroup > col");
+        Assert.assertEquals("Number of columns in the design file", i,
+                cols.size());
+
+    }
+
+    private void assertColumns(int expectedCount,
+            List<Column<Person, ?>> expectedColumns,
+            List<Column<Person, ?>> columns, Person testPerson) {
+        Assert.assertEquals(expectedCount, expectedColumns.size());
+        Assert.assertEquals(expectedCount, columns.size());
+        for (int i = 0; i < expectedColumns.size(); i++) {
+            Column<Person, ?> expectedColumn = expectedColumns.get(i);
+            Column<Person, ?> column = columns.get(i);
+
+            // Property mapping
+            Assert.assertEquals(expectedColumn.getId(), column.getId());
+            // Not tested because of
+            // https://github.com/vaadin/framework/issues/8752
+            // Header caption
+            // Assert.assertEquals(expectedColumn.getCaption(),
+            // column.getCaption());
+
+            // Value providers are not stored in the declarative file
+            // so this only works for bean properties
+            if (column.getId() != null
+                    && !column.getId().equals("column" + i)) {
+                Assert.assertEquals(
+                        expectedColumn.getValueProvider().apply(testPerson),
+                        column.getValueProvider().apply(testPerson));
+            }
+        }
+
+    }
+
+    @Test
+    public void beanGridNoColumns() {
+        Grid<Person> grid = new Grid<>(Person.class);
+        grid.setColumns();
+        String design = write(grid, false);
+        assertDeclarativeColumnCount(0, design);
+
+        Person testPerson = new Person("the first", "the last", "The email", 64,
+                Sex.MALE, new Address("the street", 12313, "The city",
+                        Country.SOUTH_AFRICA));
+        @SuppressWarnings("unchecked")
+        Grid<Person> readGrid = read(design);
+
+        assertColumns(0, grid.getColumns(), readGrid.getColumns(), testPerson);
+
+        // Can add a mapped property
+        Assert.assertEquals("The email", readGrid.addColumn("email")
+                .getValueProvider().apply(testPerson));
+    }
+
+    @Test
+    public void beanGridOnlyCustomColumns() {
+        // Writes columns without propertyId even though name matches, reads
+        // columns without propertyId mapping, can add new columns using
+        // propertyId
+        Grid<Person> grid = new Grid<>(Person.class);
+        grid.setColumns();
+        grid.addColumn(Person::getFirstName).setCaption("First Name");
+        String design = write(grid, false);
+        assertDeclarativeColumnCount(1, design);
+        Person testPerson = new Person("the first", "the last", "The email", 64,
+                Sex.MALE, new Address("the street", 12313, "The city",
+                        Country.SOUTH_AFRICA));
+        @SuppressWarnings("unchecked")
+        Grid<Person> readGrid = read(design);
+
+        assertColumns(1, grid.getColumns(), readGrid.getColumns(), testPerson);
+        // First name should not be mapped to the property
+        Assert.assertNull(readGrid.getColumns().get(0).getValueProvider()
+                .apply(testPerson));
+
+        // Can add a mapped property
+        Assert.assertEquals("the last", readGrid.addColumn("lastName")
+                .getValueProvider().apply(testPerson));
+    }
+
+    @Test
+    public void beanGridOneCustomizedColumn() {
+        // Writes columns with propertyId except one without
+        // Reads columns to match initial setup
+        Grid<Person> grid = new Grid<>(Person.class);
+        grid.addColumn(
+                person -> person.getFirstName() + " " + person.getLastName())
+                .setCaption("First and Last");
+        String design = write(grid, false);
+        assertDeclarativeColumnCount(12, design);
+        Person testPerson = new Person("the first", "the last", "The email", 64,
+                Sex.MALE, new Address("the street", 12313, "The city",
+                        Country.SOUTH_AFRICA));
+        @SuppressWarnings("unchecked")
+        Grid<Person> readGrid = read(design);
+
+        assertColumns(12, grid.getColumns(), readGrid.getColumns(), testPerson);
+        // First and last name should not be mapped to anything but should exist
+        Assert.assertNull(readGrid.getColumns().get(11).getValueProvider()
+                .apply(testPerson));
+
+    }
 }