Hide DataGenerator from Column API (#9351)

3 files changed, 107 insertions, 63 deletions

diff --git a/all/src/main/templates/release-notes.html b/all/src/main/templates/release-notes.html
index abfda6bf2e..82aab0c1b5 100644
--- a/all/src/main/templates/release-notes.html
+++ b/all/src/main/templates/release-notes.html
@@ -106,7 +106,8 @@
         <li>The <tt>DragSourceExtension</tt> and <tt>DropTargetExtension</tt> extensions replace the old DnD features</li>
         <li>OSGi bundle manifests of Vaadin Framework JARs no longer export <tt>/VAADIN</tt>, and there are new mechanisms for publishing static resources for OSGi</li>
         <li>Tooltip styles for <tt>ContentMode.PREFORMATTED</tt> have been changed in all built-in themes to use the application font and allow long lines to wrap to multiple lines.</li>
-        
+        <li><tt>Grid.Column</tt> now extends <tt>AbstractExtension</tt> instead of <tt>AbstractGridExtension</tt> to hide data generator specific API.</li>
+
         <h2>For incompatible or behaviour-altering changes in 8.0, please see <a href="https://vaadin.com/download/release/8.0/8.0.0/release-notes.html#incompatible">8.0 release notes</a></h2>
         
                 
diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java
index 845a61a42a..4f4ef43647 100644
--- a/server/src/main/java/com/vaadin/ui/Grid.java
+++ b/server/src/main/java/com/vaadin/ui/Grid.java
@@ -52,6 +52,7 @@ import com.vaadin.data.PropertySet;
 import com.vaadin.data.ValueProvider;
 import com.vaadin.data.provider.CallbackDataProvider;
 import com.vaadin.data.provider.DataCommunicator;
+import com.vaadin.data.provider.DataGenerator;
 import com.vaadin.data.provider.DataProvider;
 import com.vaadin.data.provider.GridSortOrder;
 import com.vaadin.data.provider.GridSortOrderBuilder;
@@ -65,6 +66,7 @@ import com.vaadin.event.SortEvent.SortNotifier;
 import com.vaadin.event.selection.MultiSelectionListener;
 import com.vaadin.event.selection.SelectionListener;
 import com.vaadin.event.selection.SingleSelectionListener;
+import com.vaadin.server.AbstractExtension;
 import com.vaadin.server.EncodeResult;
 import com.vaadin.server.Extension;
 import com.vaadin.server.JsonCodec;
@@ -817,7 +819,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
      * @param <V>
      *            the column value type
      */
-    public static class Column<T, V> extends AbstractGridExtension<T> {
+    public static class Column<T, V> extends AbstractExtension {
 
         private final ValueProvider<T, V> valueProvider;
 
@@ -833,6 +835,63 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         private SerializableComparator<T> comparator;
         private StyleGenerator<T> styleGenerator = item -> null;
         private DescriptionGenerator<T> descriptionGenerator;
+        private DataGenerator<T> dataGenerator = new DataGenerator<T>() {
+
+            @Override
+            public void generateData(T item, JsonObject jsonObject) {
+                ColumnState state = getState(false);
+
+                String communicationId = getConnectorId();
+
+                assert communicationId != null : "No communication ID set for column "
+                        + state.caption;
+
+                @SuppressWarnings("unchecked")
+                Renderer<V> renderer = (Renderer<V>) state.renderer;
+
+                JsonObject obj = getDataObject(jsonObject,
+                        DataCommunicatorConstants.DATA);
+
+                V providerValue = valueProvider.apply(item);
+
+                // Make Grid track components.
+                if (renderer instanceof ComponentRenderer
+                        && providerValue instanceof Component) {
+                    addComponent(item, (Component) providerValue);
+                }
+                JsonValue rendererValue = renderer.encode(providerValue);
+
+                obj.put(communicationId, rendererValue);
+
+                String style = styleGenerator.apply(item);
+                if (style != null && !style.isEmpty()) {
+                    JsonObject styleObj = getDataObject(jsonObject,
+                            GridState.JSONKEY_CELLSTYLES);
+                    styleObj.put(communicationId, style);
+                }
+                if (descriptionGenerator != null) {
+                    String description = descriptionGenerator.apply(item);
+                    if (description != null && !description.isEmpty()) {
+                        JsonObject descriptionObj = getDataObject(jsonObject,
+                                GridState.JSONKEY_CELLDESCRIPTION);
+                        descriptionObj.put(communicationId, description);
+                    }
+                }
+            }
+
+            @Override
+            public void destroyData(T item) {
+                removeComponent(item);
+            }
+
+            @Override
+            public void destroyAllData() {
+                // Make a defensive copy of keys, as the map gets cleared when
+                // removing components.
+                new HashSet<>(activeComponents.keySet())
+                        .forEach(component -> removeComponent(component));
+            }
+        };
 
         private Binding<T, ?> editorBinding;
         private Map<T, Component> activeComponents = new HashMap<>();
@@ -949,48 +1008,6 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             }
         }
 
-        @Override
-        public void generateData(T item, JsonObject jsonObject) {
-            ColumnState state = getState(false);
-
-            String communicationId = getConnectorId();
-
-            assert communicationId != null : "No communication ID set for column "
-                    + state.caption;
-
-            @SuppressWarnings("unchecked")
-            Renderer<V> renderer = (Renderer<V>) state.renderer;
-
-            JsonObject obj = getDataObject(jsonObject,
-                    DataCommunicatorConstants.DATA);
-
-            V providerValue = valueProvider.apply(item);
-
-            // Make Grid track components.
-            if (renderer instanceof ComponentRenderer
-                    && providerValue instanceof Component) {
-                addComponent(item, (Component) providerValue);
-            }
-            JsonValue rendererValue = renderer.encode(providerValue);
-
-            obj.put(communicationId, rendererValue);
-
-            String style = styleGenerator.apply(item);
-            if (style != null && !style.isEmpty()) {
-                JsonObject styleObj = getDataObject(jsonObject,
-                        GridState.JSONKEY_CELLSTYLES);
-                styleObj.put(communicationId, style);
-            }
-            if (descriptionGenerator != null) {
-                String description = descriptionGenerator.apply(item);
-                if (description != null && !description.isEmpty()) {
-                    JsonObject descriptionObj = getDataObject(jsonObject,
-                            GridState.JSONKEY_CELLDESCRIPTION);
-                    descriptionObj.put(communicationId, description);
-                }
-            }
-        }
-
         private void addComponent(T item, Component component) {
             if (activeComponents.containsKey(item)) {
                 if (activeComponents.get(item).equals(component)) {
@@ -1000,29 +1017,16 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
                 removeComponent(item);
             }
             activeComponents.put(item, component);
-            addComponentToGrid(component);
-        }
-
-        @Override
-        public void destroyData(T item) {
-            removeComponent(item);
+            getGrid().addExtensionComponent(component);
         }
 
         private void removeComponent(T item) {
             Component component = activeComponents.remove(item);
             if (component != null) {
-                removeComponentFromGrid(component);
+                getGrid().removeExtensionComponent(component);
             }
         }
 
-        @Override
-        public void destroyAllData() {
-            // Make a defensive copy of keys, as the map gets cleared when
-            // removing components.
-            new HashSet<>(activeComponents.keySet())
-                    .forEach(this::removeComponent);
-        }
-
         /**
          * Gets a data object with the given key from the given JsonObject. If
          * there is no object with the key, this method creates a new
@@ -1842,7 +1846,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             addExtension(renderer);
 
             // Trigger redraw
-            getParent().getDataCommunicator().reset();
+            getGrid().getDataCommunicator().reset();
 
             return this;
         }
@@ -1854,7 +1858,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
          *         this column has not yet been associated with any grid
          */
         protected Grid<T> getGrid() {
-            return getParent();
+            return (Grid<T>) getParent();
         }
 
         /**
@@ -2006,6 +2010,15 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
                 }
             }
         }
+
+        /**
+         * Gets the DataGenerator for this Column.
+         * 
+         * @return data generator
+         */
+        private DataGenerator<T> getDataGenerator() {
+            return dataGenerator;
+        }
     }
 
     private class HeaderImpl extends Header {
@@ -2466,7 +2479,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         columnSet.add(column);
         columnKeys.put(identifier, column);
         column.setInternalId(identifier);
-        addDataGenerator(column);
+        addDataGenerator(column.getDataGenerator());
 
         getState().columnOrder.add(identifier);
         getHeader().addColumn(identifier);
@@ -2491,6 +2504,7 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             columnKeys.remove(columnId);
             columnIds.remove(column.getId());
             column.remove();
+            removeDataGenerator(column.getDataGenerator());
             getHeader().removeColumn(columnId);
             getFooter().removeColumn(columnId);
             getState(true).columnOrder.remove(columnId);
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java
index 8662aacffa..cf43ed8339 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java
@@ -4,6 +4,8 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.text.DecimalFormat;
 import java.text.DecimalFormatSymbols;
 import java.util.ArrayList;
@@ -29,6 +31,7 @@ import org.junit.Test;
 import com.vaadin.data.Binder.Binding;
 import com.vaadin.data.ValidationException;
 import com.vaadin.data.ValueProvider;
+import com.vaadin.data.provider.DataGenerator;
 import com.vaadin.data.provider.GridSortOrder;
 import com.vaadin.data.provider.QuerySortOrder;
 import com.vaadin.data.provider.bov.Person;
@@ -45,6 +48,7 @@ import com.vaadin.ui.renderers.NumberRenderer;
 
 import elemental.json.Json;
 import elemental.json.JsonObject;
+import junit.framework.AssertionFailedError;
 
 public class GridTest {
 
@@ -624,11 +628,36 @@ public class GridTest {
         // generateData only works if Grid is attached
         new MockUI().setContent(grid);
 
-        grid.getColumns().forEach(column -> column.generateData(row, json));
+        Method getter = findDataGeneratorGetterMethod();
+        grid.getColumns().forEach(column -> {
+            DataGenerator<T> dataGenerator;
+            try {
+                dataGenerator = (DataGenerator<T>) getter.invoke(column,
+                        new Object[] {});
+                dataGenerator.generateData(row, json);
+            } catch (IllegalAccessException | IllegalArgumentException
+                    | InvocationTargetException e) {
+                throw new AssertionFailedError(
+                        "Cannot get DataGenerator from Column");
+            }
+        });
 
         // Detach again
         grid.getUI().setContent(null);
 
         return json.getObject("d");
     }
+
+    private static Method findDataGeneratorGetterMethod() {
+        Method getter;
+        try {
+            getter = Column.class.getDeclaredMethod("getDataGenerator",
+                    new Class<?>[] {});
+            getter.setAccessible(true);
+            return getter;
+        } catch (NoSuchMethodException | SecurityException e) {
+            throw new AssertionFailedError(
+                    "Cannot get DataGenerator from Column");
+        }
+    }
 }