From a3ba1707d8fc8b5fcdd5719953e38716fae5641e Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Thu, 22 Jan 2026 10:37:54 +0200 Subject: [PATCH 1/3] feat: add fast grid cell dump methods to GridElement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add three new methods to GridElement for efficient grid cell extraction: - dumpVisibleCells(): dumps currently visible cells in one browser round-trip - dumpAllCells(): dumps all grid cells by scrolling through pages - dumpCells(fromRow, toRow): dumps cells for a specific row range This addresses the performance issue when testing grid content, reducing browser round-trips from N×M (one per cell) to 1-50 requests depending on grid size, resulting in 20-500x performance improvement. Only visible columns are included in the output. Hidden columns are filtered out automatically. Fixes #1876 --- .../flow/component/grid/it/GridDumpPage.java | 100 +++++++++ .../flow/component/grid/it/GridDumpIT.java | 212 ++++++++++++++++++ .../component/grid/testbench/GridElement.java | 131 +++++++++++ 3 files changed, 443 insertions(+) create mode 100644 vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java create mode 100644 vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java new file mode 100644 index 00000000000..20cace84279 --- /dev/null +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java @@ -0,0 +1,100 @@ +/* + * Copyright 2000-2026 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 java.util.stream.Collectors; +import java.util.stream.IntStream; + +import com.vaadin.flow.component.grid.Grid; +import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.data.bean.Person; +import com.vaadin.flow.data.provider.DataProvider; +import com.vaadin.flow.router.Route; + +@Route("vaadin-grid/dump") +public class GridDumpPage extends Div { + + public GridDumpPage() { + createSmallGrid(); + createMediumGrid(); + createLargeGrid(); + createGridWithHiddenColumn(); + } + + private void createSmallGrid() { + Grid grid = new Grid<>(); + grid.setItems(IntStream.range(0, 10) + .mapToObj(i -> new Person("Person " + i, i)) + .collect(Collectors.toList())); + + grid.addColumn(Person::getFirstName).setHeader("Name"); + grid.addColumn(Person::getAge).setHeader("Age"); + + grid.setId("small-grid"); + + add(grid); + } + + private void createMediumGrid() { + Grid grid = new Grid<>(); + grid.setItems(IntStream.range(0, 100) + .mapToObj(i -> new Person("Person " + i, i)) + .collect(Collectors.toList())); + + grid.addColumn(Person::getFirstName).setHeader("Name"); + grid.addColumn(Person::getAge).setHeader("Age"); + + grid.setId("medium-grid"); + + add(grid); + } + + private void createLargeGrid() { + Grid grid = new Grid<>(); + grid.setItems( + DataProvider + .fromCallbacks( + query -> IntStream + .range(query.getOffset(), + query.getOffset() + + query.getLimit()) + .mapToObj(index -> new Person( + "Person " + index, index)), + query -> 1000)); + + grid.addColumn(Person::getFirstName).setHeader("Name"); + grid.addColumn(Person::getAge).setHeader("Age"); + + grid.setId("large-grid"); + + add(grid); + } + + private void createGridWithHiddenColumn() { + Grid grid = new Grid<>(); + grid.setItems(IntStream.range(0, 10) + .mapToObj(i -> new Person("Person " + i, i)) + .collect(Collectors.toList())); + + grid.addColumn(Person::getFirstName).setHeader("Name"); + grid.addColumn(Person::getAge).setHeader("Age").setVisible(false); + grid.addColumn(person -> "Email" + person.getAge()).setHeader("Email"); + + grid.setId("hidden-column-grid"); + + add(grid); + } +} diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java new file mode 100644 index 00000000000..91f48039169 --- /dev/null +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java @@ -0,0 +1,212 @@ +/* + * Copyright 2000-2026 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 java.util.List; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.vaadin.flow.component.grid.testbench.GridElement; +import com.vaadin.flow.testutil.TestPath; +import com.vaadin.tests.AbstractComponentIT; + +@TestPath("vaadin-grid/dump") +public class GridDumpIT extends AbstractComponentIT { + + @Before + public void init() { + open(); + } + + @Test + public void dumpVisibleCells_smallGrid_returnsVisibleCells() { + GridElement grid = $(GridElement.class).id("small-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + List> cells = grid.dumpVisibleCells(); + + Assert.assertNotNull("Cells should not be null", cells); + Assert.assertTrue("Grid should have visible cells", cells.size() > 0); + + // Check first row + List firstRow = cells.get(0); + Assert.assertEquals("First row should have 2 columns", 2, + firstRow.size()); + Assert.assertEquals("Person 0", firstRow.get(0)); + Assert.assertEquals("0", firstRow.get(1)); + } + + @Test + public void dumpAllCells_smallGrid_returnsAllCells() { + GridElement grid = $(GridElement.class).id("small-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + List> cells = grid.dumpAllCells(); + + Assert.assertEquals("Should have 10 rows", 10, cells.size()); + + // Verify first and last rows + Assert.assertEquals("Person 0", cells.get(0).get(0)); + Assert.assertEquals("0", cells.get(0).get(1)); + Assert.assertEquals("Person 9", cells.get(9).get(0)); + Assert.assertEquals("9", cells.get(9).get(1)); + } + + @Test + public void dumpAllCells_mediumGrid_returnsAllCells() { + GridElement grid = $(GridElement.class).id("medium-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + List> cells = grid.dumpAllCells(); + + Assert.assertEquals("Should have 100 rows", 100, cells.size()); + + // Verify first, middle and last rows + Assert.assertEquals("Person 0", cells.get(0).get(0)); + Assert.assertEquals("Person 50", cells.get(50).get(0)); + Assert.assertEquals("Person 99", cells.get(99).get(0)); + } + + @Test + public void dumpCells_mediumGrid_returnsSpecifiedRange() { + GridElement grid = $(GridElement.class).id("medium-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + List> cells = grid.dumpCells(20, 29); + + Assert.assertEquals("Should have 10 rows", 10, cells.size()); + + // Verify row data + Assert.assertEquals("Person 20", cells.get(0).get(0)); + Assert.assertEquals("20", cells.get(0).get(1)); + Assert.assertEquals("Person 29", cells.get(9).get(0)); + Assert.assertEquals("29", cells.get(9).get(1)); + } + + @Test + public void dumpAllCells_largeGrid_returnsAllCells() { + GridElement grid = $(GridElement.class).id("large-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + List> cells = grid.dumpAllCells(); + + Assert.assertEquals("Should have 1000 rows", 1000, cells.size()); + + // Verify sampling of rows + Assert.assertEquals("Person 0", cells.get(0).get(0)); + Assert.assertEquals("Person 500", cells.get(500).get(0)); + Assert.assertEquals("Person 999", cells.get(999).get(0)); + } + + @Test + public void dumpCells_largeGrid_returnsSpecifiedRange() { + GridElement grid = $(GridElement.class).id("large-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + List> cells = grid.dumpCells(800, 850); + + Assert.assertEquals("Should have 51 rows", 51, cells.size()); + Assert.assertEquals("Person 800", cells.get(0).get(0)); + Assert.assertEquals("Person 850", cells.get(50).get(0)); + } + + @Test + public void dumpAllCells_hiddenColumn_onlyVisibleColumns() { + GridElement grid = $(GridElement.class).id("hidden-column-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + List> cells = grid.dumpAllCells(); + + Assert.assertEquals("Should have 10 rows", 10, cells.size()); + + // Should only have 2 visible columns (Name and Email, Age is hidden) + List firstRow = cells.get(0); + Assert.assertEquals("Should have 2 visible columns", 2, + firstRow.size()); + Assert.assertEquals("Person 0", firstRow.get(0)); + Assert.assertEquals("Email0", firstRow.get(1)); + } + + @Test + public void dumpCells_invalidRange_throwsException() { + GridElement grid = $(GridElement.class).id("small-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + try { + grid.dumpCells(-1, 5); + Assert.fail("Should throw IndexOutOfBoundsException"); + } catch (IndexOutOfBoundsException e) { + // Expected + } + + try { + grid.dumpCells(0, 100); + Assert.fail("Should throw IndexOutOfBoundsException"); + } catch (IndexOutOfBoundsException e) { + // Expected + } + + try { + grid.dumpCells(5, 3); + Assert.fail("Should throw IndexOutOfBoundsException"); + } catch (IndexOutOfBoundsException e) { + // Expected + } + } + + @Test + public void dumpAllCells_fasterThanGetText() { + GridElement grid = $(GridElement.class).id("medium-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + // Test dumpAllCells performance + long startDump = System.currentTimeMillis(); + List> dumpedCells = grid.dumpAllCells(); + long dumpTime = System.currentTimeMillis() - startDump; + + // Test getText performance (just first 10 rows to keep test fast) + long startGetText = System.currentTimeMillis(); + for (int row = 0; row < 10; row++) { + for (int col = 0; col < 2; col++) { + grid.getCell(row, col).getText(); + } + } + long getTextTime = System.currentTimeMillis() - startGetText; + + // Verify data is correct + Assert.assertEquals("Should have 100 rows", 100, dumpedCells.size()); + Assert.assertEquals("Person 0", dumpedCells.get(0).get(0)); + + // dumpAllCells should be significantly faster + // Even dumping 100 rows should be faster than getText on just 10 rows + Assert.assertTrue( + "dumpAllCells should be faster than getText. dumpAllCells: " + + dumpTime + "ms, getText (10 rows): " + getTextTime + + "ms", + dumpTime < getTextTime * 5); + } +} diff --git a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java index a1995f7b7b1..95cc8e71ff1 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java +++ b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java @@ -605,6 +605,137 @@ public List getCells(int rowIndex) { getAllColumns().toArray(new GridColumnElement[0])); } + /** + * Dumps all currently visible cell text content into a 2D array. This is a + * fast operation requiring only a single browser round-trip. + *

