Refactor editor API to use Binding instead of a component generator (#8368)

Fixes #8366

4 files changed, 129 insertions, 123 deletions

diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java
index 7aed70461e..4052282fdc 100644
--- a/server/src/main/java/com/vaadin/ui/Grid.java
+++ b/server/src/main/java/com/vaadin/ui/Grid.java
@@ -42,7 +42,9 @@ import org.jsoup.nodes.Element;
 import org.jsoup.select.Elements;
 
 import com.vaadin.data.Binder;
+import com.vaadin.data.Binder.Binding;
 import com.vaadin.data.HasDataProvider;
+import com.vaadin.data.HasValue;
 import com.vaadin.data.ValueProvider;
 import com.vaadin.data.provider.DataCommunicator;
 import com.vaadin.data.provider.DataProvider;
@@ -63,6 +65,7 @@ import com.vaadin.server.Extension;
 import com.vaadin.server.JsonCodec;
 import com.vaadin.server.SerializableComparator;
 import com.vaadin.server.SerializableFunction;
+import com.vaadin.server.Setter;
 import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.Registration;
 import com.vaadin.shared.data.DataCommunicatorConstants;
@@ -84,7 +87,6 @@ import com.vaadin.ui.components.grid.ColumnVisibilityChangeListener;
 import com.vaadin.ui.components.grid.DescriptionGenerator;
 import com.vaadin.ui.components.grid.DetailsGenerator;
 import com.vaadin.ui.components.grid.Editor;
-import com.vaadin.ui.components.grid.EditorComponentGenerator;
 import com.vaadin.ui.components.grid.EditorImpl;
 import com.vaadin.ui.components.grid.Footer;
 import com.vaadin.ui.components.grid.FooterCell;
@@ -560,8 +562,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
             assert columnInternalIds.length == directions.length : "Column and sort direction counts don't match.";
 
-            List<GridSortOrder<T>> list = new ArrayList<>(
-                    directions.length);
+            List<GridSortOrder<T>> list = new ArrayList<>(directions.length);
             for (int i = 0; i < columnInternalIds.length; ++i) {
                 Column<T, ?> column = columnKeys.get(columnInternalIds[i]);
                 list.add(new GridSortOrder<>(column, directions[i]));
@@ -784,7 +785,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         private StyleGenerator<T> styleGenerator = item -> null;
         private DescriptionGenerator<T> descriptionGenerator;
 
-        private EditorComponentGenerator<T> componentGenerator;
+        private Binding<T, ?> editorBinding;
 
         private String userId;
 
@@ -793,12 +794,13 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
          * provider.
          *
          * @param valueProvider
-         *            the function to get values from items
+         *            the function to get values from items, not
+         *            <code>null</code>
          * @param renderer
-         *            the type of value
+         *            the type of value, not <code>null</code>
          */
         protected Column(ValueProvider<T, ? extends V> valueProvider,
-                         Renderer<V> renderer) {
+                Renderer<V> renderer) {
             Objects.requireNonNull(valueProvider,
                     "Value provider can't be null");
             Objects.requireNonNull(renderer, "Renderer can't be null");
@@ -819,7 +821,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             Class<V> valueType = renderer.getPresentationType();
 
             if (Comparable.class.isAssignableFrom(valueType)) {
-                comparator = (a, b) -> compareComparables(valueProvider.apply(a), valueProvider.apply(b));
+                comparator = (a, b) -> compareComparables(
+                        valueProvider.apply(a), valueProvider.apply(b));
                 state.sortable = true;
             } else if (Number.class.isAssignableFrom(valueType)) {
                 /*
@@ -827,7 +830,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
                  * Provide explicit comparison support in this case even though
                  * Number itself isn't Comparable.
                  */
-                comparator = (a, b) -> compareNumbers((Number) valueProvider.apply(a),
+                comparator = (a, b) -> compareNumbers(
+                        (Number) valueProvider.apply(a),
                         (Number) valueProvider.apply(b));
                 state.sortable = true;
             } else {
@@ -837,7 +841,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
         @SuppressWarnings("unchecked")
         private static int compareComparables(Object a, Object b) {
-            return ((Comparator) Comparator.nullsLast(Comparator.naturalOrder())).compare(a, b);
+            return ((Comparator) Comparator
+                    .nullsLast(Comparator.naturalOrder())).compare(a, b);
         }
 
         @SuppressWarnings("unchecked")
@@ -845,17 +850,20 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             Number valueA = a != null ? a : Double.POSITIVE_INFINITY;
             Number valueB = b != null ? b : Double.POSITIVE_INFINITY;
             // Most Number implementations are Comparable
-            if (valueA instanceof Comparable && valueA.getClass().isInstance(valueB)) {
+            if (valueA instanceof Comparable
+                    && valueA.getClass().isInstance(valueB)) {
                 return ((Comparable<Number>) valueA).compareTo(valueB);
             } else if (valueA.equals(valueB)) {
                 return 0;
             } else {
                 // Fall back to comparing based on potentially truncated values
-                int compare = Long.compare(valueA.longValue(), valueB.longValue());
+                int compare = Long.compare(valueA.longValue(),
+                        valueB.longValue());
                 if (compare == 0) {
                     // This might still produce 0 even though the values are not
                     // equals, but there's nothing more we can do about that
-                    compare = Double.compare(valueA.doubleValue(), valueB.doubleValue());
+                    compare = Double.compare(valueA.doubleValue(),
+                            valueB.doubleValue());
                 }
                 return compare;
             }
@@ -979,7 +987,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
                         "Column identifier cannot be changed");
             }
             this.userId = id;
-            getParent().setColumnId(id, this);
+            getGrid().setColumnId(id, this);
             return this;
         }
 
@@ -1018,7 +1026,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             Objects.requireNonNull(caption, "Header caption can't be null");
             getState().caption = caption;
 
-            HeaderRow row = getParent().getDefaultHeaderRow();
+            HeaderRow row = getGrid().getDefaultHeaderRow();
             if (row != null) {
                 row.getCell(this).setText(caption);
             }
@@ -1128,7 +1136,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             Objects.requireNonNull(cellStyleGenerator,
                     "Cell style generator must not be null");
             this.styleGenerator = cellStyleGenerator;
-            getParent().getDataCommunicator().reset();
+            getGrid().getDataCommunicator().reset();
             return this;
         }
 
@@ -1154,7 +1162,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         public Column<T, V> setDescriptionGenerator(
                 DescriptionGenerator<T> cellDescriptionGenerator) {
             this.descriptionGenerator = cellDescriptionGenerator;
-            getParent().getDataCommunicator().reset();
+            getGrid().getDataCommunicator().reset();
             return this;
         }
 
@@ -1202,7 +1210,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             checkColumnIsAttached();
             if (expandRatio != getExpandRatio()) {
                 getState().expandRatio = expandRatio;
-                getParent().markAsDirty();
+                getGrid().markAsDirty();
             }
             return this;
         }
@@ -1267,8 +1275,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             }
             if (pixelWidth != getWidth()) {
                 getState().width = pixelWidth;
-                getParent().markAsDirty();
-                getParent().fireColumnResizeEvent(this, false);
+                getGrid().markAsDirty();
+                getGrid().fireColumnResizeEvent(this, false);
             }
             return this;
         }
