From e752575ce341497190a7662f259b2fb0996d08d0 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Tue, 23 Sep 2014 13:07:31 +0300 Subject: [PATCH] Fix GeneratedPropertyContainer property adding and removing (#13334) Change-Id: Icda4f1b736054f5bb02cf3d7dfd2fdee79e88afd --- .../data/util/GeneratedPropertyContainer.java | 133 ++++++++++++------ .../util/GeneratedPropertyContainerTest.java | 35 +++-- 2 files changed, 117 insertions(+), 51 deletions(-) diff --git a/server/src/com/vaadin/data/util/GeneratedPropertyContainer.java b/server/src/com/vaadin/data/util/GeneratedPropertyContainer.java index 6708970f47..1ba8fdf1ab 100644 --- a/server/src/com/vaadin/data/util/GeneratedPropertyContainer.java +++ b/server/src/com/vaadin/data/util/GeneratedPropertyContainer.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -35,7 +36,26 @@ import com.vaadin.shared.ui.grid.SortDirection; import com.vaadin.ui.components.grid.sort.SortOrder; /** - * Container supporting generated properties. + * Container wrapper that adds support for generated properties. This container + * only supports adding new generated properties. Adding new normal properties + * should be done for the wrapped container. + * + *

+ * Removing properties from this container does not remove anything from the + * wrapped container but instead only hides them from the results. These + * properties can be returned to this container by calling + * {@link #addContainerProperty(Object, Class, Object)} with same property id + * which was removed. + * + *

+ * If wrapped container is Filterable and/or Sortable it should only be handled + * through this container as generated properties need to be handled in a + * specific way when sorting/filtering. + * + *

+ * Items returned by this container do not support adding or removing + * properties. Generated properties are always read-only. Trying to make them + * editable throws an exception. * * @since * @author Vaadin Ltd @@ -50,6 +70,9 @@ public class GeneratedPropertyContainer extends AbstractContainer implements private Sortable sortableContainer = null; private Filterable filterableContainer = null; + /* Removed properties which are hidden but not actually removed */ + private final Set removedProperties = new HashSet(); + /** * Property implementation for generated properties */ @@ -102,11 +125,10 @@ public class GeneratedPropertyContainer extends AbstractContainer implements /** * Item implementation for generated properties. */ - protected static class GeneratedPropertyItem implements Item { + protected class GeneratedPropertyItem implements Item { - private Map> generatedProperties = new HashMap>(); - Item wrappedItem; - Object itemId; + private Item wrappedItem; + private Object itemId; protected GeneratedPropertyItem(Object itemId, Item item) { this.itemId = itemId; @@ -115,29 +137,33 @@ public class GeneratedPropertyContainer extends AbstractContainer implements @Override public Property getItemProperty(Object id) { - if (generatedProperties.containsKey(id)) { - return generatedProperties.get(id); + if (propertyGenerators.containsKey(id)) { + return createProperty(this, id, itemId, + propertyGenerators.get(id)); } return wrappedItem.getItemProperty(id); } @Override public Collection getItemPropertyIds() { - return Sets.union(asSet(wrappedItem.getItemPropertyIds()), - asSet(generatedProperties.keySet())); + Set wrappedProperties = asSet(wrappedItem.getItemPropertyIds()); + return Sets.union( + Sets.difference(wrappedProperties, removedProperties), + propertyGenerators.keySet()); } @Override public boolean addItemProperty(Object id, Property property) throws UnsupportedOperationException { - generatedProperties.put(id, property); - return true; + throw new UnsupportedOperationException( + "GeneratedPropertyItem does not support adding properties"); } @Override public boolean removeItemProperty(Object id) throws UnsupportedOperationException { - return generatedProperties.remove(id) != null; + throw new UnsupportedOperationException( + "GeneratedPropertyItem does not support removing properties"); } }; @@ -298,15 +324,7 @@ public class GeneratedPropertyContainer extends AbstractContainer implements private Item createGeneratedPropertyItem(final Object itemId, final Item item) { - Item generatedItem = new GeneratedPropertyItem(itemId, item); - - for (Object propertyId : propertyGenerators.keySet()) { - generatedItem.addItemProperty( - propertyId, - createProperty(item, propertyId, itemId, - propertyGenerators.get(propertyId))); - } - return generatedItem; + return new GeneratedPropertyItem(itemId, item); } private Property createProperty(final Item item, @@ -315,11 +333,11 @@ public class GeneratedPropertyContainer extends AbstractContainer implements return new GeneratedProperty(item, propertyId, itemId, generator); } - private static Set asSet(Collection collection) { - if (collection instanceof Set) { - return (Set) collection; + private static LinkedHashSet asSet(Collection collection) { + if (collection instanceof LinkedHashSet) { + return (LinkedHashSet) collection; } else { - return new HashSet(collection); + return new LinkedHashSet(collection); } } @@ -537,20 +555,62 @@ public class GeneratedPropertyContainer extends AbstractContainer implements /* Property related overrides */ - @SuppressWarnings("rawtypes") @Override - public Property getContainerProperty(Object itemId, Object propertyId) { + public Property getContainerProperty(Object itemId, Object propertyId) { if (propertyGenerators.keySet().contains(propertyId)) { return getItem(itemId).getItemProperty(propertyId); - } else { + } else if (!removedProperties.contains(propertyId)) { return wrappedContainer.getContainerProperty(itemId, propertyId); } + return null; } + /** + * Returns a list of propety ids available in this container. This + * collection will contain properties for generated properties. Removed + * properties will not show unless there is a generated property overriding + * those. + */ @Override public Collection getContainerPropertyIds() { - return Sets.union(asSet(wrappedContainer.getContainerPropertyIds()), - asSet(propertyGenerators.keySet())); + Set wrappedProperties = asSet(wrappedContainer + .getContainerPropertyIds()); + return Sets.union( + Sets.difference(wrappedProperties, removedProperties), + propertyGenerators.keySet()); + } + + /** + * Adds a previously removed property back to GeneratedPropertyContainer. + * Adding a property that is not previously removed causes an + * UnsupportedOperationException. + */ + @Override + public boolean addContainerProperty(Object propertyId, Class type, + Object defaultValue) throws UnsupportedOperationException { + if (!removedProperties.contains(propertyId)) { + throw new UnsupportedOperationException( + "GeneratedPropertyContainer does not support adding properties."); + } + removedProperties.remove(propertyId); + fireContainerPropertySetChange(); + return true; + } + + /** + * Marks the given property as hidden. This property from wrapped container + * will be removed from {@link #getContainerPropertyIds()} and is no longer + * be available in Items retrieved from this container. + */ + @Override + public boolean removeContainerProperty(Object propertyId) + throws UnsupportedOperationException { + if (wrappedContainer.getContainerPropertyIds().contains(propertyId) + && removedProperties.add(propertyId)) { + fireContainerPropertySetChange(); + return true; + } + return false; } /* Type related overrides */ @@ -628,19 +688,6 @@ public class GeneratedPropertyContainer extends AbstractContainer implements return wrappedContainer.removeItem(itemId); } - @Override - public boolean addContainerProperty(Object propertyId, Class type, - Object defaultValue) throws UnsupportedOperationException { - return wrappedContainer.addContainerProperty(propertyId, type, - defaultValue); - } - - @Override - public boolean removeContainerProperty(Object propertyId) - throws UnsupportedOperationException { - return wrappedContainer.removeContainerProperty(propertyId); - } - @Override public boolean removeAllItems() throws UnsupportedOperationException { return wrappedContainer.removeAllItems(); diff --git a/server/tests/src/com/vaadin/data/util/GeneratedPropertyContainerTest.java b/server/tests/src/com/vaadin/data/util/GeneratedPropertyContainerTest.java index 9cdafebbc7..589976af2f 100644 --- a/server/tests/src/com/vaadin/data/util/GeneratedPropertyContainerTest.java +++ b/server/tests/src/com/vaadin/data/util/GeneratedPropertyContainerTest.java @@ -37,6 +37,7 @@ import com.vaadin.ui.components.grid.sort.SortOrder; public class GeneratedPropertyContainerTest { GeneratedPropertyContainer container; + Indexed wrappedContainer; private static double MILES_CONVERSION = 0.6214d; private class GeneratedPropertyListener implements @@ -243,7 +244,9 @@ public class GeneratedPropertyContainerTest { return String.class; } }); - container.addContainerProperty("baz", String.class, ""); + + // Adding property to wrapped container should cause an event + wrappedContainer.addContainerProperty("baz", String.class, ""); container.removePropertySetChangeListener(removedListener); container.removeGeneratedProperty("foo"); @@ -266,22 +269,38 @@ public class GeneratedPropertyContainerTest { } + @Test + public void testRemoveProperty() { + container.removeContainerProperty("foo"); + assertFalse("Container contained removed property", container + .getContainerPropertyIds().contains("foo")); + assertTrue("Wrapped container did not contain removed property", + wrappedContainer.getContainerPropertyIds().contains("foo")); + + assertFalse(container.getItem(container.firstItemId()) + .getItemPropertyIds().contains("foo")); + + container.addContainerProperty("foo", null, null); + assertTrue("Container did not contain returned property", container + .getContainerPropertyIds().contains("foo")); + } + private Indexed createContainer() { - Indexed container = new IndexedContainer(); - container.addContainerProperty("foo", String.class, "foo"); - container.addContainerProperty("bar", Integer.class, 0); + wrappedContainer = new IndexedContainer(); + wrappedContainer.addContainerProperty("foo", String.class, "foo"); + wrappedContainer.addContainerProperty("bar", Integer.class, 0); // km contains double values from 0.0 to 2.0 - container.addContainerProperty("km", Double.class, 0); + wrappedContainer.addContainerProperty("km", Double.class, 0); for (int i = 0; i <= 10; ++i) { - Object itemId = container.addItem(); - Item item = container.getItem(itemId); + Object itemId = wrappedContainer.addItem(); + Item item = wrappedContainer.getItem(itemId); item.getItemProperty("foo").setValue("foo"); item.getItemProperty("bar").setValue(i); item.getItemProperty("km").setValue(i / 5.0d); } - return container; + return wrappedContainer; } } -- 2.39.5