curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject [3/9] git commit: Added more extensive testing.
Date Fri, 08 Aug 2014 02:09:10 GMT
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/master
Commit: f4743336e09fa4f487b95bf72b2877c789371202
Parents: cf700d3
Author: Scott Blum <scottb@squareup.com>
Authored: Fri Aug 1 02:00:52 2014 -0400
Committer: Scott Blum <scottb@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);
+    }
+}


Mime
View raw message