@@ -1297,8 +1305,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             checkColumnIsAttached();
             if (!isWidthUndefined()) {
                 getState().width = -1;
-                getParent().markAsDirty();
-                getParent().fireColumnResizeEvent(this, false);
+                getGrid().markAsDirty();
+                getGrid().fireColumnResizeEvent(this, false);
             }
             return this;
         }
@@ -1324,7 +1332,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
                         + maxwidth + ")");
             }
             getState().minWidth = pixels;
-            getParent().markAsDirty();
+            getGrid().markAsDirty();
             return this;
         }
 
@@ -1361,7 +1369,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             }
 
             getState().maxWidth = pixels;
-            getParent().markAsDirty();
+            getGrid().markAsDirty();
             return this;
         }
 
@@ -1389,7 +1397,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             checkColumnIsAttached();
             if (resizable != isResizable()) {
                 getState().resizable = resizable;
-                getParent().markAsDirty();
+                getGrid().markAsDirty();
             }
             return this;
         }
@@ -1444,8 +1452,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             checkColumnIsAttached();
             if (hidden != isHidden()) {
                 getState().hidden = hidden;
-                getParent().fireColumnVisibilityChangeEvent(this, hidden,
-                        false);
+                getGrid().fireColumnVisibilityChangeEvent(this, hidden, false);
             }
             return this;
         }
