Author: manik.surtani(a)jboss.com Date: 2009-02-13 09:53:17 -0500 (Fri, 13 Feb 2009) New Revision: 7688 Modified: core/tags/3.0.3.CR1/src/main/java/org/jboss/cache/UnversionedNode.java Log: JBCACHE-1474 Removing nonexistent key in UnversionedNode causes data loss Modified: core/tags/3.0.3.CR1/src/main/java/org/jboss/cache/UnversionedNode.java =================================================================== --- core/tags/3.0.3.CR1/src/main/java/org/jboss/cache/UnversionedNode.java 2009-02-13 14:43:12 UTC (rev 7687) +++ core/tags/3.0.3.CR1/src/main/java/org/jboss/cache/UnversionedNode.java 2009-02-13 14:53:17 UTC (rev 7688) @@ -39,14 +39,14 @@ import java.util.concurrent.ConcurrentMap; /** - * Basic data node class. Throws {@link UnsupportedOperationException} for version-specific methods like {@link #getVersion()} and - * {@link #setVersion(org.jboss.cache.optimistic.DataVersion)}, defined in {@link org.jboss.cache.NodeSPI}. + * Basic data node class. Throws {@link UnsupportedOperationException} for version-specific methods like {@link + * #getVersion()} and {@link #setVersion(org.jboss.cache.optimistic.DataVersion)}, defined in {@link + * org.jboss.cache.NodeSPI}. * * @author Manik Surtani (<a href="mailto:manik AT jboss DOT org">manik AT jboss DOT org</a>) * @since 2.0.0 */ -public class UnversionedNode<K, V> extends AbstractNode<K, V> implements InternalNode<K, V> -{ +public class UnversionedNode<K, V> extends AbstractNode<K, V> implements InternalNode<K, V> { /** * Debug log. */ @@ -63,8 +63,7 @@ /** * Constructs a new node with an FQN of Root. */ - public UnversionedNode() - { + public UnversionedNode() { this.fqn = Fqn.ROOT; initFlags(); } @@ -74,64 +73,53 @@ * * @param fqn fqn of the node */ - public UnversionedNode(Fqn fqn) - { + public UnversionedNode(Fqn fqn) { this.fqn = fqn; initFlags(); } - public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache, boolean lockForChildInsertRemove) - { + public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache, boolean lockForChildInsertRemove) { initFlags(); this.cache = cache; setLockForChildInsertRemove(lockForChildInsertRemove); this.fqn = fqn; // if this is a root node, create the child map. - if (fqn.isRoot()) - { + if (fqn.isRoot()) { children = new ConcurrentHashMap<Object, InternalNode<K, V>>(64, .5f, 16); - } - else - { + } else { // this always needs to be initialized. The actual cost of the ConcurrentHashMap, however, is deferred. children = new SelfInitializingConcurrentHashMap<Object, InternalNode<K, V>>(); } } - public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache, boolean lockForChildInsertRemove, Map<K, V> data) - { + public UnversionedNode(Fqn fqn, CacheSPI<K, V> cache, boolean lockForChildInsertRemove, Map<K, V> data) { this(fqn, cache, lockForChildInsertRemove); if (data != null) this.data = new FastCopyHashMap<K, V>(data); } /** - * This method initialises flags on the node, by setting DATA_LOADED to true and VALID to true and all other flags to false. - * The flags are defined in the {@link org.jboss.cache.AbstractNode.NodeFlags} enum. + * This method initialises flags on the node, by setting DATA_LOADED to true and VALID to true and all other flags to + * false. The flags are defined in the {@link org.jboss.cache.AbstractNode.NodeFlags} enum. */ - protected void initFlags() - { + protected void initFlags() { setFlag(DATA_LOADED); setFlag(VALID); } - public NodeSPI<K, V> getDelegate() - { + public NodeSPI<K, V> getDelegate() { return delegate; } - public void setDelegate(NodeSPI<K, V> delegate) - { + public void setDelegate(NodeSPI<K, V> delegate) { this.delegate = delegate; } /** * Returns a parent by checking the TreeMap by name. */ - public NodeSPI<K, V> getParent() - { - if (fqn.isRoot()) - { + public NodeSPI<K, V> getParent() { + if (fqn.isRoot()) { return null; } return cache.peek(fqn.getParent(), true); @@ -143,42 +131,34 @@ // if (data == null) data = new FastCopyHashMap<K, V>(); // } - public CacheSPI<K, V> getCache() - { + public CacheSPI<K, V> getCache() { return cache; } - public boolean isChildrenLoaded() - { + public boolean isChildrenLoaded() { return isFlagSet(CHILDREN_LOADED); } - public void setChildrenLoaded(boolean childrenLoaded) - { + public void setChildrenLoaded(boolean childrenLoaded) { setFlag(CHILDREN_LOADED, childrenLoaded); } - public V get(K key) - { + public V get(K key) { return data == null ? null : data.get(key); } - public Map<K, V> getData() - { + public Map<K, V> getData() { if (data == null) return Collections.emptyMap(); return data; } - public V put(K key, V value) - { - if (data == null) - { + public V put(K key, V value) { + if (data == null) { // new singleton map! data = Collections.singletonMap(key, value); return null; } - if (data.size() == 1 && data.containsKey(key)) - { + if (data.size() == 1 && data.containsKey(key)) { V oldVal = data.get(key); data = Collections.singletonMap(key, value); return oldVal; @@ -188,17 +168,12 @@ } @Override - public InternalNode<K, V> getChild(Fqn f) - { - if (fqn.size() == 1) - { + public InternalNode<K, V> getChild(Fqn f) { + if (fqn.size() == 1) { return getChild(fqn.getLastElement()); - } - else - { + } else { InternalNode<K, V> currentNode = this; - for (int i = 0; i < fqn.size(); i++) - { + for (int i = 0; i < fqn.size(); i++) { Object nextChildName = fqn.get(i); currentNode = currentNode.getChild(nextChildName); if (currentNode == null) return null; @@ -208,21 +183,18 @@ } @Override - public InternalNode<K, V> getChild(Object childName) - { + public InternalNode<K, V> getChild(Object childName) { if (childName == null) return null; return children().get(childName); } @Override - public Set<InternalNode<K, V>> getChildren() - { + public Set<InternalNode<K, V>> getChildren() { // strip out deleted child nodes... if (children.isEmpty()) return Collections.emptySet(); Set<InternalNode<K, V>> exclDeleted = new HashSet<InternalNode<K, V>>(); - for (InternalNode<K, V> n : children().values()) - { + for (InternalNode<K, V> n : children().values()) { if (!n.isRemoved()) exclDeleted.add(n); } exclDeleted = Collections.unmodifiableSet(exclDeleted); @@ -230,86 +202,65 @@ } @Override - public Set<InternalNode<K, V>> getChildren(boolean includeMarkedForRemoval) - { - if (includeMarkedForRemoval) - { - if (!children.isEmpty()) - { + public Set<InternalNode<K, V>> getChildren(boolean includeMarkedForRemoval) { + if (includeMarkedForRemoval) { + if (!children.isEmpty()) { return Immutables.immutableSetConvert(children().values()); - } - else - { + } else { return Collections.emptySet(); } - } - else - { + } else { return getChildren(); } } @Override - public ConcurrentMap<Object, InternalNode<K, V>> getChildrenMap() - { + public ConcurrentMap<Object, InternalNode<K, V>> getChildrenMap() { return children(); } @Override - public void setChildrenMap(ConcurrentMap<Object, InternalNode<K, V>> children) - { + public void setChildrenMap(ConcurrentMap<Object, InternalNode<K, V>> children) { this.children = children; } @Override - public void addChild(Object nodeName, InternalNode<K, V> nodeToAdd) - { - if (nodeName != null) - { + public void addChild(Object nodeName, InternalNode<K, V> nodeToAdd) { + if (nodeName != null) { children().put(nodeName, nodeToAdd); } } @Override - public void addChild(InternalNode<K, V> child) - { + public void addChild(InternalNode<K, V> child) { addChild(child, false); } @Override - public void addChild(InternalNode<K, V> child, boolean safe) - { + public void addChild(InternalNode<K, V> child, boolean safe) { Fqn<?> childFqn = child.getFqn(); - if (safe || childFqn.isDirectChildOf(fqn)) - { + if (safe || childFqn.isDirectChildOf(fqn)) { children().put(childFqn.getLastElement(), child); - } - else - { + } else { throw new CacheException("Attempting to add a child [" + childFqn + "] to [" + fqn + "]. Can only add direct children."); } } - public V remove(K key) - { + public V remove(K key) { if (data == null) return null; V value; - if (data instanceof FastCopyHashMap) - { + if (data instanceof FastCopyHashMap) { value = data.remove(key); downgradeDataMapIfNeeded(); - } - else - { + } else { // singleton maps cannot remove! value = data.get(key); - data = null; + if (value != null) data = null; } return value; } - public void printDetails(StringBuilder sb, int indent) - { + public void printDetails(StringBuilder sb, int indent) { printDetailsInMap(sb, indent); } @@ -317,90 +268,67 @@ * Returns a debug string. */ @Override - public String toString() - { + public String toString() { StringBuilder sb = new StringBuilder(); sb.append(getClass().getSimpleName()); if (!isValid()) sb.append(" (invalid) "); - if (isRemoved()) - { + if (isRemoved()) { sb.append(" (deleted) [ ").append(fqn); - } - else - { + } else { sb.append("[ ").append(fqn); } - if (this instanceof VersionedNode) - { + if (this instanceof VersionedNode) { sb.append(" version=").append(this.getVersion()); } - if (data != null) - { - if (trace) - { + if (data != null) { + if (trace) { sb.append(" data=").append(data.keySet()); - } - else - { + } else { sb.append(" data=["); Set keys = data.keySet(); int i = 0; - for (Object o : keys) - { + for (Object o : keys) { i++; sb.append(o); - if (i == 5) - { + if (i == 5) { int more = keys.size() - 5; - if (more > 1) - { + if (more > 1) { sb.append(", and "); sb.append(more); sb.append(" more"); break; } - } - else - { + } else { sb.append(", "); } } sb.append("]"); } } - if (!children.isEmpty()) - { - if (trace) - { + if (!children.isEmpty()) { + if (trace) { sb.append(" children=").append(getChildrenNames()); - } - else - { + } else { sb.append(" children=["); Set names = getChildrenNames(); int i = 0; - for (Object o : names) - { + for (Object o : names) { i++; sb.append(o); - if (i == 5) - { + if (i == 5) { int more = names.size() - 5; - if (more > 1) - { + if (more > 1) { sb.append(", and "); sb.append(more); sb.append(" more"); break; } - } - else - { + } else { sb.append(", "); } } @@ -411,67 +339,51 @@ return sb.toString(); } - public void clear() - { + public void clear() { data = null; // if (data != null) data.clear(); } @SuppressWarnings("unchecked") - public Set<Object> getChildrenNames() - { + public Set<Object> getChildrenNames() { return children.isEmpty() ? Collections.emptySet() : Immutables.immutableSetCopy(children.keySet()); } - public Set<K> getKeys() - { - if (data == null) - { + public Set<K> getKeys() { + if (data == null) { return Collections.emptySet(); } return Immutables.immutableSetCopy(data.keySet()); } - public boolean containsKey(K key) - { + public boolean containsKey(K key) { return data != null && data.containsKey(key); } - public boolean removeChild(Object childName) - { + public boolean removeChild(Object childName) { return children.remove(childName) != null; } - public boolean removeChild(Fqn f) - { - if (f.size() == 1) - { + public boolean removeChild(Fqn f) { + if (f.size() == 1) { return removeChild(f.getLastElement()); - } - else - { + } else { NodeSPI<K, V> child = getChildDirect(f); return child != null && child.getParentDirect().removeChildDirect(f.getLastElement()); } } - public void putAll(Map<? extends K, ? extends V> data) - { - if (data != null) - { + public void putAll(Map<? extends K, ? extends V> data) { + if (data != null) { if (this.data == null) this.data = copyDataMap(data); - else - if (this.data.size() == 1 && data.size() == 1 && this.data.keySet().iterator().next().equals(data.keySet().iterator().next())) - { + else if (this.data.size() == 1 && data.size() == 1 && this.data.keySet().iterator().next().equals(data.keySet().iterator().next())) { // replace key! Entry<? extends K, ? extends V> e = data.entrySet().iterator().next(); // These casts are a work-around for an eclipse compiler bug, please don't remove ;) - this.data = Collections.singletonMap((K)e.getKey(), (V) e.getValue()); - } - else - { + this.data = Collections.singletonMap((K) e.getKey(), (V) e.getValue()); + } else { // size. Do we need to update the existing data map to a FCHM? upgradeDataMap(); this.data.putAll(data); @@ -479,41 +391,32 @@ } } - protected final void upgradeDataMap() - { + protected final void upgradeDataMap() { if (data != null && !(data instanceof FastCopyHashMap)) data = new FastCopyHashMap<K, V>(data); } - protected final void downgradeDataMapIfNeeded() - { - if (data.size() == 1 && data instanceof FastCopyHashMap) - { + protected final void downgradeDataMapIfNeeded() { + if (data.size() == 1 && data instanceof FastCopyHashMap) { Entry<K, V> e = data.entrySet().iterator().next(); data = Collections.singletonMap(e.getKey(), e.getValue()); } } - public void removeChildren() - { + public void removeChildren() { children.clear(); } - public void markAsRemoved(boolean marker, boolean recursive) - { + public void markAsRemoved(boolean marker, boolean recursive) { setFlag(REMOVED, marker); - if (recursive) - { + if (recursive) { for (InternalNode<K, V> child : children().values()) child.markAsRemoved(marker, true); } } - protected final void printIndent(StringBuilder sb, int indent) - { - if (sb != null) - { - for (int i = 0; i < indent; i++) - { + protected final void printIndent(StringBuilder sb, int indent) { + if (sb != null) { + for (int i = 0; i < indent; i++) { sb.append(" "); } } @@ -522,46 +425,39 @@ /** * Returns the name of this node. */ - public Fqn getFqn() - { + public Fqn getFqn() { return fqn; } - public void setFqn(Fqn fqn) - { - if (trace) - { + public void setFqn(Fqn fqn) { + if (trace) { log.trace(getFqn() + " set FQN " + fqn); } this.fqn = fqn; // invoke children - for (Map.Entry<Object, InternalNode<K, V>> me : children().entrySet()) - { + for (Map.Entry<Object, InternalNode<K, V>> me : children().entrySet()) { InternalNode<K, V> n = me.getValue(); Fqn cfqn = Fqn.fromRelativeElements(fqn, me.getKey()); n.setFqn(cfqn); } } - public boolean hasChildren() - { + public boolean hasChildren() { return !children.isEmpty(); } /** * Adds details of the node into a map as strings. */ - protected void printDetailsInMap(StringBuilder sb, int indent) - { + protected void printDetailsInMap(StringBuilder sb, int indent) { printIndent(sb, indent); indent += 2;// increse it sb.append(Fqn.SEPARATOR); if (!fqn.isRoot()) sb.append(fqn.getLastElement()); sb.append(" "); sb.append(data); - for (InternalNode n : children().values()) - { + for (InternalNode n : children().values()) { sb.append("\n"); n.printDetails(sb, indent); } @@ -570,94 +466,73 @@ /** * Returns true if the data was loaded from the cache loader. */ - public boolean isDataLoaded() - { + public boolean isDataLoaded() { return isFlagSet(DATA_LOADED); } /** * Sets if the data was loaded from the cache loader. */ - public void setDataLoaded(boolean dataLoaded) - { + public void setDataLoaded(boolean dataLoaded) { setFlag(DATA_LOADED, dataLoaded); } - public boolean isValid() - { + public boolean isValid() { return isFlagSet(VALID); } - public void setValid(boolean valid, boolean recursive) - { + public void setValid(boolean valid, boolean recursive) { setFlag(VALID, valid); if (trace) log.trace("Marking node " + getFqn() + " as " + (valid ? "" : "in") + "valid"); - if (recursive) - { - for (InternalNode<K, V> child : children().values()) - { + if (recursive) { + for (InternalNode<K, V> child : children().values()) { child.setValid(valid, recursive); } } } - public boolean isLockForChildInsertRemove() - { + public boolean isLockForChildInsertRemove() { return isFlagSet(LOCK_FOR_CHILD_INSERT_REMOVE); } - public void setLockForChildInsertRemove(boolean lockForChildInsertRemove) - { + public void setLockForChildInsertRemove(boolean lockForChildInsertRemove) { setFlag(LOCK_FOR_CHILD_INSERT_REMOVE, lockForChildInsertRemove); } @SuppressWarnings("unchecked") - public InternalNode<K, V> copy() - { + public InternalNode<K, V> copy() { UnversionedNode<K, V> n = new UnversionedNode<K, V>(fqn, cache, isFlagSet(LOCK_FOR_CHILD_INSERT_REMOVE)); n.data = copyDataMap(data); copyInternals(n); return n; } - protected void copyInternals(UnversionedNode n) - { + protected void copyInternals(UnversionedNode n) { // direct reference to child map n.children = children; n.delegate = delegate; n.flags = flags; } - public void setInternalState(Map state) - { - if (data == null) - { + public void setInternalState(Map state) { + if (data == null) { data = copyDataMap(state); - } - else - { + } else { // don't bother doing anything here putAll(state); } } - protected final Map copyDataMap(Map<? extends K, ? extends V> toCopyFrom) - { - if (toCopyFrom != null && toCopyFrom.size() > 0) - { + protected final Map copyDataMap(Map<? extends K, ? extends V> toCopyFrom) { + if (toCopyFrom != null && toCopyFrom.size() > 0) { Map map; - if (toCopyFrom instanceof FastCopyHashMap) - { + if (toCopyFrom instanceof FastCopyHashMap) { map = (FastCopyHashMap<K, V>) ((FastCopyHashMap<K, V>) toCopyFrom).clone(); - } - else if (toCopyFrom.size() == 1) - { + } else if (toCopyFrom.size() == 1) { Entry<? extends K, ? extends V> e = toCopyFrom.entrySet().iterator().next(); map = Collections.singletonMap(e.getKey(), e.getValue()); - } - else - { + } else { map = new FastCopyHashMap<K, V>(toCopyFrom); } return map; @@ -665,8 +540,7 @@ return null; } - public Map getInternalState(boolean onlyInternalState) - { + public Map getInternalState(boolean onlyInternalState) { if (onlyInternalState) return new HashMap<K, V>(0); // don't bother doing anything here @@ -674,31 +548,24 @@ return new HashMap<K, V>(data); } - public void releaseObjectReferences(boolean recursive) - { - if (recursive) - { - for (InternalNode<K, V> child : children().values()) - { + public void releaseObjectReferences(boolean recursive) { + if (recursive) { + for (InternalNode<K, V> child : children().values()) { child.releaseObjectReferences(recursive); } } - if (data != null) - { - for (K key : data.keySet()) - { + if (data != null) { + for (K key : data.keySet()) { // get the key first, before attempting to serialize stuff since data.get() may deserialize the key if doing // a hashcode() or equals(). Object value = data.get(key); - if (key instanceof MarshalledValue) - { + if (key instanceof MarshalledValue) { ((MarshalledValue) key).compact(true, true); } - if (value instanceof MarshalledValue) - { + if (value instanceof MarshalledValue) { ((MarshalledValue) value).compact(true, true); } @@ -710,8 +577,7 @@ * @return genericized version of the child map */ @SuppressWarnings("unchecked") - private ConcurrentMap<Object, InternalNode<K, V>> children() - { + private ConcurrentMap<Object, InternalNode<K, V>> children() { return children; } }