Added more extensive testing.
Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/f4743336 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/f4743336 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/f4743336 Branch: refs/heads/CURATOR-33 Commit: f4743336e09fa4f487b95bf72b2877c789371202 Parents: cf700d3 Author: Scott Blum <sco...@squareup.com> Authored: Fri Aug 1 02:00:52 2014 -0400 Committer: Scott Blum <sco...@squareup.com> Committed: Fri Aug 1 02:00:52 2014 -0400 ---------------------------------------------------------------------- .../framework/recipes/cache/TreeCache.java | 113 ++++++---- .../framework/recipes/cache/TreeCacheEvent.java | 10 +- .../recipes/cache/BaseTestTreeCache.java | 173 +++++++++++++++ .../framework/recipes/cache/TestTreeCache.java | 219 +++---------------- .../recipes/cache/TestTreeCacheRandomTree.java | 199 +++++++++++++++++ 5 files changed, 484 insertions(+), 230 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/f4743336/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java index 4781253..f73861d 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCache.java @@ -20,7 +20,8 @@ package org.apache.curator.framework.recipes.cache; import com.google.common.base.Function; -import com.google.common.collect.ImmutableSortedSet; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.api.BackgroundCallback; @@ -38,15 +39,15 @@ import org.apache.zookeeper.data.Stat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.Closeable; -import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.SortedSet; +import java.util.Map; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -68,7 +69,7 @@ public class TreeCache implements Closeable PENDING, LIVE, DEAD } - final class TreeNode implements Watcher, BackgroundCallback + private final class TreeNode implements Watcher, BackgroundCallback { private final AtomicReference<NodeState> nodeState = new AtomicReference<NodeState>(NodeState.PENDING); private final String path; @@ -77,12 +78,18 @@ public class TreeCache implements Closeable private final AtomicReference<byte[]> data = new AtomicReference<byte[]>(); private final AtomicReference<ConcurrentMap<String, TreeNode>> children = new AtomicReference<ConcurrentMap<String, TreeNode>>(); - TreeNode(String path, TreeNode parent) + private TreeNode(String path, TreeNode parent) { this.path = path; this.parent = parent; } + private void refresh() throws Exception + { + refreshData(); + refreshChildren(); + } + private void refreshChildren() throws Exception { outstandingOps.incrementAndGet(); @@ -104,8 +111,7 @@ public class TreeCache implements Closeable private void wasReconnected() throws Exception { - refreshData(); - refreshChildren(); + refresh(); ConcurrentMap<String, TreeNode> childMap = children.get(); if ( childMap != null ) { @@ -118,8 +124,7 @@ public class TreeCache implements Closeable private void wasCreated() throws Exception { - refreshData(); - refreshChildren(); + refresh(); } private void wasDeleted() throws Exception @@ -172,7 +177,7 @@ public class TreeCache implements Closeable switch ( event.getType() ) { case NodeCreated: - assert parent == null; + Preconditions.checkState(parent == null, "unexpected NodeCreated on non-root node"); wasCreated(); break; case NodeChildrenChanged: @@ -198,7 +203,7 @@ public class TreeCache implements Closeable switch ( event.getType() ) { case EXISTS: - // TODO: should only happen for root node + Preconditions.checkState(parent == null, "unexpected EXISTS on non-root node"); if ( event.getResultCode() == KeeperException.Code.OK.intValue() ) { nodeState.compareAndSet(NodeState.DEAD, NodeState.PENDING); @@ -284,7 +289,7 @@ public class TreeCache implements Closeable if ( outstandingOps.decrementAndGet() == 0 ) { - if ( treeState.compareAndSet(TreeState.LATENT, TreeState.STARTED) ) + if ( isInitialized.compareAndSet(false, true) ) { publishEvent(TreeCacheEvent.Type.INITIALIZED); } @@ -300,10 +305,15 @@ public class TreeCache implements Closeable } /** - * Detemines when to publish the initialized event. + * Tracks the number of outstanding background requests in flight. The first time this count reaches 0, we publish the initialized event. */ private final AtomicLong outstandingOps = new AtomicLong(0); + /** + * Have we published the {@link TreeCacheEvent.Type#INITIALIZED} event yet? + */ + private final AtomicBoolean isInitialized = new AtomicBoolean(false); + private final TreeNode root; private final CuratorFramework client; private final CloseableExecutorService executorService; @@ -391,17 +401,16 @@ public class TreeCache implements Closeable */ public void start() throws Exception { + Preconditions.checkState(treeState.compareAndSet(TreeState.LATENT, TreeState.STARTED), "already started"); client.getConnectionStateListenable().addListener(connectionStateListener); root.wasCreated(); } /** - * Close/end the cache - * - * @throws java.io.IOException errors + * Close/end the cache. */ @Override - public void close() throws IOException + public void close() { if ( treeState.compareAndSet(TreeState.STARTED, TreeState.CLOSED) ) { @@ -439,7 +448,11 @@ public class TreeCache implements Closeable TreeNode current = root; if ( fullPath.length() > root.path.length() ) { - List<String> split = ZKPaths.split(fullPath.substring(root.path.length())); + if ( root.path.length() > 1 ) + { + fullPath = fullPath.substring(root.path.length()); + } + List<String> split = ZKPaths.split(fullPath); for ( String part : split ) { ConcurrentMap<String, TreeNode> map = current.children.get(); @@ -458,14 +471,14 @@ public class TreeCache implements Closeable } /** - * Return the current set of children. There are no guarantees of accuracy. This is - * merely the most recent view of the data. The data is returned in sorted order. If there is - * no child with that path, <code>null</code> is returned. + * Return the current set of children at the given path, mapped by child name. There are no + * guarantees of accuracy; this is merely the most recent view of the data. If there is no + * node at this path, {@code null} is returned. * * @param fullPath full path to the node to check * @return a possibly-empty list of children if the node is alive, or null */ - public SortedSet<String> getCurrentChildren(String fullPath) + public Map<String, ChildData> getCurrentChildren(String fullPath) { TreeNode node = find(fullPath); if ( node == null || node.nodeState.get() != NodeState.LIVE ) @@ -473,14 +486,25 @@ public class TreeCache implements Closeable return null; } ConcurrentMap<String, TreeNode> map = node.children.get(); - SortedSet<String> result; + Map<String, ChildData> result; if ( map == null ) { - result = ImmutableSortedSet.of(); + result = ImmutableMap.of(); } else { - result = ImmutableSortedSet.copyOf(map.keySet()); + ImmutableMap.Builder<String, ChildData> builder = ImmutableMap.builder(); + for ( Map.Entry<String, TreeNode> entry : map.entrySet() ) + { + TreeNode childNode = entry.getValue(); + ChildData childData = new ChildData(childNode.path, childNode.stat.get(), childNode.data.get()); + // Double-check liveness after retreiving data. + if ( childNode.nodeState.get() == NodeState.LIVE ) + { + builder.put(entry.getKey(), childData); + } + } + result = builder.build(); } // Double-check liveness after retreiving children. @@ -489,8 +513,8 @@ public class TreeCache implements Closeable /** * Return the current data for the given path. There are no guarantees of accuracy. This is - * merely the most recent view of the data. If there is no child with that path, - * <code>null</code> is returned. + * merely the most recent view of the data. If there is no node at the given path, + * {@code null} is returned. * * @param fullPath full path to the node to check * @return data if the node is alive, or null @@ -503,29 +527,28 @@ public class TreeCache implements Closeable return null; } ChildData result = new ChildData(node.path, node.stat.get(), node.data.get()); - // Double-check liveness after retreiving stat / data. + // Double-check liveness after retreiving data. return node.nodeState.get() == NodeState.LIVE ? result : null; } - void callListeners(final TreeCacheEvent event) + private void callListeners(final TreeCacheEvent event) { listeners.forEach(new Function<TreeCacheListener, Void>() - { - @Override - public Void apply(TreeCacheListener listener) - { - try - { - listener.childEvent(client, event); - } - catch ( Exception e ) - { - handleException(e); - } - return null; - } - } - ); + { + @Override + public Void apply(TreeCacheListener listener) + { + try + { + listener.childEvent(client, event); + } + catch ( Exception e ) + { + handleException(e); + } + return null; + } + }); } /** http://git-wip-us.apache.org/repos/asf/curator/blob/f4743336/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheEvent.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheEvent.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheEvent.java index 2080d26..9548a14 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheEvent.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheEvent.java @@ -84,7 +84,15 @@ public class TreeCacheEvent CONNECTION_LOST, /** - * Posted when the initial cache has been populated. + * Posted after the initial cache has been fully populated. + * <p/> + * On startup, the cache synchronizes its internal + * state with the server, publishing a series of {@link #NODE_ADDED} events as new nodes are discovered. Once + * the cachehas been fully synchronized, this {@link #INITIALIZED} this event is published. All events + * published after this event represent actual server-side mutations. + * <p/> + * Note: because the initial population is inherently asynchronous, so it's possible to observe server-side changes + * (such as a {@link #NODE_UPDATED}) prior to this event being published. */ INITIALIZED } http://git-wip-us.apache.org/repos/asf/curator/blob/f4743336/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/BaseTestTreeCache.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/BaseTestTreeCache.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/BaseTestTreeCache.java new file mode 100644 index 0000000..f59af30 --- /dev/null +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/BaseTestTreeCache.java @@ -0,0 +1,173 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.curator.framework.recipes.cache; + +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.CuratorFrameworkFactory; +import org.apache.curator.framework.api.UnhandledErrorListener; +import org.apache.curator.retry.RetryOneTime; +import org.apache.curator.test.BaseClassForTests; +import org.apache.curator.test.Timing; +import org.apache.curator.utils.CloseableUtils; +import org.testng.Assert; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; + +public class BaseTestTreeCache extends BaseClassForTests +{ + private final Timing timing = new Timing(); + CuratorFramework client; + TreeCache cache; + private List<Throwable> exceptions; + private BlockingQueue<TreeCacheEvent> events; + TreeCacheListener eventListener; + + /** + * A TreeCache that records exceptions and automatically adds a listener. + */ + class TreeCache extends org.apache.curator.framework.recipes.cache.TreeCache + { + + TreeCache(CuratorFramework client, String path, boolean cacheData) + { + super(client, path, cacheData); + getListenable().addListener(eventListener); + } + + @Override + protected void handleException(Throwable e) + { + exceptions.add(e); + } + } + + @Override + @BeforeMethod + public void setup() throws Exception + { + super.setup(); + + exceptions = new ArrayList<Throwable>(); + events = new LinkedBlockingQueue<TreeCacheEvent>(); + eventListener = new TreeCacheListener() + { + @Override + public void childEvent(CuratorFramework client, TreeCacheEvent event) throws Exception + { + if ( event.getData() != null && event.getData().getPath().startsWith("/zookeeper") ) + { + // Suppress any events related to /zookeeper paths + return; + } + events.add(event); + } + }; + + client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + client.start(); + client.getUnhandledErrorListenable().addListener(new UnhandledErrorListener() + { + @Override + public void unhandledError(String message, Throwable e) + { + exceptions.add(e); + } + }); + cache = new TreeCache(client, "/test", true); + } + + @Override + @AfterMethod + public void teardown() throws Exception + { + try + { + try + { + if ( exceptions.size() == 1 ) + { + Assert.fail("Exception was thrown", exceptions.get(0)); + } + else if ( exceptions.size() > 1 ) + { + AssertionError error = new AssertionError("Multiple exceptions were thrown"); + for ( Throwable exception : exceptions ) + { + error.addSuppressed(exception); + } + throw error; + } + } + finally + { + CloseableUtils.closeQuietly(cache); + CloseableUtils.closeQuietly(client); + } + } + finally + { + super.teardown(); + } + } + + void assertNoMoreEvents() throws InterruptedException + { + timing.sleepABit(); + Assert.assertTrue(events.isEmpty()); + } + + TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType) throws InterruptedException + { + return assertEvent(expectedType, null); + } + + TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType, String expectedPath) throws InterruptedException + { + return assertEvent(expectedType, expectedPath, null); + } + + TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType, String expectedPath, byte[] expectedData) throws InterruptedException + { + TreeCacheEvent event = events.poll(timing.forWaiting().seconds(), TimeUnit.SECONDS); + Assert.assertNotNull(event, String.format("Expected type: %s, path: %s", expectedType, expectedPath)); + + String message = event.toString(); + Assert.assertEquals(event.getType(), expectedType, message); + if ( expectedPath == null ) + { + Assert.assertNull(event.getData(), message); + } + else + { + Assert.assertNotNull(event.getData(), message); + Assert.assertEquals(event.getData().getPath(), expectedPath, message); + } + if ( expectedData != null ) + { + Assert.assertEquals(event.getData().getData(), expectedData, message); + } + return event; + } +} http://git-wip-us.apache.org/repos/asf/curator/blob/f4743336/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java index bc999bf..f35d24d 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java @@ -19,123 +19,17 @@ package org.apache.curator.framework.recipes.cache; -import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.ImmutableSet; import org.apache.curator.framework.CuratorFramework; -import org.apache.curator.framework.CuratorFrameworkFactory; -import org.apache.curator.framework.api.UnhandledErrorListener; -import org.apache.curator.retry.RetryOneTime; -import org.apache.curator.test.BaseClassForTests; import org.apache.curator.test.KillSession; -import org.apache.curator.test.Timing; import org.apache.curator.utils.CloseableUtils; import org.apache.zookeeper.CreateMode; import org.testng.Assert; -import org.testng.annotations.AfterMethod; -import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.Semaphore; -import java.util.concurrent.TimeUnit; -public class TestTreeCache extends BaseClassForTests +public class TestTreeCache extends BaseTestTreeCache { - private final Timing timing = new Timing(); - private CuratorFramework client; - private TreeCache cache; - private List<Throwable> exceptions; - private BlockingQueue<TreeCacheEvent> events; - private TreeCacheListener eventListener; - - /** - * A TreeCache that records exceptions. - */ - class TreeCache extends org.apache.curator.framework.recipes.cache.TreeCache { - - TreeCache(CuratorFramework client, String path, boolean cacheData) - { - super(client, path, cacheData); - } - - @Override - protected void handleException(Throwable e) - { - exceptions.add(e); - } - } - - @Override - @BeforeMethod - public void setup() throws Exception - { - super.setup(); - - exceptions = new ArrayList<Throwable>(); - events = new LinkedBlockingQueue<TreeCacheEvent>(); - eventListener = new TreeCacheListener() - { - @Override - public void childEvent(CuratorFramework client, TreeCacheEvent event) throws Exception - { - if (event.getData() != null && event.getData().getPath().startsWith("/zookeeper")) - { - // Suppress any events related to /zookeeper paths - return; - } - events.add(event); - } - }; - - client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); - client.start(); - client.getUnhandledErrorListenable().addListener(new UnhandledErrorListener() - { - @Override - public void unhandledError(String message, Throwable e) - { - exceptions.add(e); - } - }); - cache = new TreeCache(client, "/test", true); - cache.getListenable().addListener(eventListener); - } - - @Override - @AfterMethod - public void teardown() throws Exception - { - try - { - try - { - if ( exceptions.size() == 1 ) - { - Assert.fail("Exception was thrown", exceptions.get(0)); - } - else if ( exceptions.size() > 1 ) - { - AssertionError error = new AssertionError("Multiple exceptions were thrown"); - for ( Throwable exception : exceptions ) - { - error.addSuppressed(exception); - } - throw error; - } - } - finally - { - CloseableUtils.closeQuietly(cache); - CloseableUtils.closeQuietly(client); - } - } - finally - { - super.teardown(); - } - } - @Test public void testStartup() throws Exception { @@ -154,9 +48,9 @@ public class TestTreeCache extends BaseClassForTests assertEvent(TreeCacheEvent.Type.INITIALIZED); assertNoMoreEvents(); - Assert.assertEquals(cache.getCurrentChildren("/test"), ImmutableSortedSet.of("1", "2", "3")); - Assert.assertEquals(cache.getCurrentChildren("/test/1"), ImmutableSortedSet.of()); - Assert.assertEquals(cache.getCurrentChildren("/test/2"), ImmutableSortedSet.of("sub")); + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of("1", "2", "3")); + Assert.assertEquals(cache.getCurrentChildren("/test/1").keySet(), ImmutableSet.of()); + Assert.assertEquals(cache.getCurrentChildren("/test/2").keySet(), ImmutableSet.of("sub")); Assert.assertNull(cache.getCurrentChildren("/test/non_exist")); } @@ -176,6 +70,7 @@ public class TestTreeCache extends BaseClassForTests { client.create().forPath("/test"); client.create().forPath("/test/one", "hey there".getBytes()); + cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/one"); @@ -188,14 +83,19 @@ public class TestTreeCache extends BaseClassForTests { client.create().forPath("/test"); client.create().forPath("/test/one", "hey there".getBytes()); + cache = new TreeCache(client, "/", true); - cache.getListenable().addListener(eventListener); cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/"); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/one"); assertEvent(TreeCacheEvent.Type.INITIALIZED); assertNoMoreEvents(); + + Assert.assertTrue(cache.getCurrentChildren("/").keySet().contains("test")); + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of("one")); + Assert.assertEquals(cache.getCurrentChildren("/test/one").keySet(), ImmutableSet.of()); + Assert.assertEquals(new String(cache.getCurrentData("/test/one").getData()), "hey there"); } @Test @@ -205,13 +105,17 @@ public class TestTreeCache extends BaseClassForTests client.create().forPath("/outer/foo"); client.create().forPath("/outer/test"); client.create().forPath("/outer/test/one", "hey there".getBytes()); + cache = new TreeCache(client.usingNamespace("outer"), "/test", true); - cache.getListenable().addListener(eventListener); cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/one"); assertEvent(TreeCacheEvent.Type.INITIALIZED); assertNoMoreEvents(); + + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of("one")); + Assert.assertEquals(cache.getCurrentChildren("/test/one").keySet(), ImmutableSet.of()); + Assert.assertEquals(new String(cache.getCurrentData("/test/one").getData()), "hey there"); } @Test @@ -221,8 +125,8 @@ public class TestTreeCache extends BaseClassForTests client.create().forPath("/outer/foo"); client.create().forPath("/outer/test"); client.create().forPath("/outer/test/one", "hey there".getBytes()); + cache = new TreeCache(client.usingNamespace("outer"), "/", true); - cache.getListenable().addListener(eventListener); cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/"); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/foo"); @@ -230,6 +134,11 @@ public class TestTreeCache extends BaseClassForTests assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/one"); assertEvent(TreeCacheEvent.Type.INITIALIZED); assertNoMoreEvents(); + Assert.assertEquals(cache.getCurrentChildren("/").keySet(), ImmutableSet.of("foo", "test")); + Assert.assertEquals(cache.getCurrentChildren("/foo").keySet(), ImmutableSet.of()); + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of("one")); + Assert.assertEquals(cache.getCurrentChildren("/test/one").keySet(), ImmutableSet.of()); + Assert.assertEquals(new String(cache.getCurrentData("/test/one").getData()), "hey there"); } @Test @@ -237,6 +146,7 @@ public class TestTreeCache extends BaseClassForTests { cache.start(); assertEvent(TreeCacheEvent.Type.INITIALIZED); + client.create().forPath("/test"); client.create().forPath("/test/one", "hey there".getBytes()); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); @@ -264,10 +174,9 @@ public class TestTreeCache extends BaseClassForTests @Test public void testUpdateWhenNotCachingData() throws Exception { - cache = new TreeCache(client, "/test", false); - cache.getListenable().addListener(eventListener); - client.create().forPath("/test"); + + cache = new TreeCache(client, "/test", false); cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); assertEvent(TreeCacheEvent.Type.INITIALIZED); @@ -285,8 +194,8 @@ public class TestTreeCache extends BaseClassForTests { client.create().forPath("/test"); client.create().forPath("/test/foo", "one".getBytes()); - cache.start(); + cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/foo"); assertEvent(TreeCacheEvent.Type.INITIALIZED); @@ -299,38 +208,12 @@ public class TestTreeCache extends BaseClassForTests assertNoMoreEvents(); } - // see https://github.com/Netflix/curator/issues/27 - was caused by not comparing old->new data - @Test - public void testIssue27() throws Exception - { - client.create().forPath("/test"); - client.create().forPath("/test/a"); - client.create().forPath("/test/b"); - client.create().forPath("/test/c"); - - client.getChildren().forPath("/test"); - - cache.start(); - assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); - assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/a"); - assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/b"); - assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/c"); - assertEvent(TreeCacheEvent.Type.INITIALIZED); - - client.delete().forPath("/test/a"); - client.create().forPath("/test/a"); - assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/a"); - assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/a"); - - assertNoMoreEvents(); - } - @Test public void testKilledSession() throws Exception { client.create().forPath("/test"); - cache.start(); + cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); assertEvent(TreeCacheEvent.Type.INITIALIZED); @@ -352,24 +235,25 @@ public class TestTreeCache extends BaseClassForTests public void testBasics() throws Exception { client.create().forPath("/test"); + cache.start(); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test"); assertEvent(TreeCacheEvent.Type.INITIALIZED); - Assert.assertEquals(cache.getCurrentChildren("/test"), ImmutableSortedSet.of()); + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of()); client.create().forPath("/test/one", "hey there".getBytes()); assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/one"); - Assert.assertEquals(cache.getCurrentChildren("/test"), ImmutableSortedSet.of("one")); + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of("one")); Assert.assertEquals(new String(cache.getCurrentData("/test/one").getData()), "hey there"); client.setData().forPath("/test/one", "sup!".getBytes()); assertEvent(TreeCacheEvent.Type.NODE_UPDATED, "/test/one"); - Assert.assertEquals(cache.getCurrentChildren("/test"), ImmutableSortedSet.of("one")); + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of("one")); Assert.assertEquals(new String(cache.getCurrentData("/test/one").getData()), "sup!"); client.delete().forPath("/test/one"); assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/one"); - Assert.assertEquals(cache.getCurrentChildren("/test"), ImmutableSortedSet.of()); + Assert.assertEquals(cache.getCurrentChildren("/test").keySet(), ImmutableSet.of()); assertNoMoreEvents(); } @@ -378,6 +262,7 @@ public class TestTreeCache extends BaseClassForTests public void testBasicsOnTwoCaches() throws Exception { TreeCache cache2 = new TreeCache(client, "/test", true); + cache2.getListenable().removeListener(eventListener); // Don't listen on the second cache. // Just ensures the same event count; enables test flow control on cache2. final Semaphore semaphore = new Semaphore(0); @@ -393,6 +278,7 @@ public class TestTreeCache extends BaseClassForTests try { client.create().forPath("/test"); + cache.start(); cache2.start(); @@ -446,39 +332,4 @@ public class TestTreeCache extends BaseClassForTests client.delete().forPath("/test/one"); assertNoMoreEvents(); } - - private void assertNoMoreEvents() throws InterruptedException - { - timing.sleepABit(); - Assert.assertTrue(events.isEmpty()); - } - - private TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType) throws InterruptedException - { - return assertEvent(expectedType, null); - } - - private TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType, String expectedPath) throws InterruptedException - { - return assertEvent(expectedType, expectedPath, null); - } - - private TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType, String expectedPath, byte[] expectedData) throws InterruptedException - { - TreeCacheEvent event = events.poll(timing.forWaiting().seconds(), TimeUnit.SECONDS); - Assert.assertEquals(event.getType(), expectedType, event.toString()); - if ( expectedPath == null ) - { - Assert.assertNull(event.getData(), event.toString()); - } - else - { - Assert.assertEquals(event.getData().getPath(), expectedPath, event.toString()); - } - if ( expectedData != null ) - { - Assert.assertEquals(event.getData().getData(), expectedData, event.toString()); - } - return event; - } } http://git-wip-us.apache.org/repos/asf/curator/blob/f4743336/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheRandomTree.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheRandomTree.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheRandomTree.java new file mode 100644 index 0000000..368b557 --- /dev/null +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheRandomTree.java @@ -0,0 +1,199 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.curator.framework.recipes.cache; + +import com.google.common.collect.Iterables; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.utils.ZKPaths; +import org.testng.Assert; +import org.testng.annotations.Test; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Random; + +public class TestTreeCacheRandomTree extends BaseTestTreeCache +{ + /** + * A randomly generated source-of-truth node for {@link #testGiantRandomDeepTree()} + */ + private static final class TestNode + { + String fullPath; + byte[] data; + Map<String, TestNode> children = new HashMap<String, TestNode>(); + + TestNode(String fullPath, byte[] data) + { + this.fullPath = fullPath; + this.data = data; + } + } + + // These constants will produce a tree about 10 levels deep. + private static final int ITERATIONS = 1000; + private static final double DIVE_CHANCE = 0.9; + + private final Random random = new Random(); + + /** + * Randomly construct a large tree of test data in memory, mirror it into ZK, and then use + * a TreeCache to follow the changes. At each step, assert that TreeCache matches our + * source-of-truth test data, and that we see exactly the set of events we expect to see. + */ + @Test + public void testGiantRandomDeepTree() throws Exception + { + client.create().forPath("/tree", null); + CuratorFramework cl = client.usingNamespace("tree"); + cache = new TreeCache(cl, "/", true); + cache.getListenable().addListener(eventListener); + cache.start(); + assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/"); + assertEvent(TreeCacheEvent.Type.INITIALIZED); + + TestNode root = new TestNode("/", null); + int maxDepth = 0; + int adds = 0; + int removals = 0; + int updates = 0; + + for ( int i = 0; i < ITERATIONS; ++i ) + { + // Select a node to update, randomly navigate down through the tree + int depth = 0; + TestNode last = null; + TestNode node = root; + while ( !node.children.isEmpty() && random.nextDouble() < DIVE_CHANCE ) + { + // Go down a level in the tree. Select a random child for the next iteration. + last = node; + node = Iterables.get(node.children.values(), random.nextInt(node.children.size())); + ++depth; + } + maxDepth = Math.max(depth, maxDepth); + + // Okay we found a node, let's do something interesting with it. + switch ( random.nextInt(3) ) + { + case 0: + // Try a removal if we have no children and we're not the root node. + if ( node != root && node.children.isEmpty() ) + { + // Delete myself from parent. + TestNode removed = last.children.remove(ZKPaths.getNodeFromPath(node.fullPath)); + Assert.assertSame(node, removed); + + // Delete from ZK + cl.delete().forPath(node.fullPath); + + // TreeCache should see the delete. + assertEvent(TreeCacheEvent.Type.NODE_REMOVED, node.fullPath); + ++removals; + } + break; + case 1: + // Do an update. + byte[] newData = new byte[10]; + random.nextBytes(newData); + + if ( Arrays.equals(node.data, newData) ) + { + // Randomly generated the same data! Very small chance, just skip. + continue; + } + + // Update source-of-truth. + node.data = newData; + + // Update in ZK. + cl.setData().forPath(node.fullPath, node.data); + + // TreeCache should see the update. + assertEvent(TreeCacheEvent.Type.NODE_UPDATED, node.fullPath, node.data); + + ++updates; + break; + case 2: + // Add a new child. + String name = Long.toHexString(random.nextLong()); + if ( node.children.containsKey(name) ) + { + // Randomly generated the same name! Very small chance, just skip. + continue; + } + + // Add a new child to our test tree. + byte[] data = new byte[10]; + random.nextBytes(data); + TestNode child = new TestNode(ZKPaths.makePath(node.fullPath, name), data); + node.children.put(name, child); + + // Add to ZK. + cl.create().forPath(child.fullPath, child.data); + + // TreeCache should see the add. + assertEvent(TreeCacheEvent.Type.NODE_ADDED, child.fullPath, child.data); + + ++adds; + break; + } + + // Each iteration, ensure the cached state matches our source-of-truth tree. + assertNodeEquals(cache.getCurrentData("/"), root); + assertTreeEquals(cache, root); + } + + // Typical stats for this test: maxDepth: 10, adds: 349, removals: 198, updates: 320 + // We get more adds than removals because removals only happen if we're at a leaf. + System.out.println(String.format("maxDepth: %s, adds: %s, removals: %s, updates: %s", maxDepth, adds, removals, updates)); + assertNoMoreEvents(); + } + + /** + * Recursively assert that current children equal expected children. + */ + private static void assertTreeEquals(TreeCache cache, TestNode expectedNode) + { + String path = expectedNode.fullPath; + Map<String, ChildData> cacheChildren = cache.getCurrentChildren(path); + Assert.assertNotNull(cacheChildren, path); + Assert.assertEquals(cacheChildren.keySet(), expectedNode.children.keySet(), path); + + for ( Map.Entry<String, TestNode> entry : expectedNode.children.entrySet() ) + { + String nodeName = entry.getKey(); + ChildData childData = cacheChildren.get(nodeName); + TestNode expectedChild = entry.getValue(); + assertNodeEquals(childData, expectedChild); + assertTreeEquals(cache, expectedChild); + } + } + + /** + * Assert that the given node data matches expected test node data. + */ + private static void assertNodeEquals(ChildData actualChild, TestNode expectedNode) + { + String path = expectedNode.fullPath; + Assert.assertNotNull(actualChild, path); + Assert.assertEquals(actualChild.getData(), expectedNode.data, path); + } +}