@@ -1511,17 +1518,19 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
         /**
          * Sets whether this Column has a component displayed in Editor or not.
+         * A column can only be editable if an editor component or binding has
+         * been set.
          *
          * @param editable
          *            {@code true} if column is editable; {@code false} if not
          * @return this column
          *
-         * @see #setEditorComponent(Component)
-         * @see #setEditorComponentGenerator(EditorComponentGenerator)
+         * @see #setEditorComponent(HasValue, Setter)
+         * @see #setEditorBinding(Binding)
          */
         public Column<T, V> setEditable(boolean editable) {
-            Objects.requireNonNull(componentGenerator,
-                    "Column has no editor component defined");
+            Objects.requireNonNull(editorBinding,
+                    "Column has no editor binding or component defined");
             getState().editable = editable;
             return this;
         }
@@ -1537,56 +1546,91 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         }
 
         /**
-         * Sets a static editor component for this column.
+         * Sets an editor binding for this column. The {@link Binding} is used
+         * when a row is in editor mode to define how to populate an editor
+         * component based on the edited row and how to update an item based on
+         * the value in the editor component.
          * <p>
-         * <strong>Note:</strong> The same component cannot be used for multiple
-         * columns.
+         * To create a binding to use with a column, define a binding for the
+         * editor binder (<code>grid.getEditor().getBinder()</code>) using e.g.
+         * {@link Binder#forField(HasValue)}. You can also use
+         * {@link #setEditorComponent(HasValue, Setter)} if no validator or
+         * converter is needed for the binding.
+         * <p>
+         * The {@link HasValue} that the binding is defined to use must be a
+         * {@link Component}.
          *
-         * @param component
-         *            the editor component
+         * @param binding
+         *            the binding to use for this column
          * @return this column
          *
+         * @see #setEditorComponent(HasValue, Setter)
+         * @see Binding
+         * @see Grid#getEditor()
          * @see Editor#getBinder()
-         * @see Editor#setBinder(Binder)
-         * @see #setEditorComponentGenerator(EditorComponentGenerator)
          */
