diff options
| -rw-r--r-- | server/src/com/vaadin/data/Container.java | 54 | ||||
| -rw-r--r-- | server/src/com/vaadin/data/ContainerHelpers.java | 91 | ||||
| -rw-r--r-- | server/src/com/vaadin/data/RangeOutOfContainerBoundsException.java | 169 | ||||
| -rw-r--r-- | server/src/com/vaadin/data/util/AbstractInMemoryContainer.java | 40 | ||||
| -rw-r--r-- | server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java | 14 | ||||
| -rw-r--r-- | server/src/com/vaadin/ui/ComboBox.java | 62 | ||||
| -rw-r--r-- | server/src/com/vaadin/ui/Table.java | 277 | ||||
| -rw-r--r-- | server/tests/src/com/vaadin/data/util/TestIndexedContainer.java | 122 |
8 files changed, 675 insertions, 154 deletions
diff --git a/server/src/com/vaadin/data/Container.java b/server/src/com/vaadin/data/Container.java index 155dde87ef..28d6cad18d 100644 --- a/server/src/com/vaadin/data/Container.java +++ b/server/src/com/vaadin/data/Container.java @@ -18,6 +18,7 @@ package com.vaadin.data; import java.io.Serializable; import java.util.Collection; +import java.util.List; import com.vaadin.data.util.filter.SimpleStringFilter; import com.vaadin.data.util.filter.UnsupportedFilterException; @@ -484,16 +485,61 @@ public interface Container extends Serializable { public int indexOfId(Object itemId); /** - * Gets the ID of an Item by an index number. + * Get the item id for the item at the position given by + * <code>index</code>. <br> + * <br> + * <b>Throws:</b> {@link IndexOutOfBoundsException} if + * <code>index</code> is outside the range of the container. (i.e. + * <code>index < 0 || container.size()-1 < index</code>) * * @param index - * Index of the requested id in (the filtered and sorted view - * of) the Container - * @return ID of the Item in the given index + * the index of the requested item id + * @return the item id of the item at the given index */ public Object getIdByIndex(int index); /** + * Get <code>numberOfItems</code> consecutive item ids from the + * container, starting with the item id at <code>startIndex</code>. <br> + * <br> + * + * Implementations should return the exact number of item ids given by + * <code>numberOfItems</code>. The returned list must hence contain all + * of the item ids from the range: <br> + * <br> + * <code>startIndex</code> to + * <code>startIndex + (numberOfItems-1)</code>. <br> + * <br> + * + * The returned list must contain all of the requested item ids or throw + * a {@link RangeOutOfContainerBoundsException} to indicate that the + * container does not contain all the requested item ids.<br> + * <br> + * For quick migration to new API see: + * {@link ContainerHelpers#getItemIdsUsingGetIdByIndex(int, int, Indexed)} + * . <br> + * <br> + * <b>Throws:</b> {@link IllegalArgumentException} if + * <code>numberOfItems</code> is < 0 <br> + * <b>Throws:</b> {@link RangeOutOfContainerBoundsException} if all of + * the requested item ids cannot be fetched <br> + * <b>Throws:</b> {@link IndexOutOfBoundsException} if + * <code>startIndex</code> is outside the range of the container. (i.e. + * <code>startIndex < 0 || container.size()-1 < startIndex</code>) + * + * @param startIndex + * the index for the first item which id to include + * @param numberOfItems + * the number of consecutive item ids to get from the given + * start index, must be >= 0 + * @return List containing all of the requested item ids or empty list + * if <code>numberOfItems</code> == 0; not null + * + * @since 7.0 + */ + public List<?> getItemIds(int startIndex, int numberOfItems); + + /** * Adds a new item at given index (in the filtered view). * <p> * The indices of the item currently in the given position and all the diff --git a/server/src/com/vaadin/data/ContainerHelpers.java b/server/src/com/vaadin/data/ContainerHelpers.java new file mode 100644 index 0000000000..9ec2da4362 --- /dev/null +++ b/server/src/com/vaadin/data/ContainerHelpers.java @@ -0,0 +1,91 @@ +package com.vaadin.data; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import com.vaadin.data.Container.Indexed; + +/** + * Contains helper methods for containers that can be used to ease development + * of containers in Vaadin. + * + * @since 7.0 + */ +public class ContainerHelpers { + + /** + * Get a range of item ids from the container using + * {@link Indexed#getIdByIndex(int)}. This is just a helper method to aid + * developers to quickly add the required functionality to a Container + * during development. This should not be used in a "finished product" + * unless fetching an id for an index is very inexpensive because a separate + * request will be performed for each index in the range. + * + * @param startIndex + * index of the first item id to get + * @param numberOfIds + * the number of consecutive items whose ids should be returned + * @param container + * the container from which the items should be fetched + * @return A list of item ids in the range specified + */ + public static List<?> getItemIdsUsingGetIdByIndex(int startIndex, + int numberOfIds, Container.Indexed container) { + + if (container == null) { + throw new IllegalArgumentException( + "The given container cannot be null!"); + } + + if (startIndex < 0) { + throw new IndexOutOfBoundsException( + "Start index cannot be negative! startIndex=" + startIndex); + } + + if (startIndex > container.size()) { + throw new IndexOutOfBoundsException( + "Start index exceeds container size! startIndex=" + + startIndex + " containerLastItemIndex=" + + (container.size() - 1)); + } + + if (numberOfIds < 1) { + if (numberOfIds == 0) { + return Collections.emptyList(); + } + + throw new IllegalArgumentException( + "Cannot get negative amount of items! numberOfItems=" + + numberOfIds); + } + + // not included in the range + int endIndex = startIndex + numberOfIds; + + if (endIndex > container.size()) { + throw new RangeOutOfContainerBoundsException( + "Cannot get all requested item ids from container. " + + "Container size might have changed, recalculate numberOfIds " + + "based on the actual container size!", + startIndex, numberOfIds, container.size()); + } + + ArrayList<Object> rangeOfIds = new ArrayList<Object>(); + for (int i = startIndex; i < endIndex; i++) { + Object idByIndex = container.getIdByIndex(i); + if (idByIndex == null) { + throw new RuntimeException( + "Unable to get item id for index: " + + i + + " from container using Container.Indexed#getIdByIndex() " + + "even though container.size() > endIndex. " + + "Returned item id was null. " + + "Check your container implementation!"); + } + rangeOfIds.add(idByIndex); + } + + return Collections.unmodifiableList(rangeOfIds); + } +} diff --git a/server/src/com/vaadin/data/RangeOutOfContainerBoundsException.java b/server/src/com/vaadin/data/RangeOutOfContainerBoundsException.java new file mode 100644 index 0000000000..43058e45d8 --- /dev/null +++ b/server/src/com/vaadin/data/RangeOutOfContainerBoundsException.java @@ -0,0 +1,169 @@ +package com.vaadin.data; + +/** + * A exception that indicates that the container is unable to return all of the + * consecutive item ids requested by the caller. This can happen if the + * container size has changed since the input parameters for + * {@link Container.Indexed#getItemIds(int, int)} were computed or if the + * requested range exceeds the containers size due to some other factor.<br> + * <br> + * + * The exception can contain additional parameters for easier debugging. The + * additional parameters are the <code>startIndex</code> and + * <code>numberOfIds</code> which were given to + * {@link Container.Indexed#getItemIds(int, int)} as well as the size of the + * container when the fetch was executed. The container size can be retrieved + * with {@link #getContainerCurrentSize()}. <br> + * <br> + * + * The additional parameters are optional but the party that received the + * exception can check whether or not these were set by calling + * {@link #isAdditionalParametersSet()}. + * + * @since 7.0 + */ +public class RangeOutOfContainerBoundsException extends RuntimeException { + + private final int startIndex; + private final int numberOfIds; + private final int containerCurrentSize; + private final boolean additionalParametersSet; + + // Discourage users to create exceptions without at least some kind of + // message... + private RangeOutOfContainerBoundsException() { + super(); + startIndex = -1; + numberOfIds = -1; + containerCurrentSize = -1; + additionalParametersSet = false; + } + + public RangeOutOfContainerBoundsException(String message) { + super(message); + startIndex = -1; + numberOfIds = -1; + containerCurrentSize = -1; + additionalParametersSet = false; + } + + public RangeOutOfContainerBoundsException(String message, + Throwable throwable) { + super(message, throwable); + startIndex = -1; + numberOfIds = -1; + containerCurrentSize = -1; + additionalParametersSet = false; + } + + public RangeOutOfContainerBoundsException(Throwable throwable) { + super(throwable); + startIndex = -1; + numberOfIds = -1; + containerCurrentSize = -1; + additionalParametersSet = false; + } + + /** + * Create a new {@link RangeOutOfContainerBoundsException} with the + * additional parameters: + * <ul> + * <li>startIndex - start index for the query</li> + * <li>numberOfIds - the number of consecutive item ids to get</li> + * <li>containerCurrentSize - the size of the container during the execution + * of the query</li> + * </ul> + * given. + * + * @param message + * @param startIndex + * the given startIndex for the query + * @param numberOfIds + * the number of item ids requested + * @param containerCurrentSize + * the current size of the container + */ + public RangeOutOfContainerBoundsException(String message, int startIndex, + int numberOfIds, int containerCurrentSize) { + super(message); + + this.startIndex = startIndex; + this.numberOfIds = numberOfIds; + this.containerCurrentSize = containerCurrentSize; + additionalParametersSet = true; + } + + /** + * Create a new {@link RangeOutOfContainerBoundsException} with the given + * query parameters set in the exception along with the containers current + * size and a @link {@link Throwable}. + * + * @param message + * @param startIndex + * the given startIndex for the query + * @param numberOfIds + * the number of item ids queried for + * @param containerCurrentSize + * the current size of the container + * @param throwable + */ + public RangeOutOfContainerBoundsException(String message, int startIndex, + int numberOfIds, int containerCurrentSize, Throwable throwable) { + super(message, throwable); + + this.startIndex = startIndex; + this.numberOfIds = numberOfIds; + this.containerCurrentSize = containerCurrentSize; + additionalParametersSet = true; + } + + /** + * Get the given startIndex for the query. Remember to check if this + * parameter is set by calling {@link #isAdditionalParametersSet()} + * + * @return the startIndex given to the container + */ + public int getStartIndex() { + return startIndex; + } + + /** + * Get the number of item ids requested. Remember to check if this parameter + * is set with {@link #isAdditionalParametersSet()} + * + * @return the number of item ids the container was ordered to fetch + */ + public int getNumberOfIds() { + return numberOfIds; + } + + /** + * Get the container size when the query was actually executed. Remember to + * check if this parameter is set with {@link #isAdditionalParametersSet()} + */ + public int getContainerCurrentSize() { + return containerCurrentSize; + } + + /** + * Check whether or not the additional parameters for the exception were set + * during creation or not. + * + * The additional parameters can be retrieved with: <br> + * <ul> + * <li> {@link #getStartIndex()}</li> + * <li> {@link #getNumberOfIds()}</li> + * <li> {@link #getContainerCurrentSize()}</li> + * </ul> + * + * @return true if parameters are set, false otherwise. + * + * @see #RangeOutOfContainerBoundsException(String, int, int, int) + * RangeOutOfContainerBoundsException(String, int, int, int) for more + * details on the additional parameters + */ + public boolean isAdditionalParametersSet() { + return additionalParametersSet; + } + +} diff --git a/server/src/com/vaadin/data/util/AbstractInMemoryContainer.java b/server/src/com/vaadin/data/util/AbstractInMemoryContainer.java index 6eea9cb421..dbfcad3982 100644 --- a/server/src/com/vaadin/data/util/AbstractInMemoryContainer.java +++ b/server/src/com/vaadin/data/util/AbstractInMemoryContainer.java @@ -26,6 +26,7 @@ import java.util.Set; import com.vaadin.data.Container; import com.vaadin.data.Container.ItemSetChangeNotifier; import com.vaadin.data.Item; +import com.vaadin.data.RangeOutOfContainerBoundsException; import com.vaadin.data.util.filter.SimpleStringFilter; import com.vaadin.data.util.filter.UnsupportedFilterException; @@ -251,6 +252,45 @@ public abstract class AbstractInMemoryContainer<ITEMIDTYPE, PROPERTYIDCLASS, ITE } @Override + public List<ITEMIDTYPE> getItemIds(int startIndex, int numberOfIds) { + if (startIndex < 0) { + throw new IndexOutOfBoundsException( + "Start index cannot be negative! startIndex=" + startIndex); + } + + if (startIndex > getVisibleItemIds().size()) { + throw new IndexOutOfBoundsException( + "Start index exceeds container size! startIndex=" + + startIndex + " containerLastItemIndex=" + + (getVisibleItemIds().size() - 1)); + } + + if (numberOfIds < 1) { + if (numberOfIds == 0) { + return Collections.emptyList(); + } + + throw new IllegalArgumentException( + "Cannot get negative amount of items! numberOfItems=" + + numberOfIds); + } + + int endIndex = startIndex + numberOfIds; + + if (endIndex > getVisibleItemIds().size()) { + throw new RangeOutOfContainerBoundsException( + "Cannot get all requested item ids from container. " + + "Container size might have changed, recalculate numberOfIds " + + "based on the actual container size!", + startIndex, numberOfIds, getVisibleItemIds().size()); + } + + return Collections.unmodifiableList(getVisibleItemIds().subList( + startIndex, endIndex)); + + } + + @Override public int indexOfId(Object itemId) { return getVisibleItemIds().indexOf(itemId); } diff --git a/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java b/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java index a53f32b96e..7a63e8c6c2 100644 --- a/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java +++ b/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java @@ -33,6 +33,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import com.vaadin.data.Container; +import com.vaadin.data.ContainerHelpers; import com.vaadin.data.Item; import com.vaadin.data.Property; import com.vaadin.data.util.filter.Compare.Equal; @@ -656,9 +657,11 @@ public class SQLContainer implements Container, Container.Filterable, @Override public Object getIdByIndex(int index) { - if (index < 0 || index > size() - 1) { - return null; + if (index < 0) { + throw new IndexOutOfBoundsException("Index is negative! index=" + + index); } + if (index < size) { if (itemIndexes.keySet().contains(index)) { return itemIndexes.get(index); @@ -672,6 +675,13 @@ public class SQLContainer implements Container, Container.Filterable, } } + @Override + public List<Object> getItemIds(int startIndex, int numberOfIds) { + // TODO create a better implementation + return (List<Object>) ContainerHelpers.getItemIdsUsingGetIdByIndex( + startIndex, numberOfIds, this); + } + /**********************************************/ /** Methods from interface Container.Ordered **/ /**********************************************/ diff --git a/server/src/com/vaadin/ui/ComboBox.java b/server/src/com/vaadin/ui/ComboBox.java index da3d2fd91d..2555aac339 100644 --- a/server/src/com/vaadin/ui/ComboBox.java +++ b/server/src/com/vaadin/ui/ComboBox.java @@ -358,39 +358,45 @@ public class ComboBox extends AbstractSelect implements filterable.addContainerFilter(filter); } - Indexed indexed = (Indexed) container; + // try-finally to ensure that the filter is removed from container even + // if a exception is thrown... + try { + Indexed indexed = (Indexed) container; - int indexToEnsureInView = -1; + int indexToEnsureInView = -1; - // if not an option request (item list when user changes page), go - // to page with the selected item after filtering if accepted by - // filter - Object selection = getValue(); - if (isScrollToSelectedItem() && !optionRequest && selection != null) { - // ensure proper page - indexToEnsureInView = indexed.indexOfId(selection); - } + // if not an option request (item list when user changes page), go + // to page with the selected item after filtering if accepted by + // filter + Object selection = getValue(); + if (isScrollToSelectedItem() && !optionRequest && selection != null) { + // ensure proper page + indexToEnsureInView = indexed.indexOfId(selection); + } - filteredSize = container.size(); - currentPage = adjustCurrentPage(currentPage, needNullSelectOption, - indexToEnsureInView, filteredSize); - int first = getFirstItemIndexOnCurrentPage(needNullSelectOption, - filteredSize); - int last = getLastItemIndexOnCurrentPage(needNullSelectOption, - filteredSize, first); - - List<Object> options = new ArrayList<Object>(); - for (int i = first; i <= last && i < filteredSize; ++i) { - options.add(indexed.getIdByIndex(i)); - } + filteredSize = container.size(); + currentPage = adjustCurrentPage(currentPage, needNullSelectOption, + indexToEnsureInView, filteredSize); + int first = getFirstItemIndexOnCurrentPage(needNullSelectOption, + filteredSize); + int last = getLastItemIndexOnCurrentPage(needNullSelectOption, + filteredSize, first); - // to the outside, filtering should not be visible - if (filter != null) { - filterable.removeContainerFilter(filter); - filteringContainer = false; - } + // Compute the number of items to fetch from the indexes given or + // based on the filtered size of the container + int lastItemToFetch = Math.min(last, filteredSize - 1); + int nrOfItemsToFetch = (lastItemToFetch + 1) - first; + + List<?> options = indexed.getItemIds(first, nrOfItemsToFetch); - return options; + return options; + } finally { + // to the outside, filtering should not be visible + if (filter != null) { + filterable.removeContainerFilter(filter); + filteringContainer = false; + } + } } /** diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java index d1bdcdd708..5eb18ee61f 100644 --- a/server/src/com/vaadin/ui/Table.java +++ b/server/src/com/vaadin/ui/Table.java @@ -27,6 +27,7 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.StringTokenizer; @@ -1066,6 +1067,23 @@ public class Table extends AbstractSelect implements Action.Container, return currentPageFirstItemId; } + /** + * Returns the item ID for the item represented by the index given. Assumes + * that the current container implements {@link Container.Indexed}. + * + * See {@link Container.Indexed#getIdByIndex(int)} for more information + * about the exceptions that can be thrown. + * + * @param index + * the index for which the item ID should be fetched + * @return the item ID for the given index + * + * @throws ClassCastException + * if container does not implement {@link Container.Indexed} + * @throws IndexOutOfBoundsException + * thrown by {@link Container.Indexed#getIdByIndex(int)} if the + * index is invalid + */ protected Object getIdByIndex(int index) { return ((Container.Indexed) items).getIdByIndex(index); } @@ -1929,17 +1947,6 @@ public class Table extends AbstractSelect implements Action.Container, return cells; } - // Gets the first item id - Object id; - if (items instanceof Container.Indexed) { - id = getIdByIndex(firstIndex); - } else { - id = firstItemId(); - for (int i = 0; i < firstIndex; i++) { - id = nextItemId(id); - } - } - final RowHeaderMode headmode = getRowHeaderMode(); final boolean[] iscomponent = new boolean[cols]; for (int i = 0; i < cols; i++) { @@ -1956,120 +1963,41 @@ public class Table extends AbstractSelect implements Action.Container, // Creates the page contents int filledRows = 0; - for (int i = 0; i < rows && id != null; i++) { - cells[CELL_ITEMID][i] = id; - cells[CELL_KEY][i] = itemIdMapper.key(id); - if (headmode != ROW_HEADER_MODE_HIDDEN) { - switch (headmode) { - case INDEX: - cells[CELL_HEADER][i] = String.valueOf(i + firstIndex + 1); - break; - default: - cells[CELL_HEADER][i] = getItemCaption(id); - } - cells[CELL_ICON][i] = getItemIcon(id); + if (items instanceof Container.Indexed) { + // more efficient implementation for containers supporting access by + // index + + Container.Indexed indexed = ((Container.Indexed) items); + List<?> itemIds = indexed.getItemIds(firstIndex, rows); + for (int i = 0; i < rows && i < itemIds.size(); i++) { + Object id = itemIds.get(i); + // Start by parsing the values, id should already be set + parseItemIdToCells(cells, id, i, firstIndex, headmode, cols, + colids, firstIndexNotInCache, iscomponent, + oldListenedProperties); + + filledRows++; } + } else { + // slow back-up implementation for cases where the container does + // not support access by index - GeneratedRow generatedRow = rowGenerator != null ? rowGenerator - .generateRow(this, id) : null; - cells[CELL_GENERATED_ROW][i] = generatedRow; - - for (int j = 0; j < cols; j++) { - if (isColumnCollapsed(colids[j])) { - continue; - } - Property<?> p = null; - Object value = ""; - boolean isGeneratedRow = generatedRow != null; - boolean isGeneratedColumn = columnGenerators - .containsKey(colids[j]); - boolean isGenerated = isGeneratedRow || isGeneratedColumn; - - if (!isGenerated) { - p = getContainerProperty(id, colids[j]); - } - - if (isGeneratedRow) { - if (generatedRow.isSpanColumns() && j > 0) { - value = null; - } else if (generatedRow.isSpanColumns() && j == 0 - && generatedRow.getValue() instanceof Component) { - value = generatedRow.getValue(); - } else if (generatedRow.getText().length > j) { - value = generatedRow.getText()[j]; - } - } else { - // check in current pageBuffer already has row - int index = firstIndex + i; - if (p != null || isGenerated) { - int indexInOldBuffer = index - pageBufferFirstIndex; - if (index < firstIndexNotInCache - && index >= pageBufferFirstIndex - && pageBuffer[CELL_GENERATED_ROW][indexInOldBuffer] == null - && id.equals(pageBuffer[CELL_ITEMID][indexInOldBuffer])) { - // we already have data in our cache, - // recycle it instead of fetching it via - // getValue/getPropertyValue - value = pageBuffer[CELL_FIRSTCOL + j][indexInOldBuffer]; - if (!isGeneratedColumn && iscomponent[j] - || !(value instanceof Component)) { - listenProperty(p, oldListenedProperties); - } - } else { - if (isGeneratedColumn) { - ColumnGenerator cg = columnGenerators - .get(colids[j]); - value = cg.generateCell(this, id, colids[j]); - if (value != null - && !(value instanceof Component) - && !(value instanceof String)) { - // Avoid errors if a generator returns - // something - // other than a Component or a String - value = value.toString(); - } - } else if (iscomponent[j]) { - value = p.getValue(); - listenProperty(p, oldListenedProperties); - } else if (p != null) { - value = getPropertyValue(id, colids[j], p); - /* - * If returned value is Component (via - * fieldfactory or overridden getPropertyValue) - * we excpect it to listen property value - * changes. Otherwise if property emits value - * change events, table will start to listen - * them and refresh content when needed. - */ - if (!(value instanceof Component)) { - listenProperty(p, oldListenedProperties); - } - } else { - value = getPropertyValue(id, colids[j], null); - } - } - } - } - - if (value instanceof Component) { - registerComponent((Component) value); - } - cells[CELL_FIRSTCOL + j][i] = value; + // Gets the first item id + Object id = firstItemId(); + for (int i = 0; i < firstIndex; i++) { + id = nextItemId(id); } + for (int i = 0; i < rows && id != null; i++) { + // Start by parsing the values, id should already be set + parseItemIdToCells(cells, id, i, firstIndex, headmode, cols, + colids, firstIndexNotInCache, iscomponent, + oldListenedProperties); - // Gets the next item id - if (items instanceof Container.Indexed) { - int index = firstIndex + i + 1; - if (index < size()) { - id = getIdByIndex(index); - } else { - id = null; - } - } else { + // Gets the next item id for non indexed container id = nextItemId(id); - } - filledRows++; + filledRows++; + } } // Assures that all the rows of the cell-buffer are valid @@ -2089,6 +2017,117 @@ public class Table extends AbstractSelect implements Action.Container, return cells; } + /** + * Update a cache array for a row, register any relevant listeners etc. + * + * This is an internal method extracted from + * {@link #getVisibleCellsNoCache(int, int, boolean)} and should be removed + * when the Table is rewritten. + */ + private void parseItemIdToCells(Object[][] cells, Object id, int i, + int firstIndex, RowHeaderMode headmode, int cols, Object[] colids, + int firstIndexNotInCache, boolean[] iscomponent, + HashSet<Property<?>> oldListenedProperties) { + + cells[CELL_ITEMID][i] = id; + cells[CELL_KEY][i] = itemIdMapper.key(id); + if (headmode != ROW_HEADER_MODE_HIDDEN) { + switch (headmode) { + case INDEX: + cells[CELL_HEADER][i] = String.valueOf(i + firstIndex + 1); + break; + default: + cells[CELL_HEADER][i] = getItemCaption(id); + } + cells[CELL_ICON][i] = getItemIcon(id); + } + + GeneratedRow generatedRow = rowGenerator != null ? rowGenerator + .generateRow(this, id) : null; + cells[CELL_GENERATED_ROW][i] = generatedRow; + + for (int j = 0; j < cols; j++) { + if (isColumnCollapsed(colids[j])) { + continue; + } + Property<?> p = null; + Object value = ""; + boolean isGeneratedRow = generatedRow != null; + boolean isGeneratedColumn = columnGenerators.containsKey(colids[j]); + boolean isGenerated = isGeneratedRow || isGeneratedColumn; + + if (!isGenerated) { + p = getContainerProperty(id, colids[j]); + } + + if (isGeneratedRow) { + if (generatedRow.isSpanColumns() && j > 0) { + value = null; + } else if (generatedRow.isSpanColumns() && j == 0 + && generatedRow.getValue() instanceof Component) { + value = generatedRow.getValue(); + } else if (generatedRow.getText().length > j) { + value = generatedRow.getText()[j]; + } + } else { + // check in current pageBuffer already has row + int index = firstIndex + i; + if (p != null || isGenerated) { + int indexInOldBuffer = index - pageBufferFirstIndex; + if (index < firstIndexNotInCache + && index >= pageBufferFirstIndex + && pageBuffer[CELL_GENERATED_ROW][indexInOldBuffer] == null + && id.equals(pageBuffer[CELL_ITEMID][indexInOldBuffer])) { + // we already have data in our cache, + // recycle it instead of fetching it via + // getValue/getPropertyValue + value = pageBuffer[CELL_FIRSTCOL + j][indexInOldBuffer]; + if (!isGeneratedColumn && iscomponent[j] + || !(value instanceof Component)) { + listenProperty(p, oldListenedProperties); + } + } else { + if (isGeneratedColumn) { + ColumnGenerator cg = columnGenerators + .get(colids[j]); + value = cg.generateCell(this, id, colids[j]); + if (value != null && !(value instanceof Component) + && !(value instanceof String)) { + // Avoid errors if a generator returns + // something + // other than a Component or a String + value = value.toString(); + } + } else if (iscomponent[j]) { + value = p.getValue(); + listenProperty(p, oldListenedProperties); + } else if (p != null) { + value = getPropertyValue(id, colids[j], p); + /* + * If returned value is Component (via fieldfactory + * or overridden getPropertyValue) we expect it to + * listen property value changes. Otherwise if + * property emits value change events, table will + * start to listen them and refresh content when + * needed. + */ + if (!(value instanceof Component)) { + listenProperty(p, oldListenedProperties); + } + } else { + value = getPropertyValue(id, colids[j], null); + } + } + } + } + + if (value instanceof Component) { + registerComponent((Component) value); + } + cells[CELL_FIRSTCOL + j][i] = value; + } + } + protected void registerComponent(Component component) { getLogger().finest( "Registered " + component.getClass().getSimpleName() + ": " diff --git a/server/tests/src/com/vaadin/data/util/TestIndexedContainer.java b/server/tests/src/com/vaadin/data/util/TestIndexedContainer.java index 156ff83883..da2e2feac7 100644 --- a/server/tests/src/com/vaadin/data/util/TestIndexedContainer.java +++ b/server/tests/src/com/vaadin/data/util/TestIndexedContainer.java @@ -1,7 +1,9 @@ package com.vaadin.data.util; +import java.util.List; + import com.vaadin.data.Item; -import com.vaadin.data.util.IndexedContainer; +import com.vaadin.data.RangeOutOfContainerBoundsException; public class TestIndexedContainer extends AbstractInMemoryContainerTest { @@ -268,4 +270,122 @@ public class TestIndexedContainer extends AbstractInMemoryContainerTest { counter.assertNone(); } + // Ticket 8028 + public void testGetItemIdsRangeIndexOutOfBounds() { + IndexedContainer ic = new IndexedContainer(); + try { + ic.getItemIds(-1, 10); + fail("Container returned items starting from index -1, something very wrong here!"); + } catch (IndexOutOfBoundsException e) { + // This is expected... + } catch (Exception e) { + // Should not happen! + fail("Container threw unspecified exception when fetching a range of items and the range started from -1"); + } + + } + + // Ticket 8028 + public void testGetItemIdsRangeIndexOutOfBounds2() { + IndexedContainer ic = new IndexedContainer(); + ic.addItem(new Object()); + try { + ic.getItemIds(2, 1); + fail("Container returned items starting from index -1, something very wrong here!"); + } catch (IndexOutOfBoundsException e) { + // This is expected... + } catch (Exception e) { + // Should not happen! + fail("Container threw unspecified exception when fetching a out of bounds range of items"); + } + + } + + // Ticket 8028 + public void testGetItemIdsRangeZeroRange() { + IndexedContainer ic = new IndexedContainer(); + ic.addItem(new Object()); + try { + List<Object> itemIds = ic.getItemIds(1, 0); + + assertTrue( + "Container returned actual values when asking for 0 items...", + itemIds.isEmpty()); + } catch (Exception e) { + // Should not happen! + fail("Container threw unspecified exception when fetching 0 items..."); + } + + } + + // Ticket 8028 + public void testGetItemIdsRangeNegativeRange() { + IndexedContainer ic = new IndexedContainer(); + ic.addItem(new Object()); + try { + List<Object> itemIds = ic.getItemIds(1, -1); + + assertTrue( + "Container returned actual values when asking for -1 items...", + itemIds.isEmpty()); + } catch (IllegalArgumentException e) { + // this is expected + + } catch (Exception e) { + // Should not happen! + fail("Container threw unspecified exception when fetching -1 items..."); + } + + } + + // Ticket 8028 + public void testGetItemIdsRangeIndexOutOfBoundsDueToSizeChange() { + IndexedContainer ic = new IndexedContainer(); + ic.addItem(new Object()); + try { + ic.getItemIds(0, 10); + fail("Container returned items when the range was >> container size"); + } catch (RangeOutOfContainerBoundsException e) { + // This is expected... + assertTrue(e.isAdditionalParametersSet()); + assertEquals(0, e.getStartIndex()); + assertEquals(10, e.getNumberOfIds()); + assertEquals(1, e.getContainerCurrentSize()); + + } catch (IndexOutOfBoundsException e) { + fail("Container threw wrong exception when the range exceeded container size... "); + } catch (Exception e) { + // Should not happen! + fail("Container threw unspecified exception when fetching a range of items and the range started from -1"); + } + } + + // Ticket 8028 + public void testGetItemIdsRangeBaseCase() { + IndexedContainer ic = new IndexedContainer(); + String object1 = new String("Obj1"); + String object2 = new String("Obj2"); + String object3 = new String("Obj3"); + String object4 = new String("Obj4"); + String object5 = new String("Obj5"); + + ic.addItem(object1); + ic.addItem(object2); + ic.addItem(object3); + ic.addItem(object4); + ic.addItem(object5); + + try { + List<Object> itemIds = ic.getItemIds(1, 2); + + assertTrue(itemIds.contains(object2)); + assertTrue(itemIds.contains(object3)); + assertEquals(2, itemIds.size()); + + } catch (Exception e) { + // Should not happen! + fail("Container threw exception when fetching a range of items "); + } + } + } |