From 1c67d72171a110c9c72983064f247458194406ff Mon Sep 17 00:00:00 2001 From: Tulio Garcia <28783969+tulioag@users.noreply.github.com> Date: Mon, 10 May 2021 09:26:18 +0300 Subject: [PATCH] Prevent IllegalArgumentException (#468)(#918) (CP 14.5) (#949) cherry-pick: #468 cherry-pick: #918 fix: Prevent IllegalArgumentException when sortable columns are removed and recreated(#468)(#918) (CP 14.5) (#931) Fixes: https://github.com/vaadin/vaadin-grid/issues/1969 Warranty: Fixes exception related to sorting when recreating columns --- .../pom-bower-mode.xml | 11 ++ .../pom.xml | 11 ++ .../grid/it/RemoveSortableColumnPage.java | 156 ++++++++++++++++++ .../grid/it/RemoveSortableColumnsIT.java | 43 +++++ .../com/vaadin/flow/component/grid/Grid.java | 2 + 5 files changed, 223 insertions(+) create mode 100644 vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnPage.java create mode 100644 vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnsIT.java diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom-bower-mode.xml b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom-bower-mode.xml index 1cfc9b471e9..7d7dbfaf342 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom-bower-mode.xml +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom-bower-mode.xml @@ -99,12 +99,23 @@ ${project.version} test + + com.vaadin + vaadin-select-testbench + ${project.version} + test + com.vaadin vaadin-flow-components-shared ${project.version} test + + javax.servlet + javax.servlet-api + provided + diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom.xml b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom.xml index 3ce1b97d3bb..290e901999e 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom.xml +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/pom.xml @@ -105,6 +105,12 @@ ${project.version} test + + com.vaadin + vaadin-select-testbench + ${project.version} + test + org.openjdk.jol jol-core @@ -116,6 +122,11 @@ ${project.version} test + + javax.servlet + javax.servlet-api + provided + diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnPage.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnPage.java new file mode 100644 index 00000000000..93682b15523 --- /dev/null +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnPage.java @@ -0,0 +1,156 @@ + +/* + * Copyright 2000-2020 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.flow.component.grid.it; + +import com.vaadin.flow.component.AttachEvent; +import com.vaadin.flow.component.Component; +import com.vaadin.flow.component.button.Button; +import com.vaadin.flow.component.grid.Grid; +import com.vaadin.flow.component.grid.GridSortOrderBuilder; +import com.vaadin.flow.component.grid.HeaderRow; +import com.vaadin.flow.component.html.Span; +import com.vaadin.flow.component.orderedlayout.VerticalLayout; +import com.vaadin.flow.component.select.Select; +import com.vaadin.flow.function.ValueProvider; +import com.vaadin.flow.router.Route; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; + +@Route("vaadin-grid/remove-sortable-column") +public class RemoveSortableColumnPage extends VerticalLayout { + + public static final String ID_SORT_BUTTON = "sort-button"; + + private MyGrid grid; + private Select[] selects; + + private final List headers = Arrays.asList("firstName", "lastName", + "age", "gender", "nickname"); + private final Map> headerToValueProvider = new HashMap>() { + { + put("firstName", person -> person.firstName); + put("lastName", person -> person.lastName); + put("age", person -> person.age); + put("gender", person -> person.gender); + put("nickname", person -> person.nickname); + } + }; + private final Person[] people = new Person[] { + new Person("Ben", "Hanks", 25, "Male", "Bensy"), + new Person("Zackhary", "Smith", 26, "Male", "Zacsy"), + new Person("Tom", "Jones", 27, "Male", "Tomsy"), + new Person("Jerry", "Stallone", 28, "Male", "Jersey"), + new Person("Bob", "Rourke", 29, "Male", "Bobsy") }; + + class Person { + String firstName; + String lastName; + Integer age; + String gender; + String nickname; + + Person(String firstName, String lastName, Integer age, String gender, + String nickname) { + this.firstName = firstName; + this.lastName = lastName; + this.age = age; + this.gender = gender; + this.nickname = nickname; + } + } + + class MyGrid extends Grid { + + private Function headerGenerator; + private HeaderRow headerRow; + + public void setHeaderGenerator( + Function headerGenerator) { + headerRow = addFirstHeaderRow(); + this.headerGenerator = headerGenerator; + redraw(); + } + + void setData() { + setItems(people); + } + + public void redraw() { + setData(); + addColumnsToContainer(); + getDataCommunicator().reset(); + } + + void addColumnsToContainer() { + removeAllColumns(); + for (int i = 0; i < 5; ++i) { + String value = selects[i].getValue(); + addColumn(headerToValueProvider.get(value)).setSortable(true); + headerRow.getCells().get(i) + .setComponent(headerGenerator.apply(i)); + } + } + } + + public Function getHeaderGenerator() { + return columnIndex -> selects[columnIndex]; + } + + @Override + public void onAttach(AttachEvent attachEvent) { + attachEvent.getUI().getSession().setErrorHandler(handler -> { + Span text = new Span("Error"); + text.setId("error-handler-message"); + add(text); + }); + } + + public RemoveSortableColumnPage() { + selects = new Select[5]; + for (int i = 0; i < 5; ++i) { + Select select = new Select<>(); + select.setId("select" + i); + select.setItems(headers); + select.setValue(headers.get(i)); + select.getElement().addEventListener("click", e -> { + }).addEventData("event.stopPropagation()"); + select.addValueChangeListener(e -> { + grid.redraw(); + }); + selects[i] = select; + + } + grid = new MyGrid(); + grid.setHeaderGenerator(getHeaderGenerator()); + + add(grid); + + Button sort = new Button("Sort"); + sort.setId(ID_SORT_BUTTON); + sort.addClickListener(event -> { + GridSortOrderBuilder sortOrderBuilder = new GridSortOrderBuilder<>(); + sortOrderBuilder.thenAsc(grid.getColumns().get(0)); + grid.sort(sortOrderBuilder.build()); + }); + add(sort); + } + +} diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnsIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnsIT.java new file mode 100644 index 00000000000..5de653e3bf8 --- /dev/null +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/RemoveSortableColumnsIT.java @@ -0,0 +1,43 @@ +/* + * Copyright 2000-2020 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.flow.component.grid.it; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.NoSuchElementException; + +import com.vaadin.flow.component.select.testbench.SelectElement; +import com.vaadin.flow.testutil.TestPath; +import com.vaadin.tests.AbstractComponentIT; +import org.openqa.selenium.support.ui.ExpectedConditions; + +@TestPath("vaadin-grid/remove-sortable-column") +public class RemoveSortableColumnsIT extends AbstractComponentIT { + + @Test + public void removeSortableColumn_shouldNotGenerateAnException() { + By buttonLocator = By.id(RemoveSortableColumnPage.ID_SORT_BUTTON); + + open(); + waitUntil(ExpectedConditions.elementToBeClickable(buttonLocator)); + findElement(buttonLocator).click(); + SelectElement select = $(SelectElement.class).first(); + select.selectItemByIndex(2); + Assert.assertThrows(NoSuchElementException.class, + () -> findElement(By.id("error-handler-message"))); + } +} \ No newline at end of file diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java index b034163bff9..ed0ac43c9ce 100755 --- a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java @@ -2793,6 +2793,8 @@ public void removeColumn(Column column) { Objects.requireNonNull(column, "column should not be null"); ensureOwner(column); + List> order = new ArrayList<>(); + setSortOrder(order, false); removeColumnAndColumnGroupsIfNeeded(column); column.destroyDataGenerators(); keyToColumnMap.remove(column.getKey());