From cc5f33115bbf6912477839974bfcc4ba07edb174 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Thu, 3 Mar 2011 14:49:53 +0000 Subject: [PATCH] #6527 Container refactoring: move most of in-memory container filtering code to superclass svn changeset:17580/svn branch:6.6 --- .../data/util/AbstractBeanContainer.java | 118 ++------- .../data/util/AbstractInMemoryContainer.java | 227 ++++++++++++++++-- .../vaadin/data/util/IndexedContainer.java | 190 ++------------- 3 files changed, 251 insertions(+), 284 deletions(-) diff --git a/src/com/vaadin/data/util/AbstractBeanContainer.java b/src/com/vaadin/data/util/AbstractBeanContainer.java index abca10c22b..56e93733a8 100644 --- a/src/com/vaadin/data/util/AbstractBeanContainer.java +++ b/src/com/vaadin/data/util/AbstractBeanContainer.java @@ -11,13 +11,9 @@ import java.lang.reflect.Method; import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedList; -import java.util.List; import java.util.Map; -import java.util.Set; import com.vaadin.data.Container.Filterable; import com.vaadin.data.Container.Sortable; @@ -37,12 +33,10 @@ import com.vaadin.data.Property.ValueChangeNotifier; *

* *

- * Subclasses should implement adding items to the container, typically calling - * the protected methods {@link #addItem(Object, Object)}, + * Subclasses should implement any public methods adding items to the container, + * typically calling the protected methods {@link #addItem(Object, Object)}, * {@link #addItemAfter(Object, Object, Object)} and - * {@link #addItemAt(int, Object, Object)}, and if necessary, - * {@link #internalAddAt(int, Object, Object)} and - * {@link #internalIndexOf(Object)}. + * {@link #addItemAt(int, Object, Object)}. *

* *

@@ -144,11 +138,6 @@ public abstract class AbstractBeanContainer extends */ private ItemSorter itemSorter = new DefaultItemSorter(); - /** - * Filters currently applied to the container. - */ - private Set filters = new HashSet(); - /** * Maps all item ids in the container (including filtered) to their * corresponding BeanItem. @@ -176,7 +165,6 @@ public abstract class AbstractBeanContainer extends */ protected AbstractBeanContainer(Class type) { super(new ListSet()); - setFilteredItemIds(new ListSet()); if (type == null) { throw new IllegalArgumentException( "The bean type passed to AbstractBeanContainer must not be null"); @@ -284,6 +272,12 @@ public abstract class AbstractBeanContainer extends */ @Override public BeanItem getItem(Object itemId) { + // TODO return only if visible? + return getUnfilteredItem(itemId); + } + + @Override + protected BeanItem getUnfilteredItem(Object itemId) { return itemIdToItem.get(itemId); } @@ -347,51 +341,6 @@ public abstract class AbstractBeanContainer extends filterAll(); } - @Override - @SuppressWarnings("unchecked") - protected void filterAll() { - // avoid notification if the filtering had no effect - List originalItems = getFilteredItemIds(); - // it is somewhat inefficient to do a (shallow) clone() every time - setFilteredItemIds((List) ((ListSet) allItemIds) - .clone()); - for (Filter f : filters) { - filter(f); - } - // check if exactly the same items are there after filtering to avoid - // unnecessary notifications - // this may be slow in some cases as it uses BEANTYPE.equals() - if (!originalItems.equals(getFilteredItemIds())) { - fireItemSetChange(); - } - } - - /** - * Returns an unmodifiable collection of filters in use by the container. - * - * @return - */ - protected Set getFilters() { - return Collections.unmodifiableSet(filters); - } - - /** - * Remove (from the filtered list) any items that do not match the given - * filter. - * - * @param f - * The filter used to determine if items should be removed - */ - protected void filter(Filter f) { - Iterator iterator = getFilteredItemIds().iterator(); - while (iterator.hasNext()) { - IDTYPE itemId = iterator.next(); - if (!f.passesFilter(getItem(itemId))) { - iterator.remove(); - } - } - } - /* * (non-Javadoc) * @@ -399,22 +348,10 @@ public abstract class AbstractBeanContainer extends * com.vaadin.data.Container.Filterable#addContainerFilter(java.lang.Object, * java.lang.String, boolean, boolean) */ - @SuppressWarnings("unchecked") public void addContainerFilter(Object propertyId, String filterString, boolean ignoreCase, boolean onlyMatchPrefix) { - if (filters.isEmpty()) { - setFilteredItemIds((List) ((ListSet) allItemIds) - .clone()); - } - // listen to change events to be able to update filtering - for (Item item : itemIdToItem.values()) { - addValueChangeListener(item, propertyId); - } - Filter f = new Filter(propertyId, filterString, ignoreCase, - onlyMatchPrefix); - filter(f); - filters.add(f); - fireItemSetChange(); + addFilter(new Filter(propertyId, filterString, ignoreCase, + onlyMatchPrefix)); } /* @@ -423,13 +360,11 @@ public abstract class AbstractBeanContainer extends * @see com.vaadin.data.Container.Filterable#removeAllContainerFilters() */ public void removeAllContainerFilters() { - if (!filters.isEmpty()) { - filters = new HashSet(); - // stop listening to change events for any property + if (!getFilters().isEmpty()) { for (Item item : itemIdToItem.values()) { removeAllValueChangeListeners(item); } - filterAll(); + removeAllFilters(); } } @@ -441,22 +376,11 @@ public abstract class AbstractBeanContainer extends * .Object) */ public void removeContainerFilters(Object propertyId) { - if (!filters.isEmpty()) { - boolean filteringChanged = false; - for (Iterator iterator = filters.iterator(); iterator - .hasNext();) { - Filter f = iterator.next(); - if (f.propertyId.equals(propertyId)) { - iterator.remove(); - filteringChanged = true; - } - } - if (filteringChanged) { - // stop listening to change events for the property - for (Item item : itemIdToItem.values()) { - removeValueChangeListener(item, propertyId); - } - filterAll(); + Collection removedFilters = super.removeFilters(propertyId); + if (!removedFilters.isEmpty()) { + // stop listening to change events for the property + for (Item item : itemIdToItem.values()) { + removeValueChangeListener(item, propertyId); } } } @@ -627,9 +551,9 @@ public abstract class AbstractBeanContainer extends /** * Adds the bean to the Container. * - * TODO Note: the behavior of this method changed in Vaadin 6.6 - now items - * are added at the very end of the unfiltered container and not after the - * last visible item if filtering is used. + * Note: the behavior of this method changed in Vaadin 6.6 - now items are + * added at the very end of the unfiltered container and not after the last + * visible item if filtering is used. * * @see com.vaadin.data.Container#addItem(Object) */ diff --git a/src/com/vaadin/data/util/AbstractInMemoryContainer.java b/src/com/vaadin/data/util/AbstractInMemoryContainer.java index ddb4ca7115..32ac3a70be 100644 --- a/src/com/vaadin/data/util/AbstractInMemoryContainer.java +++ b/src/com/vaadin/data/util/AbstractInMemoryContainer.java @@ -2,7 +2,11 @@ package com.vaadin.data.util; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; import java.util.List; +import java.util.Set; import com.vaadin.data.Container; import com.vaadin.data.Container.ItemSetChangeNotifier; @@ -16,14 +20,12 @@ import com.vaadin.data.Item; * * TODO this version does not implement {@link Container.Sortable} * - * TODO this version does not implement {@link Container.Filterable} - * - * TODO this version does not implement container modification methods - * * Features: *

* * @param @@ -61,6 +63,12 @@ public abstract class AbstractInMemoryContainer filteredItemIds; + /** + * Filters that are applied to the container to limit the items visible in + * it + */ + private Set filters = new HashSet(); + // Constructors /** @@ -78,6 +86,16 @@ public abstract class AbstractInMemoryContainer cannot // be cast to Collection @@ -180,12 +198,160 @@ public abstract class AbstractInMemoryContainer originalFilteredItemIds = getFilteredItemIds(); + if (originalFilteredItemIds == null) { + originalFilteredItemIds = Collections.emptyList(); + } + setFilteredItemIds(new ListSet()); + + // Filter + boolean equal = true; + Iterator origIt = originalFilteredItemIds.iterator(); + for (final Iterator i = allItemIds.iterator(); i.hasNext();) { + final ITEMIDTYPE id = i.next(); + if (passesFilters(id)) { + // filtered list comes from the full list, can use == + equal = equal && origIt.hasNext() && origIt.next() == id; + getFilteredItemIds().add(id); + } + } + + return !equal || origIt.hasNext(); + } + + /** + * Checks if the given itemId passes the filters set for the container. The + * caller should make sure the itemId exists in the container. For + * non-existing itemIds the behavior is undefined. + * + * @param itemId + * An itemId that exists in the container. + * @return true if the itemId passes all filters or no filters are set, + * false otherwise. + */ + protected boolean passesFilters(Object itemId) { + ITEMCLASS item = getUnfilteredItem(itemId); + if (getFilters().isEmpty()) { + return true; + } + final Iterator i = getFilters().iterator(); + while (i.hasNext()) { + final Filter f = i.next(); + if (!f.passesFilter(item)) { + return false; + } + } + return true; + } + + /** + * Add a container filter and re-filter the view + * + * This can be used to implement + * {@link Filterable#addContainerFilter(Object, String, boolean, boolean)}. + */ + protected void addFilter(Filter filter) { + getFilters().add(filter); + filterAll(); + } + + /** + * Remove all container filters for all properties and re-filter the view. + * + * This can be used to implement + * {@link Filterable#removeAllContainerFilters()}. + */ + protected void removeAllFilters() { + if (getFilters().isEmpty()) { + return; + } + getFilters().clear(); + filterAll(); + } + + /** + * Checks if there is a filter that applies to a given property. + * + * @param propertyId + * @return true if there is an active filter for the property + */ + protected boolean isPropertyFiltered(Object propertyId) { + if (getFilters().isEmpty() || propertyId == null) { + return false; + } + final Iterator i = getFilters().iterator(); + while (i.hasNext()) { + final Filter f = i.next(); + if (propertyId.equals(f.propertyId)) { + return true; + } + } + return false; + } + + /** + * Remove all container filters for a given property identifier and + * re-filter the view. + * + * This can be used to implement + * {@link Filterable#removeContainerFilters(Object)}. + * + * @param propertyId + * @return Collection removed filters + */ + protected Collection removeFilters(Object propertyId) { + if (getFilters().isEmpty() || propertyId == null) { + return Collections.emptyList(); + } + List removedFilters = new LinkedList(); + for (Iterator iterator = getFilters().iterator(); iterator + .hasNext();) { + Filter f = iterator.next(); + if (f.propertyId.equals(propertyId)) { + removedFilters.add(f); + iterator.remove(); + } + } + if (!removedFilters.isEmpty()) { + filterAll(); + return removedFilters; + } + return Collections.emptyList(); + } + + // removing items /** * Removes all items from the internal data structures of this class. This @@ -227,6 +393,8 @@ public abstract class AbstractInMemoryContainer filters) { + this.filters = filters; + } + + /** + * TODO Temporary internal helper method to get the internal list of + * filters. + * + * @return Set + */ + protected Set getFilters() { + return filters; + } + } diff --git a/src/com/vaadin/data/util/IndexedContainer.java b/src/com/vaadin/data/util/IndexedContainer.java index e3b3a9a4bb..62ace0f80d 100644 --- a/src/com/vaadin/data/util/IndexedContainer.java +++ b/src/com/vaadin/data/util/IndexedContainer.java @@ -94,12 +94,6 @@ public class IndexedContainer extends */ private ItemSorter itemSorter = new DefaultItemSorter(); - /** - * Filters that are applied to the container to limit the items visible in - * it - */ - private HashSet filters; - private HashMap defaultPropertyValues; private int nextGeneratedItemId = 1; @@ -129,8 +123,15 @@ public class IndexedContainer extends @Override public Item getItem(Object itemId) { - if (itemId != null && items.containsKey(itemId) - && (getVisibleItemIds().contains(itemId))) { + if (itemId != null && getVisibleItemIds().contains(itemId)) { + return getUnfilteredItem(itemId); + } + return null; + } + + @Override + protected Item getUnfilteredItem(Object itemId) { + if (itemId != null && items.containsKey(itemId)) { return new IndexedContainerItem(itemId); } return null; @@ -255,26 +256,8 @@ public class IndexedContainer extends * @see com.vaadin.data.Container#addItem(java.lang.Object) */ public Item addItem(Object itemId) { - // TODO move filtering to superclass? - Item item = internalAddItemAtEnd(itemId, new IndexedContainerItem( - itemId), false); - if (item == null) { - return null; - } - - // optimization, complicates code - if (getFilteredItemIds() != null) { - if (passesFilters(itemId)) { - getFilteredItemIds().add(itemId); - // Send the event - fireItemAdded(size() - 1, itemId, item); - } - } else { - // Send the event - fireItemAdded(size() - 1, itemId, item); - } - - return item; + return internalAddItemAtEnd(itemId, new IndexedContainerItem(itemId), + true); } /** @@ -352,17 +335,8 @@ public class IndexedContainer extends * java.lang.Object) */ public Item addItemAfter(Object previousItemId, Object newItemId) { - Item item = internalAddItemAfter(previousItemId, newItemId, + return internalAddItemAfter(previousItemId, newItemId, new IndexedContainerItem(newItemId)); - if (item != null && getFilteredItemIds() == null) { - // in this case, not fired by filterAll() - // TODO now a little less efficient, new scan in list - int position = indexOfId(newItemId); - if (position >= 0) { - fireItemAdded(position, newItemId, item); - } - } - return item; } /* @@ -388,17 +362,8 @@ public class IndexedContainer extends * @see com.vaadin.data.Container.Indexed#addItemAt(int, java.lang.Object) */ public Item addItemAt(int index, Object newItemId) { - Item item = internalAddItemAt(index, newItemId, - new IndexedContainerItem(newItemId)); - if (item != null && getFilteredItemIds() == null) { - // in this case, not fired by filterAll() - // TODO now a little less efficient, new scan in list - int position = indexOfId(newItemId); - if (position >= 0) { - fireItemAdded(position, newItemId, item); - } - } - return item; + return internalAddItemAt(index, newItemId, new IndexedContainerItem( + newItemId)); } /* @@ -908,7 +873,9 @@ public class IndexedContainer extends } // update the container filtering if this property is being filtered - updateContainerFiltering(propertyId); + if (isPropertyFiltered(propertyId)) { + filterAll(); + } firePropertyValueChange(this); } @@ -1102,7 +1069,8 @@ public class IndexedContainer extends nc.types = types != null ? (Hashtable>) types.clone() : null; - nc.filters = filters == null ? null : (HashSet) filters.clone(); + nc.setFilters((HashSet) ((HashSet) getFilters()) + .clone()); nc.setFilteredItemIds(getFilteredItemIds() == null ? null : (ListSet) ((ListSet) getFilteredItemIds()) @@ -1126,130 +1094,16 @@ public class IndexedContainer extends public void addContainerFilter(Object propertyId, String filterString, boolean ignoreCase, boolean onlyMatchPrefix) { - if (filters == null) { - filters = new HashSet(); - } - filters.add(new Filter(propertyId, filterString, ignoreCase, + addFilter(new Filter(propertyId, filterString, ignoreCase, onlyMatchPrefix)); - filterAll(); } public void removeAllContainerFilters() { - if (filters == null) { - return; - } - filters.clear(); - filterAll(); + removeAllFilters(); } public void removeContainerFilters(Object propertyId) { - if (filters == null || propertyId == null) { - return; - } - final Iterator i = filters.iterator(); - while (i.hasNext()) { - final Filter f = i.next(); - if (propertyId.equals(f.propertyId)) { - i.remove(); - } - } - filterAll(); - } - - private void updateContainerFiltering(Object propertyId) { - if (filters == null || propertyId == null) { - return; - } - // update container filtering if there is a filter for the given - // property - final Iterator i = filters.iterator(); - while (i.hasNext()) { - final Filter f = i.next(); - if (propertyId.equals(f.propertyId)) { - filterAll(); - return; - } - } - } - - @Override - protected void filterAll() { - - // Clearing filters? - boolean hasFilters = (filters != null && !filters.isEmpty()); - - if (doFilterContainer(hasFilters)) { - fireItemSetChange(); - } - } - - /** - * Filters the data in the container and updates internal data structures. - * This method should reset any internal data structures and then repopulate - * them so {@link #getItemIds()} and other methods only return the filtered - * items. - * - * @param hasFilters - * true if filters has been set for the container, false - * otherwise - * @return true if the item set has changed as a result of the filtering - */ - protected boolean doFilterContainer(boolean hasFilters) { - if (!hasFilters) { - setFilteredItemIds(null); - if (filters != null) { - filters = null; - return true; - } - - return false; - } - - // Reset filtered list - List originalFilteredItemIds = getFilteredItemIds(); - if (originalFilteredItemIds == null) { - originalFilteredItemIds = Collections.emptyList(); - } - setFilteredItemIds(new ListSet()); - - // Filter - boolean equal = true; - Iterator origIt = originalFilteredItemIds.iterator(); - for (final Iterator i = allItemIds.iterator(); i.hasNext();) { - final Object id = i.next(); - if (passesFilters(id)) { - // filtered list comes from the full list, can use == - equal = equal && origIt.hasNext() && origIt.next() == id; - getFilteredItemIds().add(id); - } - } - - return !equal || origIt.hasNext(); - } - - /** - * Checks if the given itemId passes the filters set for the container. The - * caller should make sure the itemId exists in the container. For - * non-existing itemIds the behavior is undefined. - * - * @param itemId - * An itemId that exists in the container. - * @return true if the itemId passes all filters or no filters are set, - * false otherwise. - */ - protected boolean passesFilters(Object itemId) { - IndexedContainerItem item = new IndexedContainerItem(itemId); - if (filters == null) { - return true; - } - final Iterator i = filters.iterator(); - while (i.hasNext()) { - final Filter f = i.next(); - if (!f.passesFilter(item)) { - return false; - } - } - return true; + removeFilters(propertyId); } } \ No newline at end of file -- 2.39.5