From 0c16623b2b9eb7acd86c2dd2411790dbdad1d1ed Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Mon, 16 Feb 2015 16:12:50 +0200 Subject: [PATCH] Resolve infinite loop when clearing and adding rows (#16747) Change-Id: Ibc364502cb756c499eb170a6a0c509f31d7fccb9 --- .../client/data/AbstractRemoteDataSource.java | 2 +- .../server/GridClearContainer.java | 72 +++++++++++++++++++ .../server/GridClearContainerTest.java | 49 +++++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainer.java create mode 100644 uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainerTest.java diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java index bd676e4ee9..1de271c646 100644 --- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java +++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java @@ -541,7 +541,7 @@ public abstract class AbstractRemoteDataSource implements DataSource { Range oldCached = cached; cached = cached.offsetBy(count); - for (int i = 1; i <= indexToRowMap.size(); i++) { + for (int i = 1; i <= cached.length(); i++) { int oldIndex = oldCached.getEnd() - i; int newIndex = cached.getEnd() - i; moveRowFromIndexToIndex(oldIndex, newIndex); diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainer.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainer.java new file mode 100644 index 0000000000..8be2dab8fd --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainer.java @@ -0,0 +1,72 @@ +/* + * Copyright 2000-2014 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.grid.basicfeatures.server; + +import com.vaadin.annotations.Theme; +import com.vaadin.data.util.IndexedContainer; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUIWithLog; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.Grid; + +/** + * Tests that removing and adding rows doesn't cause an infinite loop in the + * browser. + * + * @author Vaadin Ltd + */ +@Theme("valo") +public class GridClearContainer extends AbstractTestUIWithLog { + + private IndexedContainer ic; + + @Override + protected void setup(VaadinRequest request) { + final Grid grid = new Grid(); + ic = new IndexedContainer(); + ic.addContainerProperty("Col 1", String.class, "default"); + ic.addItem("Row 1"); + ic.addItem("Row 2"); + grid.setContainerDataSource(ic); + + Button b = new Button("Clear and re-add", new ClickListener() { + + @SuppressWarnings("unchecked") + @Override + public void buttonClick(ClickEvent event) { + ic.removeAllItems(); + ic.addItem("Row 3").getItemProperty("Col 1") + .setValue("Updated value 1"); + ic.addItem("Row 4").getItemProperty("Col 1") + .setValue("Updated value 2"); + } + }); + addComponent(b); + addComponent(grid); + } + + @Override + protected String getTestDescription() { + return "Tests that removing and adding rows doesn't cause an infinite loop in the browser."; + } + + @Override + protected Integer getTicketNumber() { + return 16747; + } +} diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainerTest.java new file mode 100644 index 0000000000..fd35dac6fc --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridClearContainerTest.java @@ -0,0 +1,49 @@ +/* + * Copyright 2000-2014 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.grid.basicfeatures.server; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Tests that removing and adding rows doesn't cause an infinite loop in the + * browser. + * + * @author Vaadin Ltd + */ +public class GridClearContainerTest extends MultiBrowserTest { + + private final String ERRORNOTE = "Unexpected cell contents."; + + @Test + public void clearAndReadd() { + openTestURL(); + ButtonElement button = $(ButtonElement.class).caption( + "Clear and re-add").first(); + GridElement grid = $(GridElement.class).first(); + Assert.assertEquals(ERRORNOTE, "default", grid.getCell(0, 0).getText()); + Assert.assertEquals(ERRORNOTE, "default", grid.getCell(1, 0).getText()); + button.click(); + Assert.assertEquals(ERRORNOTE, "Updated value 1", grid.getCell(0, 0) + .getText()); + Assert.assertEquals(ERRORNOTE, "Updated value 2", grid.getCell(1, 0) + .getText()); + } +} -- 2.39.5