Fix Push inserts producing duplicate rows in Table (#13562)

Change-Id: I050553b233fb7024049c31d9495d90f4d88239c8

diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java
index b4d79f304c846d96f445cf3197e4c6d9dad4b648..29dc52a9a6532df4ee1791a13c7e5b513cba8691 100644 (file)
--- a/server/src/com/vaadin/ui/Table.java
+++ b/server/src/com/vaadin/ui/Table.java
@@ -2954,15 +2954,23 @@ public class Table extends AbstractSelect implements Action.Container,
             // (row caches emptied by other event)
             if (!containerChangeToBeRendered) {
                 Integer value = (Integer) variables.get("reqfirstrow");
+                int tableSize = size();
                 if (value != null) {
                     reqFirstRowToPaint = value.intValue();
+                    // Sanity check
+                    if (reqFirstRowToPaint < 0) {
+                        reqFirstRowToPaint = -1;
+                    }
+                    if (reqFirstRowToPaint >= tableSize) {
+                        reqFirstRowToPaint = tableSize - 1;
+                    }
                 }
                 value = (Integer) variables.get("reqrows");
                 if (value != null) {
                     reqRowsToPaint = value.intValue();
                     // sanity check
-                    if (reqFirstRowToPaint + reqRowsToPaint > size()) {
-                        reqRowsToPaint = size() - reqFirstRowToPaint;
+                    if (reqFirstRowToPaint + reqRowsToPaint > tableSize) {
+                        reqRowsToPaint = tableSize - reqFirstRowToPaint;
                     }
                 }
             }

diff --git a/uitest/src/com/vaadin/tests/components/table/AsyncPushUpdates.java b/uitest/src/com/vaadin/tests/components/table/AsyncPushUpdates.java
new file mode 100644 (file)
index 0000000..fcbfe1e
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/AsyncPushUpdates.java
@@ -0,0 +1,126 @@
+/*
+ * Copyright 2000-2013 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.components.table;
+
+import javax.servlet.annotation.WebServlet;
+
+import com.vaadin.annotations.Push;
+import com.vaadin.annotations.VaadinServletConfiguration;
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.VaadinServlet;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Table;
+
+/**
+ * Test to see if VScrollTable handles Push updates correctly.
+ * 
+ * @author Vaadin Ltd
+ */
+@Push
+public class AsyncPushUpdates extends AbstractTestUI {
+
+    public int clickCount = 0;
+
+    @WebServlet(value = "/*", asyncSupported = true)
+    @VaadinServletConfiguration(productionMode = false, ui = AsyncPushUpdates.class)
+    public static class Servlet extends VaadinServlet {
+    }
+
+    public static final String VALUE_PROPERTY_ID = "value";
+
+    private final IndexedContainer container = createContainer();
+    private final Table table = new Table();
+
+    @Override
+    public void setup(VaadinRequest request) {
+        table.setWidth("100%");
+        table.setContainerDataSource(container);
+
+        Button button = new Button("START");
+        button.addClickListener(new Button.ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+                ++clickCount;
+
+                container.removeAllItems();
+                for (int i = 0; i < 100; i++) {
+                    container.getContainerProperty(container.addItem(),
+                            VALUE_PROPERTY_ID).setValue("A" + i);
+                }
+
+                Runnable generateNewRows = new Runnable() {
+                    public int id = 0;
+
+                    @Override
+                    public void run() {
+                        getSession().lock();
+                        try {
+                            Thread.sleep(500);
+                            ++id;
+                            container.removeAllItems();
+                            for (int i = 0; i < 11; i++) {
+                                container.getContainerProperty(
+                                        container.addItem(), VALUE_PROPERTY_ID)
+                                        .setValue(
+                                                clickCount + " - " + id + " - "
+                                                        + i);
+                            }
+
+                        } catch (InterruptedException e) {
+                            // NOOP
+                        } finally {
+                            getSession().unlock();
+                        }
+                    }
+                };
+                new Thread(generateNewRows).start();
+            }
+        });
+        addComponent(table);
+        addComponent(button);
+    }
+
+    private static IndexedContainer createContainer() {
+        IndexedContainer container = new IndexedContainer();
+        container.addContainerProperty(VALUE_PROPERTY_ID, String.class, "");
+        return container;
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        return "Make sure there are no duplicates on the table.";
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber()
+     */
+    @Override
+    protected Integer getTicketNumber() {
+        return 13562;
+    }
+
+}
\ No newline at end of file

diff --git a/uitest/src/com/vaadin/tests/components/table/AsyncPushUpdatesTest.java b/uitest/src/com/vaadin/tests/components/table/AsyncPushUpdatesTest.java
new file mode 100644 (file)
index 0000000..b33204c
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/AsyncPushUpdatesTest.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2000-2013 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.components.table;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TableElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Test to see if VScrollTable handles Push updates correctly.
+ * 
+ * @author Vaadin Ltd
+ */
+public class AsyncPushUpdatesTest extends MultiBrowserTest {
+
+    @Test(expected = NoSuchElementException.class)
+    public void InsertedRowsAreNotDuplicated() {
+        openTestURL();
+
+        WebElement button = $(ButtonElement.class).first();
+
+        button.click();
+
+        $(TableElement.class).first().getCell(12, 0);
+        Assert.fail("Duplicates are present.");
+    }
+
+}