-        public Column<T, V> setEditorComponent(Component component) {
-            Objects.requireNonNull(component,
-                    "null is not a valid editor field");
-            return setEditorComponentGenerator(t -> component);
+        public Column<T, V> setEditorBinding(Binding<T, ?> binding) {
+            Objects.requireNonNull(binding, "null is not a valid editor field");
+
+            if (!(binding.getField() instanceof Component)) {
+                throw new IllegalArgumentException(
+                        "Binding target must be a component.");
+            }
+
+            this.editorBinding = binding;
+
+            return setEditable(true);
         }
 
         /**
-         * Sets a component generator to provide an editor component for this
-         * Column. This method can be used to generate any dynamic component to
-         * be displayed in the editor row.
+         * Gets the binder binding that is currently used for this column.
+         *
+         * @return the used binder binding, or <code>null</code> if no binding
+         *         is configured
+         *
+         * @see #setEditorBinding(Binding)
+         */
+        public Binding<T, ?> getEditorBinding() {
+            return editorBinding;
+        }
+
+        /**
+         * Sets a component and setter to use for editing values of this column
+         * in the editor row. This is a shorthand for use in simple cases where
+         * no validator or converter is needed. Use
+         * {@link #setEditorBinding(Binding)} to support more complex cases.
          * <p>
          * <strong>Note:</strong> The same component cannot be used for multiple
          * columns.
          *
-         * @param componentGenerator
-         *            the editor component generator
+         * @param editorComponent
+         *            the editor component
+         * @param setter
+         *            a setter that stores the component value in the row item
          * @return this column
          *
-         * @see EditorComponentGenerator
-         * @see #setEditorComponent(Component)
+         * @see Grid#getEditor()
          */
-        public Column<T, V> setEditorComponentGenerator(
-                EditorComponentGenerator<T> componentGenerator) {
-            Objects.requireNonNull(componentGenerator);
-            this.componentGenerator = componentGenerator;
-            return setEditable(true);
+        public <C extends HasValue<V> & Component> Column<T, V> setEditorComponent(
+                C editorComponent, Setter<T, V> setter) {
+            Objects.requireNonNull(editorComponent,
+                    "Editor component cannot be null");
+            Objects.requireNonNull(setter, "Setter cannot be null");
+
+            Binding<T, V> binding = getGrid().getEditor().getBinder()
+                    .bind(editorComponent, valueProvider::apply, setter);
+
+            return setEditorBinding(binding);
         }
 
         /**
-         * Gets the editor component generator for this Column.
-         *
-         * @return editor component generator
+         * Gets the grid that this column belongs to.
          *
-         * @see EditorComponentGenerator
+         * @return the grid that this column belongs to, or <code>null</code> if
+         *         this column has not yet been associated with any grid
          */
-        public EditorComponentGenerator<T> getEditorComponentGenerator() {
-            return componentGenerator;
+        protected Grid<T> getGrid() {
+            return getParent();
         }
 
         /**
@@ -1597,7 +1641,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
          *             if the column is no longer attached to any grid
          */
         protected void checkColumnIsAttached() throws IllegalStateException {
-            if (getParent() == null) {
+            if (getGrid() == null) {
                 throw new IllegalStateException(
                         "Column is no longer attached to a grid.");
             }
@@ -1621,7 +1665,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             ColumnState defaultState = new ColumnState();
 
             if (getId() == null) {
-                setId("column" + getParent().getColumns().indexOf(this));
+                setId("column" + getGrid().getColumns().indexOf(this));
             }
 
             DesignAttributeHandler.writeAttribute("column-id", attributes,
@@ -1672,6 +1716,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
          * @param designContext
          *            the design context
          */
+        @SuppressWarnings("unchecked")
         protected void readDesign(Element design, DesignContext designContext) {
             Attributes attributes = design.attributes();
 
@@ -1690,9 +1735,10 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
                  * inline data type. It will work incorrectly for other types
                  * but we don't support them anyway.
                  */
-                setEditorComponentGenerator(item -> new TextField(
-                        Optional.ofNullable(valueProvider.apply(item))
-                                .map(Object::toString).orElse("")));
+                setEditorComponent((HasValue<V> & Component) new TextField(),
+                        (item, value) -> {
+                            // Ignore user value since we don't know the setter
+                        });
                 setEditable(DesignAttributeHandler.readAttribute("editable",
                         attributes, boolean.class));
             }
@@ -1840,7 +1886,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
     /**
      * Creates a new {@code Grid} using the given caption
-     * 
+     *
      * @param caption
      *            the caption of the grid
      */
@@ -1852,7 +1898,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
     /**
      * Creates a new {@code Grid} using the given caption and
      * {@code DataProvider}
-     * 
+     *
      * @param caption
      *            the caption of the grid
      * @param dataProvider
@@ -1865,7 +1911,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
     /**
      * Creates a new {@code Grid} using the given {@code DataProvider}
-     * 
+     *
      * @param dataProvider
      *            the data provider, not {@code null}
      */
@@ -1877,7 +1923,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
     /**
      * Creates a new {@code Grid} using the given caption and collection of
      * items
-     * 
+     *
      * @param caption
      *            the caption of the grid
      * @param items
@@ -2833,9 +2879,8 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
      *
      */
     public void sort(Column<T, ?> column, SortDirection direction) {
-        setSortOrder(
-                Collections
-                        .singletonList(new GridSortOrder<>(column, direction)));
+        setSortOrder(Collections
+                .singletonList(new GridSortOrder<>(column, direction)));
     }
 
     /**
@@ -3287,9 +3332,9 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
     protected SerializableComparator<T> createSortingComparator() {
         BinaryOperator<SerializableComparator<T>> operator = (comparator1,
-                                                              comparator2) -> SerializableComparator
-                .asInstance((Comparator<T> & Serializable) comparator1
-                        .thenComparing(comparator2));
+                comparator2) -> SerializableComparator
+                        .asInstance((Comparator<T> & Serializable) comparator1
+                                .thenComparing(comparator2));
         return sortOrder.stream().map(
                 order -> order.getSorted().getComparator(order.getDirection()))
                 .reduce((x, y) -> 0, operator);
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/EditorComponentGenerator.java b/server/src/main/java/com/vaadin/ui/components/grid/EditorComponentGenerator.java
deleted file mode 100644
index b33c59a891..0000000000
--- a/server/src/main/java/com/vaadin/ui/components/grid/EditorComponentGenerator.java
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * 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.ui.components.grid;
-
-import com.vaadin.server.SerializableFunction;
-import com.vaadin.ui.Component;
-
-/**
- * A callback interface for generating an editor component corresponding to an
- * editable column of a grid. The generated component will be used in the grid
- * editor to edit the value of the column for the selected grid row.
- *
- * @author Vaadin Ltd.
- * @since 8.0
- *
- * @param <BEAN>
- *            the bean type this generator is compatible with
- */
-@FunctionalInterface
-public interface EditorComponentGenerator<BEAN>
-        extends SerializableFunction<BEAN, Component> {
-
-    /**
-     * Gets a component for a given {@code bean}.
-     *
-     * @param bean
-     *            the bean this component will be used to edit
-     * @return the generated component
-     */
-    @Override
-    public Component apply(BEAN bean);
-}
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java b/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java
index 56bf4d6414..a001a5026a 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java
@@ -24,6 +24,7 @@ import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import com.vaadin.data.Binder;
+import com.vaadin.data.Binder.Binding;
 import com.vaadin.data.BinderValidationStatus;
 import com.vaadin.data.BinderValidationStatusHandler;
 import com.vaadin.shared.ui.grid.editor.EditorClientRpc;
@@ -216,8 +217,12 @@ public class EditorImpl<T> extends AbstractGridExtension<T>
 
         getParent().getColumns().stream().filter(Column::isEditable)
                 .forEach(c -> {
-                    Component component = c.getEditorComponentGenerator()
-                            .apply(edited);
+                    Binding<T, ?> binding = c.getEditorBinding();
+
+                    assert binding
+                            .getField() instanceof Component : "Grid should enforce that the binding field is a component";
+
+                    Component component = (Component) binding.getField();
                     addComponentToGrid(component);
                     columnFields.put(c, component);
                     getState().columnFields.put(getInternalIdForColumn(c),
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java
index 5cbf5eb8b1..a24d4dac15 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java
@@ -37,6 +37,7 @@ import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.Column;
 import com.vaadin.ui.Grid.SelectionMode;
 import com.vaadin.ui.Label;
+import com.vaadin.ui.TextField;
 import com.vaadin.ui.components.grid.FooterCell;
 import com.vaadin.ui.components.grid.FooterRow;
 import com.vaadin.ui.components.grid.HeaderCell;
@@ -175,7 +176,7 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         boolean sortable = false;
         column1.setSortable(sortable);
         boolean editable = true;
-        column1.setEditorComponentGenerator(component -> null);
+        column1.setEditorComponent(new TextField(), Person::setLastName);
         column1.setEditable(editable);
         boolean resizable = false;
         column1.setResizable(resizable);