From: Johannes Dahlström Date: Tue, 13 Jan 2015 10:21:53 +0000 (+0200) Subject: Fix IndexOutOfBoundsException in RpcDataProviderExtension (#15443) X-Git-Tag: 7.4.0.beta3~4^2^2~19^2~13 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=714384ea8a053ab75bec050a1fc67a56aeee8bd5;p=vaadin-framework.git Fix IndexOutOfBoundsException in RpcDataProviderExtension (#15443) Change-Id: I5688e369bd6247afe0c8ed381964445dfc2c1ec1 --- diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index 2e10ccfef1..2e6b4a8414 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -96,15 +96,14 @@ public class RpcDataProviderExtension extends AbstractExtension { // private implementation } - void preActiveRowsChange(Range newActiveRange, int firstNewIndex, - List itemIds) { + void setActiveRange(Range newActiveRange) { final Range[] removed = activeRange.partitionWith(newActiveRange); final Range[] added = newActiveRange.partitionWith(activeRange); removeActiveRows(removed[0]); removeActiveRows(removed[2]); - addActiveRows(added[0], firstNewIndex, itemIds); - addActiveRows(added[2], firstNewIndex, itemIds); + addActiveRows(added[0]); + addActiveRows(added[2]); activeRange = newActiveRange; } @@ -121,10 +120,17 @@ public class RpcDataProviderExtension extends AbstractExtension { } } - private void addActiveRows(final Range added, int firstNewIndex, - List newItemIds) { + private void addActiveRows(Range added) { + if (added.isEmpty()) { + // Some container.getItemIds() implementations just might be + // expensive even for an empty range, so bail out early + return; + } - for (int i = added.getStart(); i < added.getEnd(); i++) { + List newItemIds = container.getItemIds(added.getStart(), + added.length()); + + for (int i = 0; i < newItemIds.size(); i++) { /* * We might be in a situation we have an index <-> itemId entry @@ -137,8 +143,8 @@ public class RpcDataProviderExtension extends AbstractExtension { * if-state. But it sounds too stupid (and most often too * insignificant) to try out. */ - final Integer ii = Integer.valueOf(i); - if (indexToItemId.containsKey(ii)) { + final Integer index = Integer.valueOf(i + added.getStart()); + if (indexToItemId.containsKey(index)) { continue; } @@ -149,11 +155,11 @@ public class RpcDataProviderExtension extends AbstractExtension { * In that case, we only want to add an index for that entry, * and not overwrite the key. */ - final Object itemId = newItemIds.get(i - firstNewIndex); + final Object itemId = newItemIds.get(i); if (!itemIdToKey.containsKey(itemId)) { itemIdToKey.put(itemId, nextKey()); } - indexToItemId.forcePut(ii, itemId); + indexToItemId.forcePut(index, itemId); } } @@ -757,10 +763,9 @@ public class RpcDataProviderExtension extends AbstractExtension { active = active.combineWith(cached); } - List itemIds = RpcDataProviderExtension.this.container.getItemIds( - firstRowToPush, numberOfRows); - keyMapper.preActiveRowsChange(active, firstRowToPush, itemIds); + keyMapper.setActiveRange(active); + List itemIds = container.getItemIds(firstRowToPush, numberOfRows); JsonArray rows = Json.createArray(); for (int i = 0; i < itemIds.size(); ++i) { rows.set(i, getRowData(getGrid().getColumns(), itemIds.get(i))); diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java index 35b2fc24fe..74c3c0db35 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java @@ -59,8 +59,7 @@ public class GridEditorTest extends GridBasicFeaturesTest { selectMenuPath("Component", "Editor", "Enabled"); selectMenuPath("Component", "Editor", "Edit item 5"); assertEditorClosed(); - boolean thrown = getLogRow(0).startsWith( - "5. Exception occured, java.lang.IllegalStateException"); + boolean thrown = logContainsText("Exception occured, java.lang.IllegalStateException"); assertTrue("IllegalStateException thrown", thrown); } @@ -69,8 +68,7 @@ public class GridEditorTest extends GridBasicFeaturesTest { selectMenuPath("Component", "Editor", "Edit item 5"); selectMenuPath("Component", "Editor", "Enabled"); assertEditorOpen(); - boolean thrown = getLogRow(0).startsWith( - "5. Exception occured, java.lang.IllegalStateException"); + boolean thrown = logContainsText("Exception occured, java.lang.IllegalStateException"); assertTrue("IllegalStateException thrown", thrown); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridRowAddRemoveTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridRowAddRemoveTest.java index 28c9e441dc..8535efb9ef 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridRowAddRemoveTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridRowAddRemoveTest.java @@ -30,9 +30,9 @@ public class GridRowAddRemoveTest extends GridBasicFeaturesTest { selectMenuPath("Component", "Body rows", "Remove all rows"); selectMenuPath("Component", "Body rows", "Add 18 rows"); - Assert.assertEquals( + Assert.assertTrue( "All added rows should be fetched in the same round trip.", - "2. Requested items 0 - 18", getLogRow(0)); + logContainsText("Requested items 0 - 18")); } @Test diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java index bf1d1329aa..2fbaa58cab 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java @@ -35,7 +35,8 @@ public class GridStaticSectionComponentTest extends GridBasicFeaturesTest { getGridElement().$(ButtonElement.class).first().click(); - assertEquals("3. Button clicked!", getLogRow(0)); + assertTrue("Button click should be logged", + logContainsText("Button clicked!")); } @Test @@ -49,7 +50,8 @@ public class GridStaticSectionComponentTest extends GridBasicFeaturesTest { getGridElement().$(ButtonElement.class).first().click(); - assertEquals("5. Button clicked!", getLogRow(0)); + assertTrue("Button click should be logged", + logContainsText("Button clicked!")); } @Test diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java index 1f16efdd39..b122eb02e9 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/LoadingIndicatorTest.java @@ -68,8 +68,9 @@ public class LoadingIndicatorTest extends GridBasicFeaturesTest { Thread.sleep(2000); String firstLogRow = getLogRow(0); - Assert.assertTrue("Last log message was not the fourth message: " - + firstLogRow, firstLogRow.startsWith("4. Requested items")); + Assert.assertTrue( + "Last log message should be number 6: " + firstLogRow, + firstLogRow.startsWith("6. Requested items")); } private boolean isLoadingIndicatorVisible() {