From 12c4745519cc3b182846998c221fb93be279c755 Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Thu, 3 Nov 2011 15:58:23 +0000 Subject: [PATCH] #7839 Out of Sync error when using Generated Columns with TreeTable Fixed multiple indexing issues that could occur in partial updates in Table and TreeTable svn changeset:21889/svn branch:6.7 --- .../terminal/gwt/client/ui/VScrollTable.java | 86 ++++++++- src/com/vaadin/ui/Table.java | 182 ++++++++++++++---- 2 files changed, 226 insertions(+), 42 deletions(-) diff --git a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java index 933c5de1c9..1588936c3c 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java +++ b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java @@ -104,6 +104,9 @@ import com.vaadin.terminal.gwt.client.ui.dd.VerticalDropLocation; public class VScrollTable extends FlowPanel implements Table, ScrollHandler, VHasDropHandler, FocusHandler, BlurHandler, Focusable, ActionOwner { + public static final String ATTRIBUTE_PAGEBUFFER_FIRST = "pb-ft"; + public static final String ATTRIBUTE_PAGEBUFFER_LAST = "pb-l"; + private static final String ROW_HEADER_COLUMN_KEY = "0"; public static final String CLASSNAME = "v-table"; @@ -203,6 +206,8 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler, private String[] bodyActionKeys; + private boolean enableDebug = false; + /** * Represents a select range of rows */ @@ -423,6 +428,21 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler, private int lastRenderedHeight; + /** + * Values (serverCacheFirst+serverCacheLast) sent by server that tells which + * rows (indexes) are in the server side cache (page buffer). -1 means + * unknown. The server side cache row MUST MATCH the client side cache rows. + * + * If the client side cache contains additional rows with e.g. buttons, it + * will cause out of sync when such a button is pressed. + * + * If the server side cache contains additional rows with e.g. buttons, + * scrolling in the client will cause empty buttons to be rendered + * (cached=true request for non-existing components) + */ + private int serverCacheFirst = -1; + private int serverCacheLast = -1; + public VScrollTable() { setMultiSelectMode(MULTISELECT_MODE_DEFAULT); @@ -798,6 +818,13 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler, public void updateFromUIDL(UIDL uidl, ApplicationConnection client) { rendering = true; + if (uidl.hasAttribute(ATTRIBUTE_PAGEBUFFER_FIRST)) { + serverCacheFirst = uidl.getIntAttribute(ATTRIBUTE_PAGEBUFFER_FIRST); + serverCacheLast = uidl.getIntAttribute(ATTRIBUTE_PAGEBUFFER_LAST); + } else { + serverCacheFirst = -1; + serverCacheLast = -1; + } /* * We need to do this before updateComponent since updateComponent calls * this.setHeight() which will calculate a new body height depending on @@ -1413,24 +1440,67 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler, * caching window. */ protected void discardRowsOutsideCacheWindow() { - final int optimalFirstRow = (int) (firstRowInViewPort - pageLength + int firstRowToKeep = (int) (firstRowInViewPort - pageLength + * cache_rate); + int lastRowToKeep = (int) (firstRowInViewPort + pageLength + pageLength * cache_rate); + debug("Client side calculated cache rows to keep: " + firstRowToKeep + + "-" + lastRowToKeep); + + if (serverCacheFirst != -1) { + firstRowToKeep = serverCacheFirst; + lastRowToKeep = serverCacheLast; + debug("Server cache rows that override: " + serverCacheFirst + "-" + + serverCacheLast); + if (firstRowToKeep < scrollBody.getFirstRendered() + || lastRowToKeep > scrollBody.getLastRendered()) { + debug("*** Server wants us to keep " + serverCacheFirst + "-" + + serverCacheLast + " but we only have rows " + + scrollBody.getFirstRendered() + "-" + + scrollBody.getLastRendered() + " rendered!"); + } + } + discardCacheRows(firstRowToKeep, lastRowToKeep); + + scrollBody.fixSpacers(); + + scrollBody.restoreRowVisibility(); + } + + private void discardCacheRows(int optimalFirstRow, int optimalLastRow) { + int firstDiscarded = -1, lastDiscarded = -1; boolean cont = true; while (cont && scrollBody.getLastRendered() > optimalFirstRow && scrollBody.getFirstRendered() < optimalFirstRow) { + if (firstDiscarded == -1) { + firstDiscarded = scrollBody.getFirstRendered(); + } + // removing row from start cont = scrollBody.unlinkRow(true); } - final int optimalLastRow = (int) (firstRowInViewPort + pageLength + pageLength - * cache_rate); + if (firstDiscarded != -1) { + lastDiscarded = scrollBody.getFirstRendered() - 1; + debug("Discarded rows " + firstDiscarded + "-" + lastDiscarded); + } + firstDiscarded = lastDiscarded = -1; + cont = true; while (cont && scrollBody.getLastRendered() > optimalLastRow) { + if (lastDiscarded == -1) { + lastDiscarded = scrollBody.getLastRendered(); + } + // removing row from the end cont = scrollBody.unlinkRow(false); } - scrollBody.fixSpacers(); + if (lastDiscarded != -1) { + firstDiscarded = scrollBody.getLastRendered() + 1; + debug("Discarded rows " + firstDiscarded + "-" + lastDiscarded); + } - scrollBody.restoreRowVisibility(); + debug("Now in cache: " + scrollBody.getFirstRendered() + "-" + + scrollBody.getLastRendered()); } /** @@ -6752,4 +6822,10 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler, lazyAdjustColumnWidths.schedule(LAZY_COLUMN_ADJUST_TIMEOUT); } } + + private void debug(String msg) { + if (enableDebug) { + VConsole.error(msg); + } + } } diff --git a/src/com/vaadin/ui/Table.java b/src/com/vaadin/ui/Table.java index 4f906ec11f..9107ec06ef 100644 --- a/src/com/vaadin/ui/Table.java +++ b/src/com/vaadin/ui/Table.java @@ -1488,36 +1488,77 @@ public class Table extends AbstractSelect implements Action.Container, private void removeRowsFromCacheAndFillBottom(int firstIndex, int rows) { int totalCachedRows = pageBuffer[CELL_ITEMID].length; int totalRows = size(); - int cacheIx = firstIndex - pageBufferFirstIndex; + int firstIndexInPageBuffer = firstIndex - pageBufferFirstIndex; + + /* + * firstIndexInPageBuffer is the first row to be removed. "rows" rows + * after that should be removed. If the page buffer does not contain + * that many rows, we only remove the rows that actually are in the page + * buffer. + */ + if (firstIndexInPageBuffer + rows > totalCachedRows) { + rows = totalCachedRows - firstIndexInPageBuffer; + } - // Make sure that no components leak. + /* + * Unregister components that will no longer be in the page buffer to + * make sure that no components leak. + */ unregisterComponentsAndPropertiesInRows(firstIndex, rows); - int newCachedRowCount = totalRows < totalCachedRows ? totalRows - : totalCachedRows; - int firstAppendedRow = newCachedRowCount > rows ? newCachedRowCount - - rows : firstIndex; - int rowsToAdd = Math.min(rows, totalCachedRows - firstAppendedRow); - rowsToAdd = Math.min(rowsToAdd, totalRows - - (firstAppendedRow + pageBufferFirstIndex)); - if (rowsToAdd <= 0) { - return; + /* + * The number of rows that should be in the cache after this operation + * is done (pageBuffer currently contains the expanded items). + */ + int newCachedRowCount = totalCachedRows; + if (newCachedRowCount + pageBufferFirstIndex > totalRows) { + newCachedRowCount = totalRows - pageBufferFirstIndex; } - Object[][] cells = getVisibleCellsNoCache(firstAppendedRow, rowsToAdd, - false); - // Create the new cache buffer by copying data from the old one and - // appending more rows if applicable. + /* + * The index at which we should render the first row that does not come + * from the previous page buffer. + */ + int firstAppendedRowInPageBuffer = totalCachedRows - rows; + int firstAppendedRow = firstAppendedRowInPageBuffer + + pageBufferFirstIndex; + + /* + * Calculate the maximum number of new rows that we can add to the page + * buffer. Less than the rows we removed if the container does not + * contain that many items afterwards. + */ + int maxRowsToRender = (totalRows - firstAppendedRow); + int rowsToAdd = rows; + if (rowsToAdd > maxRowsToRender) { + rowsToAdd = maxRowsToRender; + } + + Object[][] cells = null; + if (rowsToAdd > 0) { + cells = getVisibleCellsNoCache(firstAppendedRow, rowsToAdd, false); + } + /* + * Create the new cache buffer by copying the first rows from the old + * buffer, moving the following rows upwards and appending more rows if + * applicable. + */ Object[][] newPageBuffer = new Object[pageBuffer.length][newCachedRowCount]; - for (int ix = 0; ix < newCachedRowCount; ix++) { - for (int i = 0; i < pageBuffer.length; i++) { - if (ix >= firstAppendedRow) { - newPageBuffer[i][ix] = cells[i][ix - firstAppendedRow]; - } else if (ix >= cacheIx && ix + rows < totalCachedRows) { - newPageBuffer[i][ix] = pageBuffer[i][ix + rows]; - } else { - newPageBuffer[i][ix] = pageBuffer[i][ix]; - } + + for (int i = 0; i < pageBuffer.length; i++) { + for (int row = 0; row < firstIndexInPageBuffer; row++) { + // Copy the first rows + newPageBuffer[i][row] = pageBuffer[i][row]; + } + for (int row = firstIndexInPageBuffer; row < firstAppendedRowInPageBuffer; row++) { + // Move the rows that were after the expanded rows + newPageBuffer[i][row] = pageBuffer[i][row + rows]; + } + for (int row = firstAppendedRowInPageBuffer; row < newCachedRowCount; row++) { + // Add the newly rendered rows. Only used if rowsToAdd > 0 + // (cells != null) + newPageBuffer[i][row] = cells[i][row + - firstAppendedRowInPageBuffer]; } } pageBuffer = newPageBuffer; @@ -1537,23 +1578,84 @@ public class Table extends AbstractSelect implements Action.Container, return cells; } + /** + * @param firstIndex + * The position where new rows should be inserted + * @param rows + * The number of rows that should be inserted + * @param maxRows + * The maximum number of rows that + * @return + */ private Object[][] getVisibleCellsInsertIntoCache(int firstIndex, int rows) { Object[][] cells = getVisibleCellsNoCache(firstIndex, rows, false); + logger.finest("Insert " + rows + " rows at index " + firstIndex + + " to existing page buffer with " + cells.length + " items"); + + // Page buffer must not become larger than pageLength*cacheRate before + // or after the current page + int minPageBufferIndex = getCurrentPageFirstItemIndex() + - (int) (getPageLength() * getCacheRate()); + if (minPageBufferIndex < 0) { + minPageBufferIndex = 0; + } + + int maxPageBufferIndex = getCurrentPageFirstItemIndex() + + (int) (getPageLength() * (1 + getCacheRate())); + int maxBufferSize = maxPageBufferIndex - minPageBufferIndex; + + /* + * Number of rows that were previously cached. This is not necessarily + * the same as pageLength if we do not have enough rows in the + * container. + */ int currentlyCachedRowCount = pageBuffer[CELL_ITEMID].length; - int lastCachedRow = currentlyCachedRowCount - rows; - int cacheIx = firstIndex - pageBufferFirstIndex; - // Unregister all components that fall beyond the cache limits after - // inserting the new rows. - unregisterComponentsAndPropertiesInRows(lastCachedRow + 1, - currentlyCachedRowCount - lastCachedRow); + /* + * firstIndexInPageBuffer is the offset in pageBuffer where the new rows + * will be inserted (firstIndex is the index in the whole table). + * + * E.g. scrolled down to row 1000: firstIndex==1010, + * pageBufferFirstIndex==1000 -> cacheIx==10 + */ + int firstIndexInPageBuffer = firstIndex - pageBufferFirstIndex; + + /* + * "rows" rows will be inserted at firstIndex. Find out how many old + * rows fall outside the new buffer so we can unregister components in + * the cache. + */ + + /* All rows until the insertion point remain, always. */ + int firstCacheRowToRemoveInPageBuffer = firstIndexInPageBuffer; + + /* + * IF there is space remaining in the buffer after the rows have been + * inserted, we can keep more rows. + */ + int numberOfOldRowsAfterInsertedRows = maxBufferSize - firstIndex + - rows; + if (numberOfOldRowsAfterInsertedRows > 0) { + firstCacheRowToRemoveInPageBuffer += numberOfOldRowsAfterInsertedRows; + } + + if (firstCacheRowToRemoveInPageBuffer <= currentlyCachedRowCount) { + /* + * Unregister all components that fall beyond the cache limits after + * inserting the new rows. + */ + unregisterComponentsAndPropertiesInRows( + firstCacheRowToRemoveInPageBuffer + pageBufferFirstIndex, + currentlyCachedRowCount - firstCacheRowToRemoveInPageBuffer + + pageBufferFirstIndex); + } // Calculate the new cache size int newCachedRowCount = currentlyCachedRowCount; - if (pageLength == 0 || currentlyCachedRowCount < pageLength) { + if (maxBufferSize == 0 || currentlyCachedRowCount < maxBufferSize) { newCachedRowCount = currentlyCachedRowCount + rows; - if (pageLength > 0 && newCachedRowCount > pageLength) { - newCachedRowCount = pageLength; + if (maxBufferSize > 0 && newCachedRowCount > maxBufferSize) { + newCachedRowCount = maxBufferSize; } } @@ -1562,9 +1664,10 @@ public class Table extends AbstractSelect implements Action.Container, Object[][] newPageBuffer = new Object[pageBuffer.length][newCachedRowCount]; for (int ix = 0; ix < newCachedRowCount; ix++) { for (int i = 0; i < pageBuffer.length; i++) { - if (ix >= cacheIx && ix < cacheIx + rows) { - newPageBuffer[i][ix] = cells[i][ix - cacheIx]; - } else if (ix >= cacheIx + rows) { + if (ix >= firstIndexInPageBuffer + && ix < firstIndexInPageBuffer + rows) { + newPageBuffer[i][ix] = cells[i][ix - firstIndexInPageBuffer]; + } else if (ix >= firstIndexInPageBuffer + rows) { newPageBuffer[i][ix] = pageBuffer[i][ix - rows]; } else { newPageBuffer[i][ix] = pageBuffer[i][ix]; @@ -2638,7 +2741,12 @@ public class Table extends AbstractSelect implements Action.Container, target.startTag("prows"); - int maxRows = (int) (getPageLength() * getCacheRate()); + /* + * Caching says we should cache the current page and + * cacheRate*pageLength rows below it (and the same above). We only add + * rows below in this case. + */ + int maxRows = (int) (getPageLength() * (1 + getCacheRate())); if (!shouldHideAddedRows() && count > maxRows) { count = maxRows + 1; // delete the rows below, since they will fall beyond the cache -- 2.39.5