From 3c4cc9e277acf5b60bc22ed189d9784ad06d840f Mon Sep 17 00:00:00 2001 From: Anna Koskinen Date: Wed, 19 Dec 2012 13:41:49 +0200 Subject: Merge of (#9788) to Vaadin 7. HierarchicalContainer sometimes left content change events disabled. Change-Id: I77bbee1987f324246a6253f91a6edc23b153d4b1 --- .../vaadin/data/util/HierarchicalContainer.java | 227 ++++++++++++--------- 1 file changed, 131 insertions(+), 96 deletions(-) (limited to 'server') diff --git a/server/src/com/vaadin/data/util/HierarchicalContainer.java b/server/src/com/vaadin/data/util/HierarchicalContainer.java index 8338e3c190..6505a96097 100644 --- a/server/src/com/vaadin/data/util/HierarchicalContainer.java +++ b/server/src/com/vaadin/data/util/HierarchicalContainer.java @@ -23,6 +23,8 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; import com.vaadin.data.Container; import com.vaadin.data.Item; @@ -79,7 +81,12 @@ public class HierarchicalContainer extends IndexedContainer implements */ private boolean includeParentsWhenFiltering = true; - private boolean contentChangedEventsDisabled = false; + /** + * Counts how many nested contents change disable calls are in progress. + * + * Pending events are only fired when the counter reaches zero again. + */ + private int contentChangedEventsDisabledCount = 0; private boolean contentsChangedEventPending; @@ -401,21 +408,24 @@ public class HierarchicalContainer extends IndexedContainer implements @Override public Object addItem() { disableContentsChangeEvents(); - final Object itemId = super.addItem(); - if (itemId == null) { - return null; - } + try { + final Object itemId = super.addItem(); + if (itemId == null) { + return null; + } - if (!roots.contains(itemId)) { - roots.add(itemId); - if (filteredRoots != null) { - if (passesFilters(itemId)) { - filteredRoots.add(itemId); + if (!roots.contains(itemId)) { + roots.add(itemId); + if (filteredRoots != null) { + if (passesFilters(itemId)) { + filteredRoots.add(itemId); + } } } + return itemId; + } finally { + enableAndFireContentsChangeEvents(); } - enableAndFireContentsChangeEvents(); - return itemId; } @Override @@ -429,19 +439,28 @@ public class HierarchicalContainer extends IndexedContainer implements } private boolean contentsChangeEventsOn() { - return !contentChangedEventsDisabled; + return contentChangedEventsDisabledCount == 0; } private void disableContentsChangeEvents() { - contentChangedEventsDisabled = true; + contentChangedEventsDisabledCount++; } private void enableAndFireContentsChangeEvents() { - contentChangedEventsDisabled = false; - if (contentsChangedEventPending) { - fireItemSetChange(); + if (contentChangedEventsDisabledCount <= 0) { + getLogger() + .log(Level.WARNING, + "Mismatched calls to disable and enable contents change events in HierarchicalContainer"); + contentChangedEventsDisabledCount = 0; + } else { + contentChangedEventsDisabledCount--; + } + if (contentChangedEventsDisabledCount == 0) { + if (contentsChangedEventPending) { + fireItemSetChange(); + } + contentsChangedEventPending = false; } - contentsChangedEventPending = false; } /* @@ -452,20 +471,23 @@ public class HierarchicalContainer extends IndexedContainer implements @Override public Item addItem(Object itemId) { disableContentsChangeEvents(); - final Item item = super.addItem(itemId); - if (item == null) { - return null; - } + try { + final Item item = super.addItem(itemId); + if (item == null) { + return null; + } - roots.add(itemId); + roots.add(itemId); - if (filteredRoots != null) { - if (passesFilters(itemId)) { - filteredRoots.add(itemId); + if (filteredRoots != null) { + if (passesFilters(itemId)) { + filteredRoots.add(itemId); + } } + return item; + } finally { + enableAndFireContentsChangeEvents(); } - enableAndFireContentsChangeEvents(); - return item; } /* @@ -476,25 +498,28 @@ public class HierarchicalContainer extends IndexedContainer implements @Override public boolean removeAllItems() { disableContentsChangeEvents(); - final boolean success = super.removeAllItems(); - - if (success) { - roots.clear(); - parent.clear(); - children.clear(); - noChildrenAllowed.clear(); - if (filteredRoots != null) { - filteredRoots = null; - } - if (filteredChildren != null) { - filteredChildren = null; - } - if (filteredParent != null) { - filteredParent = null; + try { + final boolean success = super.removeAllItems(); + + if (success) { + roots.clear(); + parent.clear(); + children.clear(); + noChildrenAllowed.clear(); + if (filteredRoots != null) { + filteredRoots = null; + } + if (filteredChildren != null) { + filteredChildren = null; + } + if (filteredParent != null) { + filteredParent = null; + } } + return success; + } finally { + enableAndFireContentsChangeEvents(); } - enableAndFireContentsChangeEvents(); - return success; } /* @@ -505,68 +530,71 @@ public class HierarchicalContainer extends IndexedContainer implements @Override public boolean removeItem(Object itemId) { disableContentsChangeEvents(); - final boolean success = super.removeItem(itemId); + try { + final boolean success = super.removeItem(itemId); - if (success) { - // Remove from roots if this was a root - if (roots.remove(itemId)) { + if (success) { + // Remove from roots if this was a root + if (roots.remove(itemId)) { - // If filtering is enabled we might need to remove it from the - // filtered list also - if (filteredRoots != null) { - filteredRoots.remove(itemId); + // If filtering is enabled we might need to remove it from + // the filtered list also + if (filteredRoots != null) { + filteredRoots.remove(itemId); + } } - } - // Clear the children list. Old children will now become root nodes - LinkedList childNodeIds = children.remove(itemId); - if (childNodeIds != null) { - if (filteredChildren != null) { - filteredChildren.remove(itemId); - } - for (Object childId : childNodeIds) { - setParent(childId, null); + // Clear the children list. Old children will now become root + // nodes + LinkedList childNodeIds = children.remove(itemId); + if (childNodeIds != null) { + if (filteredChildren != null) { + filteredChildren.remove(itemId); + } + for (Object childId : childNodeIds) { + setParent(childId, null); + } } - } - // Parent of the item that we are removing will contain the item id - // in its children list - final Object parentItemId = parent.get(itemId); - if (parentItemId != null) { - final LinkedList c = children.get(parentItemId); - if (c != null) { - c.remove(itemId); + // Parent of the item that we are removing will contain the item + // id in its children list + final Object parentItemId = parent.get(itemId); + if (parentItemId != null) { + final LinkedList c = children.get(parentItemId); + if (c != null) { + c.remove(itemId); - if (c.isEmpty()) { - children.remove(parentItemId); - } + if (c.isEmpty()) { + children.remove(parentItemId); + } - // Found in the children list so might also be in the - // filteredChildren list - if (filteredChildren != null) { - LinkedList f = filteredChildren - .get(parentItemId); - if (f != null) { - f.remove(itemId); - if (f.isEmpty()) { - filteredChildren.remove(parentItemId); + // Found in the children list so might also be in the + // filteredChildren list + if (filteredChildren != null) { + LinkedList f = filteredChildren + .get(parentItemId); + if (f != null) { + f.remove(itemId); + if (f.isEmpty()) { + filteredChildren.remove(parentItemId); + } } } } } + parent.remove(itemId); + if (filteredParent != null) { + // Item id no longer has a parent as the item id is not in + // the container. + filteredParent.remove(itemId); + } + noChildrenAllowed.remove(itemId); } - parent.remove(itemId); - if (filteredParent != null) { - // Item id no longer has a parent as the item id is not in the - // container. - filteredParent.remove(itemId); - } - noChildrenAllowed.remove(itemId); - } - - enableAndFireContentsChangeEvents(); - return success; + return success; + } finally { + enableAndFireContentsChangeEvents(); + } } /** @@ -579,9 +607,12 @@ public class HierarchicalContainer extends IndexedContainer implements */ public boolean removeItemRecursively(Object itemId) { disableContentsChangeEvents(); - boolean removeItemRecursively = removeItemRecursively(this, itemId); - enableAndFireContentsChangeEvents(); - return removeItemRecursively; + try { + boolean removeItemRecursively = removeItemRecursively(this, itemId); + return removeItemRecursively; + } finally { + enableAndFireContentsChangeEvents(); + } } /** @@ -821,4 +852,8 @@ public class HierarchicalContainer extends IndexedContainer implements return super.passesFilters(itemId); } } + + private static final Logger getLogger() { + return Logger.getLogger(HierarchicalContainer.class.getName()); + } } -- cgit v1.2.3