Author: manik.surtani(a)jboss.com Date: 2008-05-12 15:24:51 -0400 (Mon, 12 May 2008) New Revision: 5832 Modified: core/trunk/src/main/java/org/jboss/cache/commands/read/GetDataMapCommand.java core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java Log: JBCACHE-1337 - Node.getData() is immutable but not defensively copied. Modified: core/trunk/src/main/java/org/jboss/cache/commands/read/GetDataMapCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/read/GetDataMapCommand.java 2008-05-12 17:20:48 UTC (rev 5831) +++ core/trunk/src/main/java/org/jboss/cache/commands/read/GetDataMapCommand.java 2008-05-12 19:24:51 UTC (rev 5832) @@ -4,9 +4,8 @@ import org.jboss.cache.InvocationContext; import org.jboss.cache.NodeSPI; import org.jboss.cache.commands.Visitor; +import org.jboss.cache.util.MapCopy; -import java.util.Collections; - /** * Implements functionality defined by {@link org.jboss.cache.Cache#getData(org.jboss.cache.Fqn)} * <p/> @@ -39,7 +38,7 @@ { NodeSPI n = dataContainer.peek(fqn); if (n == null) return null; - return Collections.unmodifiableMap(n.getDataDirect()); + return new MapCopy(n.getDataDirect()); } public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable Modified: core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java 2008-05-12 17:20:48 UTC (rev 5831) +++ core/trunk/src/test/java/org/jboss/cache/api/NodeAPITest.java 2008-05-12 19:24:51 UTC (rev 5832) @@ -216,6 +216,56 @@ } } + public void testDefensiveCopyOfData() + { + rootNode.put("key", "value"); + Map<Object, Object> data = rootNode.getData(); + Set<Object> keys = rootNode.getKeys(); + + assert keys.size() == 1; + assert keys.contains("key"); + + assert data.size() == 1; + assert data.containsKey("key"); + + // now change stuff. + + rootNode.put("key2", "value2"); + + // assert that the collections we initially got have not changed. + assert keys.size() == 1; + assert keys.contains("key"); + + assert data.size() == 1; + assert data.containsKey("key"); + } + + public void testDefensiveCopyOfChildren() + { + Fqn childFqn = Fqn.fromString("/child"); + rootNode.addChild(childFqn).put("k", "v"); + Set<Node<Object, Object>> children = rootNode.getChildren(); + Set<Object> childrenNames = rootNode.getChildrenNames(); + + assert childrenNames.size() == 1; + assert childrenNames.contains(childFqn.getLastElement()); + + assert children.size() == 1; + assert children.iterator().next().getFqn().equals(childFqn); + + // now change stuff. + + rootNode.addChild(Fqn.fromString("/child2")); + + // assert that the collections we initially got have not changed. + assert childrenNames.size() == 1; + assert childrenNames.contains(childFqn.getLastElement()); + + assert children.size() == 1; + assert children.iterator().next().getFqn().equals(childFqn); + } + + public void testImmutabilityOfChildren() { rootNode.addChild(A);