From caf9376c8f7535c31477516048799d5dc1981090 Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Wed, 1 Apr 2009 11:51:38 +0000 Subject: [PATCH] #1061: merge to 6.0 of BeanItemContainer filtering fixes and other improvements, interactive test class svn changeset:7269/svn branch:6.0 --- src/com/itmill/toolkit/data/Container.java | 17 +- .../itmill/toolkit/data/util/BeanItem.java | 7 +- .../toolkit/data/util/BeanItemContainer.java | 228 ++++++++++++++---- .../BeanItemContainerFilteringTest.java | 172 +++++++++++++ .../containers/BeanItemContainerTest.java | 2 +- 5 files changed, 370 insertions(+), 56 deletions(-) create mode 100644 src/com/itmill/toolkit/tests/containers/BeanItemContainerFilteringTest.java diff --git a/src/com/itmill/toolkit/data/Container.java b/src/com/itmill/toolkit/data/Container.java index 36b0caca93..5df1b34f91 100644 --- a/src/com/itmill/toolkit/data/Container.java +++ b/src/com/itmill/toolkit/data/Container.java @@ -543,8 +543,8 @@ public interface Container { } /** - * Interface is implemented by containers that allow reducing their - * visiblecontents with set of filters. + * Interface is implemented by containers that allow reducing their visible + * contents with set of filters. * * When a set of filters are set, only items that match the filters are * included in the visible contents of the container. Still new items that @@ -553,6 +553,19 @@ public interface Container { * multiple filters are added, all filters must match for an item to be * visible in the container. * + * When an {@link com.itmill.toolkit.data.Ordered} or + * {@link com.itmill.toolkit.data.Indexed} container is filtered, all + * operations of these interfaces should only use the filtered contents and + * the filtered indices to the container. + * + * Adding items (if supported) to a filtered + * {@link com.itmill.toolkit.data.Ordered} or + * {@link com.itmill.toolkit.data.Indexed} container should insert them + * immediately after the indicated visible item. The unfiltered position of + * items added at index 0, at index + * {@link com.itmill.toolkit.data.Container#size()} or at an undefined + * position is up to the implementation. + * * @since 5.0 */ public interface Filterable extends Container { diff --git a/src/com/itmill/toolkit/data/util/BeanItem.java b/src/com/itmill/toolkit/data/util/BeanItem.java index f30d5fd820..b867491cdb 100644 --- a/src/com/itmill/toolkit/data/util/BeanItem.java +++ b/src/com/itmill/toolkit/data/util/BeanItem.java @@ -60,7 +60,7 @@ public class BeanItem extends PropertysetItem { for (int i = 0; i < pd.length; i++) { final Method getMethod = pd[i].getReadMethod(); final Method setMethod = pd[i].getWriteMethod(); - final Class type = pd[i].getPropertyType(); + final Class type = pd[i].getPropertyType(); final String name = pd[i].getName(); if ((getMethod != null) @@ -103,14 +103,15 @@ public class BeanItem extends PropertysetItem { final PropertyDescriptor[] pd = info.getPropertyDescriptors(); // Add all the bean properties as MethodProperties to this Item - for (final Iterator iter = propertyIds.iterator(); iter.hasNext();) { + final Iterator iter = propertyIds.iterator(); + while (iter.hasNext()) { final Object id = iter.next(); for (int i = 0; i < pd.length; i++) { final String name = pd[i].getName(); if (name.equals(id)) { final Method getMethod = pd[i].getReadMethod(); final Method setMethod = pd[i].getWriteMethod(); - final Class type = pd[i].getPropertyType(); + final Class type = pd[i].getPropertyType(); if ((getMethod != null)) { final Property p = new MethodProperty(type, bean, getMethod, setMethod); diff --git a/src/com/itmill/toolkit/data/util/BeanItemContainer.java b/src/com/itmill/toolkit/data/util/BeanItemContainer.java index 24b16573fc..7fa747372d 100644 --- a/src/com/itmill/toolkit/data/util/BeanItemContainer.java +++ b/src/com/itmill/toolkit/data/util/BeanItemContainer.java @@ -8,6 +8,9 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Set; import com.itmill.toolkit.data.Container; import com.itmill.toolkit.data.Item; @@ -16,23 +19,33 @@ import com.itmill.toolkit.data.Container.Filterable; import com.itmill.toolkit.data.Container.Indexed; import com.itmill.toolkit.data.Container.ItemSetChangeNotifier; import com.itmill.toolkit.data.Container.Sortable; +import com.itmill.toolkit.data.Property.ValueChangeEvent; +import com.itmill.toolkit.data.Property.ValueChangeListener; +import com.itmill.toolkit.data.Property.ValueChangeNotifier; /** * An {@link ArrayList} backed container for {@link BeanItem}s. *

