Add Container.Indexed.getItemIds(int, int) for a range of items (#8028)

This permits optimization of data fetches from various containers
where getting single items by index in a loop might be costly.

Also add a helper method in ContainerHelpers to make it easier to
implement the new method where performance of fetching an id by index is
not an issue. SQLContainer still uses this helper instead of an
optimized implementation.

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 &lt; 0 || container.size()-1 &lt; 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 &lt; 0 || container.size()-1 &lt; 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 ");
+        }
+    }
+
 }