+ * Only visible columns are included in the output. + * + * @return a 2D array where each inner list represents a row, containing the + * text content of each visible column + */ + public List> dumpVisibleCells() { + waitUntilLoadingFinished(); + String script = "const grid = arguments[0];" + + "const rows = grid._getRenderedRows();" + + "return Array.from(rows).map(row => {" + + " return Array.from(row.children)" + + " .filter(cell => cell._column && !cell._column.hidden)" + + " .sort((a, b) => a._column._order - b._column._order)" + + " .map(cell => {" + + " return Array.from(cell.firstElementChild.assignedNodes())" + + " .map(node => node.textContent)" + + " .join('');" + " });" + "});"; + @SuppressWarnings("unchecked") + List> result = (List>) executeScript(script, + this); + return result != null ? result : new ArrayList<>(); + } + + /** + * Dumps cell text content for a specific row range. Automatically scrolls + * to ensure the specified rows are loaded. + *

+ * Only visible columns are included in the output. + * + * @param fromRow + * starting row index (inclusive) + * @param toRow + * ending row index (inclusive) + * @return a 2D array with cell text for the specified rows + * @throws IndexOutOfBoundsException + * if row indexes are out of bounds + */ + public List> dumpCells(int fromRow, int toRow) + throws IndexOutOfBoundsException { + int rowCount = getRowCount(); + if (fromRow < 0 || toRow < 0 || fromRow >= rowCount || toRow >= rowCount + || fromRow > toRow) { + throw new IndexOutOfBoundsException( + "fromRow and toRow: expected to be 0.." + (rowCount - 1) + + " with fromRow <= toRow, but were " + fromRow + + " and " + toRow); + } + + // Use a map to store cells by row index to avoid duplicates + java.util.Map> cellMap = new java.util.HashMap<>(); + int currentScrollRow = fromRow; + int targetRowCount = toRow - fromRow + 1; + + // Keep scrolling and collecting until we have all rows + while (cellMap.size() < targetRowCount) { + // Scroll to current position + scrollToRowByFlatIndex(currentScrollRow); + + // Extract cells with row indices to avoid duplicates + String script = "const grid = arguments[0];" + + "const fromRow = arguments[1];" + + "const toRow = arguments[2];" + + "const rows = grid._getRenderedRows();" + + "return Array.from(rows)" + + " .filter(row => row.index >= fromRow && row.index <= toRow)" + + " .map(row => ({" + " index: row.index," + + " cells: Array.from(row.children)" + + " .filter(cell => cell._column && !cell._column.hidden)" + + " .sort((a, b) => a._column._order - b._column._order)" + + " .map(cell => Array.from(cell.firstElementChild.assignedNodes())" + + " .map(node => node.textContent)" + + " .join(''))" + " }));"; + + @SuppressWarnings("unchecked") + List> chunk = (List>) executeScript( + script, this, fromRow, toRow); + + if (chunk != null && !chunk.isEmpty()) { + int maxIndex = currentScrollRow; + for (java.util.Map rowData : chunk) { + int index = ((Number) rowData.get("index")).intValue(); + @SuppressWarnings("unchecked") + List cells = (List) rowData.get("cells"); + cellMap.putIfAbsent(index, cells); + maxIndex = Math.max(maxIndex, index); + } + + // Scroll forward for next iteration + currentScrollRow = maxIndex + 1; + if (currentScrollRow > toRow) { + break; + } + } else { + // No more rows rendered, break to avoid infinite loop + break; + } + } + + // Convert map to list in correct order + List> result = new ArrayList<>(); + for (int i = fromRow; i <= toRow; i++) { + if (cellMap.containsKey(i)) { + result.add(cellMap.get(i)); + } + } + + return result; + } + + /** + * Dumps all cell text content in the grid by scrolling through all pages. + * This operation may take several seconds for large grids but is much + * faster than calling getText() on individual cells. + *

+ * Only visible columns are included in the output. + * + * @return a 2D array where each inner list represents a row, containing the + * text content of each visible column + */ + public List> dumpAllCells() { + int rowCount = getRowCount(); + if (rowCount == 0) { + return new ArrayList<>(); + } + return dumpCells(0, rowCount - 1); + } + /** * Gets the empty state content. * From 6539936b12393a2533b444f2bf386080262cd2ec Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Tue, 7 Jul 2026 11:52:27 +0300 Subject: [PATCH 2/3] refactor: align grid cell dump methods with getText and conventions Rename dump* to get*CellContents to match GridElement naming. Fix row ordering in the visible-rows dump, match GridTHTDElement.getText() text extraction exactly, deduplicate the extraction JS into one shared snippet, and drop the flaky timing assertion. --- .../flow/component/grid/it/GridDumpIT.java | 100 +++++----- .../component/grid/testbench/GridElement.java | 174 ++++++++++-------- 2 files changed, 154 insertions(+), 120 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java index 91f48039169..22e3ae4eb3b 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java @@ -34,12 +34,12 @@ public void init() { } @Test - public void dumpVisibleCells_smallGrid_returnsVisibleCells() { + public void getVisibleCellContents_smallGrid_returnsVisibleCells() { GridElement grid = $(GridElement.class).id("small-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - List> cells = grid.dumpVisibleCells(); + List> cells = grid.getVisibleCellContents(); Assert.assertNotNull("Cells should not be null", cells); Assert.assertTrue("Grid should have visible cells", cells.size() > 0); @@ -53,12 +53,12 @@ public void dumpVisibleCells_smallGrid_returnsVisibleCells() { } @Test - public void dumpAllCells_smallGrid_returnsAllCells() { + public void getAllCellContents_smallGrid_returnsAllCells() { GridElement grid = $(GridElement.class).id("small-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - List> cells = grid.dumpAllCells(); + List> cells = grid.getAllCellContents(); Assert.assertEquals("Should have 10 rows", 10, cells.size()); @@ -70,12 +70,12 @@ public void dumpAllCells_smallGrid_returnsAllCells() { } @Test - public void dumpAllCells_mediumGrid_returnsAllCells() { + public void getAllCellContents_mediumGrid_returnsAllCells() { GridElement grid = $(GridElement.class).id("medium-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - List> cells = grid.dumpAllCells(); + List> cells = grid.getAllCellContents(); Assert.assertEquals("Should have 100 rows", 100, cells.size()); @@ -86,12 +86,12 @@ public void dumpAllCells_mediumGrid_returnsAllCells() { } @Test - public void dumpCells_mediumGrid_returnsSpecifiedRange() { + public void getCellContents_mediumGrid_returnsSpecifiedRange() { GridElement grid = $(GridElement.class).id("medium-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - List> cells = grid.dumpCells(20, 29); + List> cells = grid.getCellContents(20, 29); Assert.assertEquals("Should have 10 rows", 10, cells.size()); @@ -103,12 +103,12 @@ public void dumpCells_mediumGrid_returnsSpecifiedRange() { } @Test - public void dumpAllCells_largeGrid_returnsAllCells() { + public void getAllCellContents_largeGrid_returnsAllCells() { GridElement grid = $(GridElement.class).id("large-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - List> cells = grid.dumpAllCells(); + List> cells = grid.getAllCellContents(); Assert.assertEquals("Should have 1000 rows", 1000, cells.size()); @@ -119,12 +119,12 @@ public void dumpAllCells_largeGrid_returnsAllCells() { } @Test - public void dumpCells_largeGrid_returnsSpecifiedRange() { + public void getCellContents_largeGrid_returnsSpecifiedRange() { GridElement grid = $(GridElement.class).id("large-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - List> cells = grid.dumpCells(800, 850); + List> cells = grid.getCellContents(800, 850); Assert.assertEquals("Should have 51 rows", 51, cells.size()); Assert.assertEquals("Person 800", cells.get(0).get(0)); @@ -132,12 +132,12 @@ public void dumpCells_largeGrid_returnsSpecifiedRange() { } @Test - public void dumpAllCells_hiddenColumn_onlyVisibleColumns() { + public void getAllCellContents_hiddenColumn_onlyVisibleColumns() { GridElement grid = $(GridElement.class).id("hidden-column-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - List> cells = grid.dumpAllCells(); + List> cells = grid.getAllCellContents(); Assert.assertEquals("Should have 10 rows", 10, cells.size()); @@ -150,27 +150,27 @@ public void dumpAllCells_hiddenColumn_onlyVisibleColumns() { } @Test - public void dumpCells_invalidRange_throwsException() { + public void getCellContents_invalidRange_throwsException() { GridElement grid = $(GridElement.class).id("small-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); try { - grid.dumpCells(-1, 5); + grid.getCellContents(-1, 5); Assert.fail("Should throw IndexOutOfBoundsException"); } catch (IndexOutOfBoundsException e) { // Expected } try { - grid.dumpCells(0, 100); + grid.getCellContents(0, 100); Assert.fail("Should throw IndexOutOfBoundsException"); } catch (IndexOutOfBoundsException e) { // Expected } try { - grid.dumpCells(5, 3); + grid.getCellContents(5, 3); Assert.fail("Should throw IndexOutOfBoundsException"); } catch (IndexOutOfBoundsException e) { // Expected @@ -178,35 +178,51 @@ public void dumpCells_invalidRange_throwsException() { } @Test - public void dumpAllCells_fasterThanGetText() { - GridElement grid = $(GridElement.class).id("medium-grid"); + public void getCellContents_matchesGetCellText() { + GridElement grid = $(GridElement.class).id("hidden-column-grid"); scrollToElement(grid); waitUntil(driver -> grid.getRowCount() > 0); - // Test dumpAllCells performance - long startDump = System.currentTimeMillis(); - List> dumpedCells = grid.dumpAllCells(); - long dumpTime = System.currentTimeMillis() - startDump; + List> cells = grid.getAllCellContents(); - // Test getText performance (just first 10 rows to keep test fast) - long startGetText = System.currentTimeMillis(); - for (int row = 0; row < 10; row++) { - for (int col = 0; col < 2; col++) { - grid.getCell(row, col).getText(); + int visibleColumns = grid.getVisibleColumns().size(); + for (int row = 0; row < cells.size(); row++) { + for (int col = 0; col < visibleColumns; col++) { + Assert.assertEquals( + "Cell content should match getCell().getText() at row " + + row + ", column " + col, + grid.getCell(row, col).getText(), + cells.get(row).get(col)); } } - long getTextTime = System.currentTimeMillis() - startGetText; - - // Verify data is correct - Assert.assertEquals("Should have 100 rows", 100, dumpedCells.size()); - Assert.assertEquals("Person 0", dumpedCells.get(0).get(0)); - - // dumpAllCells should be significantly faster - // Even dumping 100 rows should be faster than getText on just 10 rows - Assert.assertTrue( - "dumpAllCells should be faster than getText. dumpAllCells: " - + dumpTime + "ms, getText (10 rows): " + getTextTime - + "ms", - dumpTime < getTextTime * 5); + } + + @Test + public void getVisibleCellContents_scrolled_returnsVisibleRowsInOrder() { + GridElement grid = $(GridElement.class).id("large-grid"); + scrollToElement(grid); + waitUntil(driver -> grid.getRowCount() > 0); + + grid.scrollToRow(500); + + List> cells = grid.getVisibleCellContents(); + + Assert.assertTrue("Grid should show more than one visible row", + cells.size() > 1); + + // Only the rows in the viewport should be returned, in ascending index + // order, excluding the buffer rows the grid renders out of view. The + // grid also recycles row elements, so DOM order does not match index + // order after scrolling. + int firstVisible = grid.getFirstVisibleRowIndex(); + int lastVisible = grid.getLastVisibleRowIndex(); + Assert.assertEquals("Should return exactly the visible rows", + lastVisible - firstVisible + 1, cells.size()); + + int expectedIndex = firstVisible; + for (List row : cells) { + Assert.assertEquals("Person " + expectedIndex, row.get(0)); + expectedIndex++; + } } } diff --git a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java index 95cc8e71ff1..141ac275f2b 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java +++ b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java @@ -16,7 +16,9 @@ package com.vaadin.flow.component.grid.testbench; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.openqa.selenium.By; import org.openqa.selenium.NoSuchElementException; @@ -606,26 +608,53 @@ public List getCells(int rowIndex) { } /** - * Dumps all currently visible cell text content into a 2D array. This is a - * fast operation requiring only a single browser round-trip. + * JavaScript arrow function that, given a grid row (a {@code tr} element), + * returns the text content of its visible cells in column order. Hidden + * columns are skipped. The text extraction matches + * {@link GridTHTDElement#getText()}: the {@code textContent} of the nodes + * assigned to the cell's slot are joined, and a value that is only + * whitespace is returned as an empty string. + */ + private static final String ROW_CELL_CONTENTS_FUNCTION = """ + (row) => Array.from(row.children) + .filter((cell) => cell._column && !cell._column.hidden) + .sort((a, b) => a._column._order - b._column._order) + .map((cell) => { + const slot = cell.firstElementChild; + if (!slot || !slot.assignedNodes) { + return ''; + } + const text = Array.from(slot.assignedNodes()) + .map((node) => node.textContent).join(''); + return text.trim() === '' ? '' : text; + }) + """; + + /** + * Gets the text content of the rows currently visible in the viewport as a + * 2D list, ordered by row index. This is a fast operation requiring only a + * single browser round-trip. *

- * Only visible columns are included in the output. + * Rows that are rendered but scrolled out of view are not included; use + * {@link #getAllCellContents()} or {@link #getCellContents(int, int)} to + * include rows outside the viewport. Only visible columns are included, and + * the text of each cell matches {@link GridTHTDElement#getText()}. * - * @return a 2D array where each inner list represents a row, containing the - * text content of each visible column + * @return a list of rows, each a list holding the text of every visible + * column */ - public List> dumpVisibleCells() { + public List> getVisibleCellContents() { waitUntilLoadingFinished(); - String script = "const grid = arguments[0];" - + "const rows = grid._getRenderedRows();" - + "return Array.from(rows).map(row => {" - + " return Array.from(row.children)" - + " .filter(cell => cell._column && !cell._column.hidden)" - + " .sort((a, b) => a._column._order - b._column._order)" - + " .map(cell => {" - + " return Array.from(cell.firstElementChild.assignedNodes())" - + " .map(node => node.textContent)" - + " .join('');" + " });" + "});"; + // The grid renders a buffer of rows outside the viewport, so filter the + // rendered rows down to the ones that are at least partially visible. + String script = "const [grid] = arguments;" + "const rowCellContents = " + + ROW_CELL_CONTENTS_FUNCTION + ";" + + "const first = grid._firstVisibleIndex;" + + "const last = grid._lastVisibleIndex;" + + "return Array.from(grid._getRenderedRows())" + + " .filter((row) => row.index >= first && row.index <= last)" + + " .sort((a, b) => a.index - b.index)" + + " .map(rowCellContents);"; @SuppressWarnings("unchecked") List> result = (List>) executeScript(script, this); @@ -633,20 +662,23 @@ public List> dumpVisibleCells() { } /** - * Dumps cell text content for a specific row range. Automatically scrolls - * to ensure the specified rows are loaded. + * Gets the text content of the given row range as a 2D list, ordered by row + * index. The grid is scrolled as needed to load the requested rows. *

- * Only visible columns are included in the output. + * Only visible columns are included, and the text of each cell matches + * {@link GridTHTDElement#getText()}. * * @param fromRow * starting row index (inclusive) * @param toRow * ending row index (inclusive) - * @return a 2D array with cell text for the specified rows + * @return a list of rows, each a list holding the text of every visible + * column * @throws IndexOutOfBoundsException - * if row indexes are out of bounds + * if the row indexes are out of bounds or {@code fromRow} is + * greater than {@code toRow} */ - public List> dumpCells(int fromRow, int toRow) + public List> getCellContents(int fromRow, int toRow) throws IndexOutOfBoundsException { int rowCount = getRowCount(); if (fromRow < 0 || toRow < 0 || fromRow >= rowCount || toRow >= rowCount @@ -656,84 +688,70 @@ public List> dumpCells(int fromRow, int toRow) + " with fromRow <= toRow, but were " + fromRow + " and " + toRow); } + return collectCellContents(fromRow, toRow); + } - // Use a map to store cells by row index to avoid duplicates - java.util.Map> cellMap = new java.util.HashMap<>(); - int currentScrollRow = fromRow; - int targetRowCount = toRow - fromRow + 1; - - // Keep scrolling and collecting until we have all rows - while (cellMap.size() < targetRowCount) { - // Scroll to current position - scrollToRowByFlatIndex(currentScrollRow); - - // Extract cells with row indices to avoid duplicates - String script = "const grid = arguments[0];" - + "const fromRow = arguments[1];" - + "const toRow = arguments[2];" - + "const rows = grid._getRenderedRows();" - + "return Array.from(rows)" - + " .filter(row => row.index >= fromRow && row.index <= toRow)" - + " .map(row => ({" + " index: row.index," - + " cells: Array.from(row.children)" - + " .filter(cell => cell._column && !cell._column.hidden)" - + " .sort((a, b) => a._column._order - b._column._order)" - + " .map(cell => Array.from(cell.firstElementChild.assignedNodes())" - + " .map(node => node.textContent)" - + " .join(''))" + " }));"; + /** + * Scrolls through the given row range and collects the text content of each + * row. The caller is responsible for validating the range against the row + * count. + */ + private List> collectCellContents(int fromRow, int toRow) { + // Only a window of rows is rendered at a time, so scroll through the + // range and collect each rendered row by its index to avoid duplicates. + String script = "const [grid, fromRow, toRow] = arguments;" + + "const rowCellContents = " + ROW_CELL_CONTENTS_FUNCTION + ";" + + "return Array.from(grid._getRenderedRows())" + + " .filter((row) => row.index >= fromRow && row.index <= toRow)" + + " .map((row) => [row.index, rowCellContents(row)]);"; + + Map> cellsByRow = new HashMap<>(); + int scrollRow = fromRow; + while (scrollRow <= toRow) { + scrollToRowByFlatIndex(scrollRow); @SuppressWarnings("unchecked") - List> chunk = (List>) executeScript( + List> chunk = (List>) executeScript( script, this, fromRow, toRow); - if (chunk != null && !chunk.isEmpty()) { - int maxIndex = currentScrollRow; - for (java.util.Map rowData : chunk) { - int index = ((Number) rowData.get("index")).intValue(); - @SuppressWarnings("unchecked") - List cells = (List) rowData.get("cells"); - cellMap.putIfAbsent(index, cells); - maxIndex = Math.max(maxIndex, index); - } - - // Scroll forward for next iteration - currentScrollRow = maxIndex + 1; - if (currentScrollRow > toRow) { - break; - } - } else { - // No more rows rendered, break to avoid infinite loop - break; + int maxIndex = scrollRow; + for (List row : chunk) { + int index = ((Number) row.get(0)).intValue(); + @SuppressWarnings("unchecked") + List contents = (List) row.get(1); + cellsByRow.putIfAbsent(index, contents); + maxIndex = Math.max(maxIndex, index); } + // Advance past the rows just collected. The scroll target is always + // rendered, so maxIndex >= scrollRow and the loop makes progress. + scrollRow = maxIndex + 1; } - // Convert map to list in correct order List> result = new ArrayList<>(); for (int i = fromRow; i <= toRow; i++) { - if (cellMap.containsKey(i)) { - result.add(cellMap.get(i)); - } + result.add(cellsByRow.get(i)); } - return result; } /** - * Dumps all cell text content in the grid by scrolling through all pages. - * This operation may take several seconds for large grids but is much - * faster than calling getText() on individual cells. + * Gets the text content of all rows in the grid as a 2D list, ordered by + * row index. The grid is scrolled through all pages, so this may take a few + * seconds for large grids, but is much faster than calling + * {@link GridTHTDElement#getText()} on individual cells. *

- * Only visible columns are included in the output. + * Only visible columns are included, and the text of each cell matches + * {@link GridTHTDElement#getText()}. * - * @return a 2D array where each inner list represents a row, containing the - * text content of each visible column + * @return a list of rows, each a list holding the text of every visible + * column */ - public List> dumpAllCells() { + public List> getAllCellContents() { int rowCount = getRowCount(); if (rowCount == 0) { return new ArrayList<>(); } - return dumpCells(0, rowCount - 1); + return collectCellContents(0, rowCount - 1); } /** From 53c16f8ac3d612b1d8a4fba2a4d67a75dc4b9a6c Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Wed, 8 Jul 2026 15:21:08 +0300 Subject: [PATCH 3/3] fix: give clear error for getCellContents on empty grid getCellContents(0, 0) on an empty grid threw with a broken "0..-1" bound. Detect the empty grid first and throw with a clear message. getAllCellContents keeps returning an empty list. --- .../flow/component/grid/it/GridDumpPage.java | 14 +++++++++++ .../flow/component/grid/it/GridDumpIT.java | 24 +++++++++++++++++++ .../component/grid/testbench/GridElement.java | 8 +++++-- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java index 20cace84279..cb8de46fcb7 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/GridDumpPage.java @@ -15,6 +15,7 @@ */ package com.vaadin.flow.component.grid.it; +import java.util.List; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -32,6 +33,19 @@ public GridDumpPage() { createMediumGrid(); createLargeGrid(); createGridWithHiddenColumn(); + createEmptyGrid(); + } + + private void createEmptyGrid() { + Grid grid = new Grid<>(); + grid.setItems(List.of()); + + grid.addColumn(Person::getFirstName).setHeader("Name"); + grid.addColumn(Person::getAge).setHeader("Age"); + + grid.setId("empty-grid"); + + add(grid); } private void createSmallGrid() { diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java index 22e3ae4eb3b..dc01ad42e69 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/GridDumpIT.java @@ -177,6 +177,30 @@ public void getCellContents_invalidRange_throwsException() { } } + @Test + public void getAllCellContents_emptyGrid_returnsEmptyList() { + GridElement grid = $(GridElement.class).id("empty-grid"); + scrollToElement(grid); + + List> cells = grid.getAllCellContents(); + + Assert.assertTrue("Empty grid should return no rows", cells.isEmpty()); + } + + @Test + public void getCellContents_emptyGrid_throwsWithClearMessage() { + GridElement grid = $(GridElement.class).id("empty-grid"); + scrollToElement(grid); + + try { + grid.getCellContents(0, 0); + Assert.fail("Should throw IndexOutOfBoundsException"); + } catch (IndexOutOfBoundsException e) { + Assert.assertTrue("Message should mention the empty grid, but was: " + + e.getMessage(), e.getMessage().contains("empty")); + } + } + @Test public void getCellContents_matchesGetCellText() { GridElement grid = $(GridElement.class).id("hidden-column-grid"); diff --git a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java index 141ac275f2b..6b78f14f9da 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java +++ b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java @@ -675,12 +675,16 @@ public List> getVisibleCellContents() { * @return a list of rows, each a list holding the text of every visible * column * @throws IndexOutOfBoundsException - * if the row indexes are out of bounds or {@code fromRow} is - * greater than {@code toRow} + * if the grid is empty, if the row indexes are out of bounds, + * or if {@code fromRow} is greater than {@code toRow} */ public List> getCellContents(int fromRow, int toRow) throws IndexOutOfBoundsException { int rowCount = getRowCount(); + if (rowCount == 0) { + throw new IndexOutOfBoundsException( + "Cannot get cell contents: the grid is empty"); + } if (fromRow < 0 || toRow < 0 || fromRow >= rowCount || toRow >= rowCount || fromRow > toRow) { throw new IndexOutOfBoundsException(