Fix RpcDataProviderExtension serialization issue (#19906)

Change-Id: I74e8e82df2a7bb26c45c9c3797d044061ec3c24d

2 files changed, 67 insertions, 8 deletions

diff --git a/server/src/main/java/com/vaadin/server/communication/data/RpcDataProviderExtension.java b/server/src/main/java/com/vaadin/server/communication/data/RpcDataProviderExtension.java
index 1b78ca4518..08c70c14cf 100644
--- a/server/src/main/java/com/vaadin/server/communication/data/RpcDataProviderExtension.java
+++ b/server/src/main/java/com/vaadin/server/communication/data/RpcDataProviderExtension.java
@@ -261,13 +261,13 @@ public class RpcDataProviderExtension extends AbstractExtension {
     private boolean refreshCache = false;
 
     /** Set of updated item ids */
-    private Set<Object> updatedItemIds = new LinkedHashSet<Object>();
+    private transient Set<Object> updatedItemIds;
 
     /**
      * Queued RPC calls for adding and removing rows. Queue will be handled in
      * {@link beforeClientResponse}
      */
-    private List<Runnable> rowChanges = new ArrayList<Runnable>();
+    private transient List<Runnable> rowChanges;
 
     /** Size possibly changed with a bare ItemSetChangeEvent */
     private boolean bareItemSetTriggeredSizeChange = false;
@@ -334,22 +334,30 @@ public class RpcDataProviderExtension extends AbstractExtension {
             pushRowData(0, numberOfRows, 0, 0);
         } else {
             // Only do row changes if not initial response.
-            for (Runnable r : rowChanges) {
-                r.run();
+            if (rowChanges != null) {
+                for (Runnable r : rowChanges) {
+                    r.run();
+                }
             }
 
             // Send current rows again if needed.
             if (refreshCache) {
-                updatedItemIds.addAll(activeItemHandler.getActiveItemIds());
+                for (Object itemId : activeItemHandler.getActiveItemIds()) {
+                    updateRowData(itemId);
+                }
             }
         }
 
         internalUpdateRows(updatedItemIds);
 
         // Clear all changes.
-        rowChanges.clear();
+        if (rowChanges != null) {
+            rowChanges.clear();
+        }
+        if (updatedItemIds != null) {
+            updatedItemIds.clear();
+        }
         refreshCache = false;
-        updatedItemIds.clear();
         bareItemSetTriggeredSizeChange = false;
 
         super.beforeClientResponse(initial);
@@ -446,6 +454,10 @@ public class RpcDataProviderExtension extends AbstractExtension {
      *            the number of rows inserted at <code>index</code>
      */
     private void insertRowData(final int index, final int count) {
+        if (rowChanges == null) {
+            rowChanges = new ArrayList<Runnable>();
+        }
+
         if (rowChanges.isEmpty()) {
             markAsDirty();
         }
@@ -475,6 +487,10 @@ public class RpcDataProviderExtension extends AbstractExtension {
      *            the item id of the first removed item
      */
     private void removeRowData(final int index, final int count) {
+        if (rowChanges == null) {
+            rowChanges = new ArrayList<Runnable>();
+        }
+
         if (rowChanges.isEmpty()) {
             markAsDirty();
         }
@@ -496,6 +512,10 @@ public class RpcDataProviderExtension extends AbstractExtension {
      *            the item Id the row that was updated
      */
     public void updateRowData(Object itemId) {
+        if (updatedItemIds == null) {
+            updatedItemIds = new LinkedHashSet<Object>();
+        }
+
         if (updatedItemIds.isEmpty()) {
             // At least one new item will be updated. Mark as dirty to actually
             // update before response to client.
@@ -506,7 +526,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
     }
 
     private void internalUpdateRows(Set<Object> itemIds) {
-        if (itemIds.isEmpty()) {
+        if (itemIds == null || itemIds.isEmpty()) {
             return;
         }
 
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridContainerTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridContainerTest.java
index 079487d01f..c5747e4073 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridContainerTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridContainerTest.java
@@ -15,18 +15,36 @@
  */
 package com.vaadin.tests.server.component.grid;
 
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+import java.lang.reflect.Field;
+
 import org.junit.Assert;
 import org.junit.Test;
 
 import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.communication.data.RpcDataProviderExtension;
 import com.vaadin.ui.Component;
+import com.vaadin.ui.ConnectorTracker;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.DetailsGenerator;
 import com.vaadin.ui.Grid.RowReference;
 import com.vaadin.ui.Label;
+import com.vaadin.ui.UI;
 
 public class GridContainerTest {
 
+    /**
+     * Null Stream used with serialization tests
+     */
+    protected static OutputStream NULLSTREAM = new OutputStream() {
+        @Override
+        public void write(int b) {
+        }
+    };
+
     @Test
     public void testDetailsGeneratorDoesNotResetOnContainerChange() {
         Grid grid = new Grid();
@@ -119,4 +137,25 @@ public class GridContainerTest {
         grid.addColumn("foo");
         grid.addColumn("foo");
     }
+
+    @Test
+    public void testSerializeRpcDataProviderWithRowChanges() throws IOException {
+        Grid grid = new Grid();
+        IndexedContainer container = new IndexedContainer();
+        grid.setContainerDataSource(container);
+        container.addItem();
+        serializeComponent(grid);
+    }
+
+    protected void serializeComponent(Component component) throws IOException {
+        ObjectOutputStream stream = null;
+        try {
+            stream = new ObjectOutputStream(NULLSTREAM);
+            stream.writeObject(component);
+        } finally {
+            if (stream != null) {
+                stream.close();
+            }
+        }
+    }
 }