Make improve of caching for hierarchical data optional Fixes #11477tags/8.7.2^0
@@ -202,6 +202,21 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { | |||
*/ | |||
private Map<Integer, T> invalidatedRows; | |||
/** | |||
* Tracking the invalidated rows inside {{@link #insertRowData(int, int)}} | |||
* and then filling cache from those invalidated rows is a feature | |||
* introduced to improve caching in hierarchical data in V8, but as this | |||
* interface is also shared with V7 compatibility package, this change | |||
* causes this issue: issue https://github.com/vaadin/framework/issues/11477 | |||
* | |||
* By having {#AbstractRemoteDataSource} define whether or not to track and | |||
* then fill cache with the invalidated rows, allows different | |||
* implementation of {#AbstractRemoteDataSource} to enable/disable this | |||
* feature, as a consequence it is possible for V7 compatibility-package of | |||
* this class to disabled it and fix the above issue. | |||
*/ | |||
private boolean trackInvalidatedRows = true; | |||
private Set<DataChangeHandler> dataChangeHandlers = new LinkedHashSet<>(); | |||
private CacheStrategy cacheStrategy = new CacheStrategy.DefaultCacheStrategy(); | |||
@@ -526,7 +541,10 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { | |||
if (!cached.isEmpty()) { | |||
cached = cached.combineWith(newUsefulData); | |||
// Attempt to restore invalidated items | |||
fillCacheFromInvalidatedRows(maxCacheRange); | |||
if (trackInvalidatedRows) { | |||
fillCacheFromInvalidatedRows(maxCacheRange); | |||
} | |||
} else { | |||
cached = newUsefulData; | |||
} | |||
@@ -721,31 +739,21 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { | |||
} | |||
} else if (cached.contains(firstRowIndex)) { | |||
int oldCacheEnd = cached.getEnd(); | |||
/* | |||
* We need to invalidate the cache from the inserted row onwards, | |||
* since the cache wants to be a contiguous range. It doesn't | |||
* support holes. | |||
* | |||
* If holes were supported, we could shift the higher part of | |||
* "cached" and leave a hole the size of "count" in the middle. | |||
*/ | |||
Range[] splitAt = cached.splitAt(firstRowIndex); | |||
cached = splitAt[0]; | |||
Range invalid = splitAt[1]; | |||
/* | |||
* If we already have a map in invalidatedRows, we're in a state | |||
* where multiple row manipulations without data received have | |||
* happened and the cache restoration is prevented completely. | |||
*/ | |||
Range[] splitOldCache = cached.splitAt(firstRowIndex); | |||
cached = splitOldCache[0]; | |||
Range invalidated = splitOldCache[1]; | |||
if (!invalid.isEmpty() && invalidatedRows == null) { | |||
invalidatedRows = new HashMap<>(); | |||
// Store all invalidated items to a map. Indices are updated to | |||
// match what they should be after the insertion. | |||
for (int i = invalid.getStart(); i < invalid.getEnd(); ++i) { | |||
invalidatedRows.put(i + count, indexToRowMap.get(i)); | |||
} | |||
if (trackInvalidatedRows) { | |||
/* | |||
* We need to invalidate the cache from the inserted row onwards, | |||
* since the cache wants to be a contiguous range. It doesn't | |||
* support holes. | |||
* | |||
* If holes were supported, we could shift the higher part of | |||
* "cached" and leave a hole the size of "count" in the middle. | |||
*/ | |||
trackInvalidatedRowsFromCache(invalidated, count); | |||
} | |||
for (int i = firstRowIndex; i < oldCacheEnd; i++) { | |||
@@ -761,6 +769,24 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { | |||
Profiler.leave("AbstractRemoteDataSource.insertRowData"); | |||
} | |||
private void trackInvalidatedRowsFromCache(Range invalidated, int insertedRowCount){ | |||
/* | |||
* If we already have a map in invalidatedRows, we're in a state | |||
* where multiple row manipulations without data received have | |||
* happened and the cache restoration is prevented completely. | |||
*/ | |||
if (!invalidated.isEmpty() && invalidatedRows == null) { | |||
invalidatedRows = new HashMap<>(); | |||
// Store all invalidated items to a map. Indices are updated | |||
// to match what they should be after the insertion. | |||
for (int i = invalidated.getStart(); i < invalidated | |||
.getEnd(); ++i) { | |||
invalidatedRows.put(i + insertedRowCount, indexToRowMap.get(i)); | |||
} | |||
} | |||
} | |||
@SuppressWarnings("boxing") | |||
private void moveRowFromIndexToIndex(int oldIndex, int newIndex) { | |||
T row = indexToRowMap.remove(oldIndex); | |||
@@ -934,4 +960,17 @@ public abstract class AbstractRemoteDataSource<T> implements DataSource<T> { | |||
protected boolean canFetchData() { | |||
return true; | |||
} | |||
/** | |||
* Sets whether or not to track invalidated rows inside | |||
* {@link #insertRowData(int, int)} and use them to fill cache when | |||
* {{@link #setRowData(int, List)}} is called. | |||
* | |||
* @param trackInvalidatedRows | |||
* a boolean value specifying if to track invalidated rows or | |||
* not, default value <code>true</code> | |||
*/ | |||
public void setTrackInvalidatedRows(boolean trackInvalidatedRows) { | |||
this.trackInvalidatedRows = trackInvalidatedRows; | |||
} | |||
} |
@@ -70,6 +70,11 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { | |||
public class RpcDataSource extends AbstractRemoteDataSource<JsonObject> { | |||
protected RpcDataSource() { | |||
/*Fixes issue https://github.com/vaadin/framework/issues/11477 | |||
More details in AbstractRemoteDataSource.trackInvalidatedRows private field*/ | |||
setTrackInvalidatedRows(false); | |||
registerRpc(DataProviderRpc.class, new DataProviderRpc() { | |||
@Override | |||
public void setRowData(int firstRow, JsonArray rowArray) { |
@@ -0,0 +1,108 @@ | |||
package com.vaadin.v7.tests.components.grid.basicfeatures.server; | |||
import com.vaadin.annotations.Widgetset; | |||
import com.vaadin.server.VaadinRequest; | |||
import com.vaadin.tests.components.AbstractReindeerTestUI; | |||
import com.vaadin.ui.Button; | |||
import com.vaadin.v7.data.Item; | |||
import com.vaadin.v7.data.sort.Sort; | |||
import com.vaadin.v7.data.util.AbstractInMemoryContainer; | |||
import com.vaadin.v7.data.util.IndexedContainer; | |||
import com.vaadin.v7.ui.CheckBox; | |||
import com.vaadin.v7.ui.Grid; | |||
import com.vaadin.v7.ui.HorizontalLayout; | |||
import com.vaadin.v7.ui.VerticalLayout; | |||
@Widgetset("com.vaadin.v7.Vaadin7WidgetSet") | |||
public class GridIndexedContainerInsertSelect extends AbstractReindeerTestUI { | |||
private static final String INFO_COLUMN_PATTERN = "InsertedIndex: %d ItemUid: %d SelectedRowIndex: %d SelectedItemUid: %s"; | |||
private static final String COLUMN1 = "Column1"; | |||
private static final String COLUMN2 = "Column2"; | |||
private int currInsertionIndex = 4; | |||
private VerticalLayout layout; | |||
private Grid grid; | |||
private IndexedContainer container; | |||
protected void setup(VaadinRequest vaadinRequest) { | |||
layout = new VerticalLayout(); | |||
layout.setSizeFull(); | |||
initGrid(); | |||
initData(container); | |||
selectFirstRow(); | |||
CheckBox checkBox = new CheckBox("Select row after insert", true); | |||
Button.ClickListener clickListener = e -> { | |||
addNewItem(); | |||
selectOnlyLastItemIfRequested(checkBox); | |||
currInsertionIndex++; | |||
}; | |||
Button button = new Button("Add row after selected row!", | |||
clickListener); | |||
HorizontalLayout inputBox = new HorizontalLayout(); | |||
inputBox.addComponents(checkBox, button); | |||
layout.addComponents(grid, inputBox); | |||
addComponent(layout); | |||
} | |||
private void initGrid() { | |||
grid = new Grid(); | |||
grid.setSizeFull(); | |||
container = new IndexedContainer(); | |||
container.addContainerProperty(COLUMN1, String.class, null); | |||
container.addContainerProperty(COLUMN2, String.class, null); | |||
grid.setContainerDataSource(container); | |||
} | |||
private void selectFirstRow() { | |||
grid.select("1"); | |||
} | |||
private void initData(IndexedContainer container) { | |||
createRowItem("1", "Item 1", "ItemCol2 1", container); | |||
createRowItem("2", "Item 2", "ItemCol2 2", container); | |||
createRowItem("3", "Item 3", "ItemCol2 3", container); | |||
} | |||
private void selectOnlyLastItemIfRequested(CheckBox checkBox) { | |||
if (checkBox.getValue()) { | |||
grid.select(currInsertionIndex + ""); | |||
} | |||
} | |||
private void addNewItem() { | |||
final Object selectedRow = grid.getSelectedRow(); | |||
int currentIndex = container.indexOfId(selectedRow); | |||
int insertIndex = currentIndex + 1; | |||
final Item thirdItem = container.addItemAt(insertIndex, | |||
currInsertionIndex + ""); | |||
setRowData(thirdItem, "Item " + currInsertionIndex, | |||
getInfoColumnContent(selectedRow, currentIndex, insertIndex)); | |||
} | |||
private void setRowData(Item thirdItem, String column1Data, | |||
String column2Data) { | |||
thirdItem.getItemProperty(COLUMN1).setValue(column1Data); | |||
thirdItem.getItemProperty(COLUMN2).setValue(column2Data); | |||
} | |||
private String getInfoColumnContent(Object selectedRow, int currentIndex, | |||
int insertIndex) { | |||
return String.format(INFO_COLUMN_PATTERN, insertIndex, | |||
currInsertionIndex, currentIndex, selectedRow); | |||
} | |||
private void createRowItem(String id, String column1, String column2, | |||
AbstractInMemoryContainer<Object, Object, Item> container) { | |||
Item firstRow = container.addItem(id); | |||
setRowData(firstRow, column1, column2); | |||
} | |||
} |
@@ -0,0 +1,66 @@ | |||
package com.vaadin.v7.tests.components.grid.basicfeatures.server; | |||
import com.vaadin.testbench.elements.ButtonElement; | |||
import com.vaadin.testbench.elements.GridElement; | |||
import com.vaadin.testbench.parallel.TestCategory; | |||
import com.vaadin.tests.tb3.SingleBrowserTest; | |||
import org.junit.Assert; | |||
import org.junit.Test; | |||
import org.openqa.selenium.By; | |||
@TestCategory("grid") | |||
public class GridIndexedContainerInsertSelectTest extends SingleBrowserTest { | |||
@Override | |||
public void setup() throws Exception { | |||
super.setup(); | |||
openTestURL(); | |||
waitForElementPresent(By.className("v-grid")); | |||
} | |||
/** | |||
* Test asserting that issue https://github.com/vaadin/framework/issues/11477 | |||
* is fixed. | |||
*/ | |||
@Test | |||
public void test_insertRowAfterSelected_newRowIsSelected() { | |||
openTestURL(); | |||
// Assert that first row is already selected when ui loaded | |||
Assert.assertTrue( | |||
"First row should be selected to continue with the test!", | |||
isRowSelected(0)); | |||
// Add new row after the selected one | |||
$(ButtonElement.class).first().click(); | |||
// Assert that the new row is added correctly | |||
Assert.assertEquals("Item 4", | |||
$(GridElement.class).first().getRow(1).getCell(0).getText()); | |||
// Assert that the new added row is selected | |||
Assert.assertTrue("Newly inserted row should be selected!", | |||
isRowSelected(1)); | |||
// Select row at index 2 | |||
$(GridElement.class).first().getRow(2).click(); | |||
// Add new row after the selected one | |||
$(ButtonElement.class).first().click(); | |||
// Assert that the new row is added correctly | |||
Assert.assertEquals("Item 5", | |||
$(GridElement.class).first().getRow(3).getCell(0).getText()); | |||
// Assert that the new added row is selected | |||
Assert.assertTrue("Newly inserted row should be selected!", | |||
isRowSelected(3)); | |||
} | |||
protected boolean isRowSelected(int index) { | |||
return $(GridElement.class).first().getRow(index).isSelected(); | |||
} | |||
} |