#6527 Container refactoring: move common parts of item removal to AbstractInMemoryContainer

svn changeset:17530/svn branch:6.6

3 files changed, 63 insertions, 25 deletions

diff --git a/src/com/vaadin/data/util/AbstractBeanContainer.java b/src/com/vaadin/data/util/AbstractBeanContainer.java
index 1c65003c9b..faeda2ad4c 100644
--- a/src/com/vaadin/data/util/AbstractBeanContainer.java
+++ b/src/com/vaadin/data/util/AbstractBeanContainer.java
@@ -260,14 +260,16 @@ public abstract class AbstractBeanContainer<IDTYPE, BEANTYPE> extends
      * @see com.vaadin.data.Container#removeAllItems()
      */
     public boolean removeAllItems() {
-        allItemIds.clear();
-        getFilteredItemIds().clear();
+        internalRemoveAllItems();
+
         // detach listeners from all Items
         for (Item item : itemIdToItem.values()) {
             removeAllValueChangeListeners(item);
         }
         itemIdToItem.clear();
+
         fireItemSetChange();
+
         return true;
     }
 
@@ -312,16 +314,21 @@ public abstract class AbstractBeanContainer<IDTYPE, BEANTYPE> extends
      * @see com.vaadin.data.Container#removeItem(java.lang.Object)
      */
     public boolean removeItem(Object itemId) {
-        if (!allItemIds.remove(itemId)) {
+        Item item = getItem(itemId);
+
+        if (internalRemoveItem(itemId)) {
+            // detach listeners from Item
+            removeAllValueChangeListeners(item);
+
+            // remove item
+            itemIdToItem.remove(itemId);
+
+            fireItemSetChange();
+
+            return true;
+        } else {
             return false;
         }
-        // detach listeners from Item
-        removeAllValueChangeListeners(getItem(itemId));
-        // remove item
-        itemIdToItem.remove(itemId);
-        getFilteredItemIds().remove(itemId);
-        fireItemSetChange();
-        return true;
     }
 
     /**
diff --git a/src/com/vaadin/data/util/AbstractInMemoryContainer.java b/src/com/vaadin/data/util/AbstractInMemoryContainer.java
index f83e50be04..63f90399d9 100644
--- a/src/com/vaadin/data/util/AbstractInMemoryContainer.java
+++ b/src/com/vaadin/data/util/AbstractInMemoryContainer.java
@@ -181,6 +181,46 @@ public abstract class AbstractInMemoryContainer<ITEMIDTYPE, PROPERTYIDCLASS, ITE
     // internal methods
 
     /**
+     * Removes all items from the internal data structures of this class. This
+     * can be used to implement {@link #removeAllItems()} in subclasses.
+     * 
+     * No notification is sent, the caller has to fire a suitable item set
+     * change notification.
+     */
+    protected void internalRemoveAllItems() {
+        // Removes all Items
+        allItemIds.clear();
+        if (getFilteredItemIds() != null) {
+            getFilteredItemIds().clear();
+        }
+    }
+
+    /**
+     * Removes a single item from the internal data structures of this class.
+     * This can be used to implement {@link #removeItem(Object)} in subclasses.
+     * 
+     * No notification is sent, the caller has to fire a suitable item set
+     * change notification.
+     * 
+     * @param itemId
+     *            the identifier of the item to remove
+     * @return true if an item was successfully removed, false if failed to
+     *         remove or no such item
+     */
+    protected boolean internalRemoveItem(Object itemId) {
+        if (itemId == null) {
+            return false;
+        }
+
+        boolean result = allItemIds.remove(itemId);
+        if (result && getFilteredItemIds() != null) {
+            getFilteredItemIds().remove(itemId);
+        }
+
+        return result;
+    }
+
+    /**
      * Returns the internal list of visible item identifiers after filtering.
      * 
      * For internal use only.
diff --git a/src/com/vaadin/data/util/IndexedContainer.java b/src/com/vaadin/data/util/IndexedContainer.java
index e424c294ca..fda93fe84d 100644
--- a/src/com/vaadin/data/util/IndexedContainer.java
+++ b/src/com/vaadin/data/util/IndexedContainer.java
@@ -225,13 +225,9 @@ public class IndexedContainer extends
      * @see com.vaadin.data.Container#removeAllItems()
      */
     public boolean removeAllItems() {
+        internalRemoveAllItems();
 
-        // Removes all Items
-        allItemIds.clear();
         items.clear();
-        if (getFilteredItemIds() != null) {
-            getFilteredItemIds().clear();
-        }
 
         // Sends a change event
         fireContentsChange(-1);
@@ -308,21 +304,16 @@ public class IndexedContainer extends
      * @see com.vaadin.data.Container#removeItem(java.lang.Object)
      */
     public boolean removeItem(Object itemId) {
-        if (itemId == null) {
+        if (itemId == null || items.remove(itemId) == null) {
             return false;
         }
+        if (internalRemoveItem(itemId)) {
+            fireContentsChange(-1);
 
-        if (items.remove(itemId) == null) {
+            return true;
+        } else {
             return false;
         }
-        allItemIds.remove(itemId);
-        if (getFilteredItemIds() != null) {
-            getFilteredItemIds().remove(itemId);
-        }
-
-        fireContentsChange(-1);
-
-        return true;
     }
 
     /*