- * Bean objects act as identifiers. + * Bean objects act as identifiers. For this reason, they should implement + * Object.equals(Object) and Object.hashCode() . * * @param + * + * @since 5.4 */ public class BeanItemContainer implements Indexed, Sortable, Filterable, - ItemSetChangeNotifier { + ItemSetChangeNotifier, ValueChangeListener { + // filtered and unfiltered item IDs private ArrayList list = new ArrayList(); - private final HashMap beanToItem = new HashMap(); + private ArrayList allItems = new ArrayList(); + private final Map beanToItem = new HashMap(); + private final Class type; private final BeanItem model; - private LinkedList itemSetChangeListeners; - private ArrayList allItems; - private HashSet filters; + + private List itemSetChangeListeners; + + private Set filters = new HashSet(); public BeanItemContainer(Class type) throws InstantiationException, IllegalAccessException { @@ -65,15 +78,52 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, throw new UnsupportedOperationException(); } - @SuppressWarnings("unchecked") public Item addItemAt(int index, Object newItemId) throws UnsupportedOperationException { + if (index < 0 || index > size()) { + return null; + } else if (index == 0) { + // add before any item, visible or not + return addItemAtInternalIndex(0, newItemId); + } else { + // if index==size(), adds immediately after last visible item + return addItemAfter(getIdByIndex(index - 1), newItemId); + } + } + + /** + * Adds new item at given index of the internal (unfiltered) list. + *

+ * The item is also added in the visible part of the list if it passes the + * filters. + *

+ * + * @param index + * Internal index to add the new item. + * @param newItemId + * Id of the new item to be added. + * @return Returns new item or null if the operation fails. + */ + @SuppressWarnings("unchecked") + private Item addItemAtInternalIndex(int index, Object newItemId) { + // Make sure that the Item has not been created yet + if (allItems.contains(newItemId)) { + return null; + } if (newItemId.getClass().isAssignableFrom(type)) { BT pojo = (BT) newItemId; - list.add(index, pojo); + // "list" will be updated in filterAll() + allItems.add(index, pojo); BeanItem beanItem = new BeanItem(pojo); beanToItem.put(pojo, beanItem); - fireItemSetChange(); + // add listeners to be able to update filtering on property changes + for (Filter filter : filters) { + // addValueChangeListener avoids adding duplicates + addValueChangeListener(beanItem, filter.propertyId); + } + + // it is somewhat suboptimal to filter all items + filterAll(); return beanItem; } else { return null; @@ -95,15 +145,21 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, public Item addItemAfter(Object previousItemId, Object newItemId) throws UnsupportedOperationException { - int index = indexOfId(previousItemId) + 1; - if (index > 0) { - addItemAt(index, newItemId); + // only add if the previous item is visible + if (list.contains(previousItemId)) { + return addItemAtInternalIndex(allItems.indexOf(previousItemId) + 1, + newItemId); + } else { + return null; } - return null; } public Object firstItemId() { - return list.iterator().next(); + if (list.size() > 0) { + return list.get(0); + } else { + return null; + } } public boolean isFirstId(Object itemId) { @@ -115,26 +171,28 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, } public Object lastItemId() { - try { + if (list.size() > 0) { return list.get(list.size() - 1); - } catch (ArrayIndexOutOfBoundsException e) { + } else { return null; } } public Object nextItemId(Object itemId) { - try { - return list.get(list.indexOf(itemId) + 1); - } catch (Exception e) { + int index = list.indexOf(itemId); + if (index >= 0 && index < list.size() - 1) { + return list.get(index + 1); + } else { // out of bounds return null; } } public Object prevItemId(Object itemId) { - try { - return list.get(list.indexOf(itemId) - 1); - } catch (Exception e) { + int index = list.indexOf(itemId); + if (index > 0) { + return list.get(index - 1); + } else { // out of bounds return null; } @@ -151,14 +209,17 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, } public Item addItem(Object itemId) throws UnsupportedOperationException { - if (list.isEmpty()) { - return addItemAt(0, itemId); + if (list.size() > 0) { + // add immediately after last visible item + int lastIndex = allItems.indexOf(lastItemId()); + return addItemAtInternalIndex(lastIndex + 1, itemId); } else { - return addItemAt(indexOfId(lastItemId()) + 1, itemId); + return addItemAtInternalIndex(0, itemId); } } public boolean containsId(Object itemId) { + // only look at visible items after filtering return list.contains(itemId); } @@ -185,7 +246,12 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, } public boolean removeAllItems() throws UnsupportedOperationException { + allItems.clear(); list.clear(); + // detach listeners from all BeanItems + for (BeanItem item : beanToItem.values()) { + removeAllValueChangeListeners(item); + } beanToItem.clear(); fireItemSetChange(); return true; @@ -198,14 +264,40 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, public boolean removeItem(Object itemId) throws UnsupportedOperationException { - if (list.contains(itemId)) { - beanToItem.remove(itemId); - list.remove(itemId); - fireItemSetChange(); - return true; - } else { + if (!allItems.remove(itemId)) { return false; } + // detach listeners from Item + removeAllValueChangeListeners(beanToItem.get(itemId)); + // remove item + beanToItem.remove(itemId); + list.remove(itemId); + fireItemSetChange(); + return true; + } + + private void addValueChangeListener(BeanItem beanItem, Object propertyId) { + Property property = beanItem.getItemProperty(propertyId); + if (property instanceof ValueChangeNotifier) { + // avoid multiple notifications for the same property if + // multiple filters are in use + ValueChangeNotifier notifier = (ValueChangeNotifier) property; + notifier.removeListener(this); + notifier.addListener(this); + } + } + + private void removeValueChangeListener(BeanItem item, Object propertyId) { + Property property = item.getItemProperty(propertyId); + if (property instanceof ValueChangeNotifier) { + ((ValueChangeNotifier) property).removeListener(this); + } + } + + private void removeAllValueChangeListeners(BeanItem item) { + for (Object propertyId : item.getItemPropertyIds()) { + removeValueChangeListener(item, propertyId); + } } public int size() { @@ -227,7 +319,8 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, for (int i = 0; i < ascending.length; i++) { final boolean asc = ascending[i]; final Object property = propertyId[i]; - Collections.sort(list, new Comparator() { + // sort allItems, then filter and notify + Collections.sort(allItems, new Comparator() { @SuppressWarnings("unchecked") public int compare(BT a, BT b) { Comparable va = (Comparable) beanToItem.get(a) @@ -239,6 +332,8 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, } }); } + // notifies if anything changes in the filtered list, including order + filterAll(); } public void addListener(ItemSetChangeListener listener) { @@ -256,14 +351,13 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, private void fireItemSetChange() { if (itemSetChangeListeners != null) { - final Object[] l = itemSetChangeListeners.toArray(); final Container.ItemSetChangeEvent event = new Container.ItemSetChangeEvent() { public Container getContainer() { return BeanItemContainer.this; } }; - for (int i = 0; i < l.length; i++) { - ((ItemSetChangeListener) l[i]).containerItemSetChange(event); + for (ItemSetChangeListener listener : itemSetChangeListeners) { + listener.containerItemSetChange(event); } } } @@ -289,10 +383,12 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, @SuppressWarnings("unchecked") public void addContainerFilter(Object propertyId, String filterString, boolean ignoreCase, boolean onlyMatchPrefix) { - if (filters == null) { - allItems = list; - list = (ArrayList) list.clone(); - filters = new HashSet(); + if (filters.isEmpty()) { + list = (ArrayList) allItems.clone(); + } + // listen to change events to be able to update filtering + for (BeanItem item : beanToItem.values()) { + addValueChangeListener(item, propertyId); } Filter f = new Filter(propertyId, filterString, ignoreCase, onlyMatchPrefix); @@ -301,9 +397,34 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, fireItemSetChange(); } + /** + * Filter the view to recreate the visible item list from the unfiltered + * items, and send a notification if the set of visible items changed in any + * way. + */ + @SuppressWarnings("unchecked") + protected void filterAll() { + // avoid notification if the filtering had no effect + List originalItems = list; + // it is somewhat inefficient to do a (shallow) clone() every time + list = (ArrayList) allItems.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 BT.equals() + if (!originalItems.equals(list)) { + fireItemSetChange(); + } + } + protected void filter(Filter f) { - for (Iterator iterator = list.iterator(); iterator.hasNext();) { + Iterator iterator = list.iterator(); + while (iterator.hasNext()) { BT bean = iterator.next(); + // TODO #2517: should not swallow exceptions - requires several + // checks try { String value = getContainerProperty(bean, f.propertyId) .getValue().toString(); @@ -326,16 +447,18 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, } public void removeAllContainerFilters() { - if (filters != null) { - filters = null; - list = allItems; - fireItemSetChange(); + if (!filters.isEmpty()) { + filters = new HashSet(); + // stop listening to change events for any property + for (BeanItem item : beanToItem.values()) { + removeAllValueChangeListeners(item); + } + filterAll(); } } - @SuppressWarnings("unchecked") public void removeContainerFilters(Object propertyId) { - if (filters != null) { + if (!filters.isEmpty()) { for (Iterator iterator = filters.iterator(); iterator .hasNext();) { Filter f = iterator.next(); @@ -343,12 +466,17 @@ public class BeanItemContainer implements Indexed, Sortable, Filterable, iterator.remove(); } } - list = (ArrayList) list.clone(); - for (Filter f : filters) { - filter(f); + // stop listening to change events for the property + for (BeanItem item : beanToItem.values()) { + removeValueChangeListener(item, propertyId); } - fireItemSetChange(); + filterAll(); } } + public void valueChange(ValueChangeEvent event) { + // if a property that is used in a filter is changed, refresh filtering + filterAll(); + } + } diff --git a/src/com/itmill/toolkit/tests/containers/BeanItemContainerFilteringTest.java b/src/com/itmill/toolkit/tests/containers/BeanItemContainerFilteringTest.java new file mode 100644 index 0000000000..be3f8b1861 --- /dev/null +++ b/src/com/itmill/toolkit/tests/containers/BeanItemContainerFilteringTest.java @@ -0,0 +1,172 @@ +package com.itmill.toolkit.tests.containers; + +import com.itmill.toolkit.data.Item; +import com.itmill.toolkit.data.util.BeanItemContainer; +import com.itmill.toolkit.terminal.Sizeable; +import com.itmill.toolkit.tests.components.TestBase; +import com.itmill.toolkit.ui.Button; +import com.itmill.toolkit.ui.CheckBox; +import com.itmill.toolkit.ui.Label; +import com.itmill.toolkit.ui.Table; +import com.itmill.toolkit.ui.TextField; +import com.itmill.toolkit.ui.VerticalLayout; +import com.itmill.toolkit.ui.Button.ClickEvent; + +public class BeanItemContainerFilteringTest extends TestBase { + + private Table table; + private BeanItemContainer container; + private TextField filterString; + private TextField position; + private int nextToAdd = 1; + private Label nextLabel; + + protected static class TestBean { + private String id; + private String value; + + public TestBean() { + } + + public TestBean(String id, String value) { + setId(id); + setValue(value); + } + + public void setId(String id) { + this.id = id; + } + + public String getId() { + return id; + } + + public void setValue(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + } + + @Override + protected String getDescription() { + return "Test adding items in a filtered BeanItemContainer."; + } + + @Override + protected Integer getTicketNumber() { + return new Integer(1061); + } + + @Override + protected void setup() { + table = new Table(); + try { + container = new BeanItemContainer(TestBean.class); + table.setContainerDataSource(container); + + table.setWidth(300, Sizeable.UNITS_PIXELS); + table.setSelectable(true); + table.setMultiSelect(false); + table.setEditable(true); + table.setImmediate(true); + // table.addContainerProperty("column1", String.class, "test"); + + for (int i = 0; i < 25; ++i) { + container.addItem(new TestBean("Item " + i, "Value for " + i)); + } + + VerticalLayout vl = new VerticalLayout(); + + // activate & deactivate filtering + filterString = new TextField("Filter string:", "1"); + vl.addComponent(filterString); + + final CheckBox cb = new CheckBox("Filter on value", + new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + container.removeAllContainerFilters(); + if (((CheckBox) event.getSource()).booleanValue()) { + container.addContainerFilter("value", + filterString.getValue().toString(), + false, false); + } + } + }); + cb.setImmediate(true); + vl.addComponent(cb); + + nextLabel = new Label(); + nextLabel.setCaption("Next id: " + nextToAdd); + vl.addComponent(nextLabel); + + // addItemAt(idx), addItemAfter(selection), addItem() + + final Button addItemButton = new Button("addItem()", + new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + container.addItem(new TestBean("addItem() " + + nextToAdd, "value " + nextToAdd)); + nextToAdd++; + nextLabel.setCaption("Next id: " + nextToAdd); + } + }); + addItemButton.setImmediate(true); + vl.addComponent(addItemButton); + + final Button addItemAfterButton = new Button("addItemAfter()", + new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + Object selection = table.getValue(); + if (selection == null) { + return; + } + TestBean bean = new TestBean("addItemAfter() " + + nextToAdd, "value " + nextToAdd); + Item item = container.addItemAfter(selection, bean); + if (item == null) { + getMainWindow().showNotification( + "Adding item after " + selection + + " failed"); + } + nextToAdd++; + nextLabel.setCaption("Next id: " + nextToAdd); + } + }); + addItemAfterButton.setImmediate(true); + vl.addComponent(addItemAfterButton); + + position = new TextField("Position:", "0"); + vl.addComponent(position); + + final Button addItemAtButton = new Button("addItemAt()", + new Button.ClickListener() { + public void buttonClick(ClickEvent event) { + int index = Integer.parseInt(position.getValue() + .toString()); + TestBean bean = new TestBean("addItemAt() " + + nextToAdd, "value " + nextToAdd); + Item item = container.addItemAt(index, bean); + if (item == null) { + getMainWindow().showNotification( + "Adding item at index " + + position.getValue() + + " failed"); + } + nextToAdd++; + nextLabel.setCaption("Next id: " + nextToAdd); + } + }); + addItemAtButton.setImmediate(true); + vl.addComponent(addItemAtButton); + + getLayout().addComponent(table); + getLayout().addComponent(vl); + } catch (Exception ex) { + ex.printStackTrace(); + } + } + +} diff --git a/src/com/itmill/toolkit/tests/containers/BeanItemContainerTest.java b/src/com/itmill/toolkit/tests/containers/BeanItemContainerTest.java index 7cfb17041d..cc5b04a699 100644 --- a/src/com/itmill/toolkit/tests/containers/BeanItemContainerTest.java +++ b/src/com/itmill/toolkit/tests/containers/BeanItemContainerTest.java @@ -25,7 +25,7 @@ public class BeanItemContainerTest { c = new BeanItemContainer(col); - System.out.print(c + " contains " + c.size() + "objects"); + System.out.print(c + " contains " + c.size() + " objects"); } -- 2.39.5